Re: [Xen-devel] [PATCH 1/5] libxl: remove DM path in libxl__device_model_destroy

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2015-03-13 at 10:34 +0000, Wei Liu wrote:
> ... because it is the right place to clean up device model stuffs.

... and not devices_destroy_cb because it is the right ...

(also "stuff").

> And the path should use LIBXL_TOOLSTACK_DOMID instead of hardcoded 0.

Between this and what is in the next patch I think you probably should
refactor into a helper function to get the correct path for a given
domid.

FWIW I think you might also be able to do:
        GCSPRINTF("/local/domain/" #LIBXL_TOOLSTACK_DOMID "/device-model/%d",
                  domid);

If you want, although perhaps the intention is for it to eventually not
be a hard-define of 0 and become e.g. a function call or a global
variable reference, in which case best not?

> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxl/libxl.c    | 2 --
>  tools/libxl/libxl_dm.c | 6 ++++++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 088786e..46a43d5 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1659,8 +1659,6 @@ static void devices_destroy_cb(libxl__egc *egc,
>  
>      xs_rm(ctx->xsh, XBT_NULL, libxl__xs_libxl_path(gc, domid));
>      xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc,
> -                                "/local/domain/0/device-model/%d", domid));
> -    xs_rm(ctx->xsh, XBT_NULL, libxl__sprintf(gc,
>                                  "/local/domain/%d/hvmloader", domid));
>  
>      /* This is async operation, we already hold CTX lock */
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8599a6a..0fd5ffa 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1629,6 +1629,12 @@ out:
>  
>  int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
>  {
> +    char *path = GCSPRINTF("/local/domain/%d/device-model/%d",
> +                           LIBXL_TOOLSTACK_DOMID, domid);
> +
> +    if (!xs_rm(CTX->xsh, XBT_NULL, path))
> +        LOG(ERROR,"xs_rm failed for %s", path);
> +    /* We should try to destroy the device model anyway. */
>      return kill_device_model(gc,
>                  GCSPRINTF("/local/domain/%d/image/device-model-pid", domid));
>  }



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