Re: [Xen-devel] [PATCH 17/25] libxl: ao: convert libxl__spawn_*

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-04-25 at 16:55 +0100, Ian Jackson wrote:
> libxl__spawn_spawn becomes a callback-style asynchronous function.
> The implementation is now in terms of libxl__ev_* including
> libxl_ev_child.
> 
> All the callers need to be updated.  This includes the device model
> spawning functions libxl__create_device_model and
> libxl__create_stubdom; these are replaced with libxl__spawn_local_dm
> and libxl__spawn_stubdom.  libxl__confirm_device_model_startup is
> abolished; instead the dm spawner calls back.
> 
> (The choice of which kind of device model to create is lifted out of
> what used to be libxl__create_device_model, because that function was
> indirectly recursive.  Recursive callback-style operations are clumsy
> because they require a pointer indirection for the nested states.)
> 
> Waiting for proper device model startup it is no longer notionally
> optional.  Previously the code appeared to tolerate this by passing
> NULL for various libxl__spawner_starting* parameters to device model
> spawners.  However, this was not used anywhere.
> 
> Conversely, the "for_spawn" parameter to libxl__wait_for_offspring is
> no longer supported.  It remains as an unused formal parameter to
> avoid updating, in this patch, all the call sites which pass NULL.
> libxl__wait_for_offspring is in any case itself an obsolete function,
> so this wrinkle will go away when its callers are updated to use the
> event system.  Consequently libxl__spawn_check is also abolished.
> 
> The "console ready" callback also remains unchanged in this patch.
> The API for this needs to be reviewed in the context of the event
> series and its reentrancy restrictions documented.
> 
> Thus their callers need to be updated.  These are the domain creation
> functions libxl_domain_create_new and _restore.  These functions now
> take ao_hows, and have a private state structure.
> 
> However domain creation remains not completely converted to the event
> mechanism; in particular it runs the outward-facing function
> libxl_run_bootloader with a NULL ao_how, which is quite wrong.  As it
> happens in the current code this is not a bug because none of the rest
> of the functionality surrounding the bootloader call will mind if the
> event loop is reentered in the middle of its execution.
> 
> The file-scope function libxl__set_fd_flag which was used by the
> previous spawn arrangements becomes unused and is removed; other
> places in libxl can use libxl_fd_set_nonblock and
> libxl_fd_set_cloexec, which of course remain.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> 
> Changes since v7:

Based on this, and a fairly cursory glance through the new version:
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

>  * Rename libxl__spawn_stubdom to libxl__spawn_stub_dm (and ..._state);
>    rename the state's stubdom_* members to dm_*.
>  * Eliminate the union between the two dm creation states in
>    libxl__domain_create_state.  Instead, the domain creation code
>    simply uses libxl__stub_dm_spawn_state.dm directly, if we're
>    taking the local dm path.
>  * Remove a spurious "break".
>  * In domain creation, move the PV non-qemu case into the switch.
>  * Code style fixes.
>  * Constify some convenience aliases.
>  * Improve comments (including typo fixes).
> ---
>  tools/libxl/libxl.h          |   14 ++-
>  tools/libxl/libxl_create.c   |  206 +++++++++++++++++++-----
>  tools/libxl/libxl_dm.c       |  219 +++++++++++++++-----------
>  tools/libxl/libxl_exec.c     |  354 
> +++++++++++++++++++++---------------------
>  tools/libxl/libxl_internal.h |  286 +++++++++++++++++++++++-----------
>  tools/libxl/xl_cmdimpl.c     |    6 +-
>  6 files changed, 683 insertions(+), 402 deletions(-)



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel