Re: [PATCH 3/5] libxl: use new QEMU xenstore protocol

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

On Fri, 2015-03-13 at 10:34 +0000, Wei Liu wrote:
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 3dedad4..4a38455 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -998,7 +998,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
>      libxl_device_vfb *vfb;
>      libxl_device_vkb *vkb;
>      char **args;
> -    struct xs_permissions perm[2];
> +    struct xs_permissions perm[1];
>      xs_transaction_t t;
>  
>      /* convenience aliases */
> @@ -1106,16 +1106,16 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
>      }
>      xs_set_target(ctx->xsh, dm_domid, guest_domid);
>  
> -    perm[0].id = dm_domid;
> -    perm[0].perms = XS_PERM_NONE;
> -    perm[1].id = guest_domid;
> -    perm[1].perms = XS_PERM_READ;
> +    perm[0].id = guest_domid;
> +    perm[0].perms = XS_PERM_READ;

This will make the node owned by the guest (and hence r/w to the guest)
and set the perms for everyone else to r/o. I don't think this is what
you meant?

(The way perms are specified is a bit confusing, check out
http://wiki.xen.org/wiki/XenBus#Permissions )


> -    return GCSPRINTF("/local/domain/0/device-model/%d/physmap/%s/%s",
> -            domid, phys_offset, node);
> +    return GCSPRINTF("/local/domain/%d/device-model/%d/physmap/%s/%s",
> +                     dm_domid, domid, phys_offset, node);

This sort of thing might imply that the helper takes the tail of the
path?
> SPRINTF(
> -                "/local/domain/0/device-model/%d/physmap", domid), &num);
> +                "/local/domain/%d/device-model/%d/physmap",

I only just noticed, but I think you want %u not %d (since dm_domid is
unsigned), although given the existing domid one is wrong too maybe you
don't want to bother...

> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index f3ae132..4aeb2bb 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -850,11 +850,14 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
>      int rc = 0;
>      char *path;
>      char *state, *vdevfn;
> +    uint32_t dm_domid;
>  
> -    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
> +    dm_domid = libxl_get_stubdom_id(CTX, domid);
> +    path = libxl__sprintf(gc, "/local/domain/%d/device-model/%d/state",
> +                          dm_domid, domid);

Maybe (up to you) switch thing to GCSPRINTF as you touch them?

Ian.