Re: [PATCH 08/27] tools/libxl: Extra APIs for the save helper

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-06-15 at 14:44 +0100, Andrew Cooper wrote:
> With libxl migration v2, there will be other moving parts which might fail,
> requiring the helper to be stopped for reasons which are not its fault.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_internal.h     |    8 ++++++++
>  tools/libxl/libxl_save_callout.c |   16 ++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 4f204f9..3fcc37a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3182,6 +3182,14 @@ _hidden void libxl__xc_domain_restore(libxl__egc *egc,
>  _hidden void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
>                                             int rc, int retval, int errnoval);
>  
> +_hidden void libxl__save_helper_abort(libxl__egc *egc,
> +                                      libxl__save_helper_state *shs);
> +
> +static inline bool libxl__save_helper_inuse(const libxl__save_helper_state *shs)
> +{
> +    return libxl__ev_child_inuse(&shs->child);
> +}

Will this be used other than in libxl__save_helper_abort?

> +
>  /* Each time the dm needs to be saved, we must call suspend and then save */
>  _hidden int libxl__domain_suspend_device_model(libxl__gc *gc,
>                                             libxl__domain_suspend_state *dss);
> diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
> index 231de2f..71de297 100644
> --- a/tools/libxl/libxl_save_callout.c
> +++ b/tools/libxl/libxl_save_callout.c
> @@ -256,6 +256,22 @@ static void helper_failed(libxl__egc *egc, libxl__save_helper_state *shs,
>      libxl__kill(gc, shs->child.pid, SIGKILL, "save/restore helper");
>  }
>  
> +void libxl__save_helper_abort(libxl__egc *egc,
> +                              libxl__save_helper_state *shs)
> +{
> +    STATE_AO_GC(shs->ao);
> +
> +    if (!libxl__ev_child_inuse(&shs->child)) {
> +        helper_failed(egc, shs, ERROR_FAIL);

Did you mean helper_done here?

helper_failed deregisters the fd, and potentially sends SIGKILL,
although in this case it won't because its not inuse.

So all this is doing is deregistering the fd, which AFAICT isn't
registered if there is no pid.

In fact, apart from the specific signal used, this function looks a lot
like helper_failed.

> +        return;
> +    }
> +
> +    if (!shs->rc)
> +        shs->rc = ERROR_FAIL;
> +
> +    libxl__kill(gc, shs->child.pid, SIGTERM, "save/restore helper");
> +}
> +
>  static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
>                                     int fd, short events, short revents)
>  {