Re: [Xen-devel] [PATCH] fix rename: xenstore not fully updated

[Wei Liu <wei.liu2@xxxxxxxxxx>]

Is this a regression? Can it wait until 4.6?

On Mon, Nov 17, 2014 at 05:19:47PM +0800, Chunyan Liu wrote:
> Currently libxl__domain_rename only update /local/domain/<domid>/name,
> still some places in xenstore are not updated, including:
> /vm/<uuid>/name and /local/domain/0/backend/<device>/<domid>/.../domain.
> 

I noticed this problem a few days ago and thanks for looking into this.

I have some comments, though.

> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> ---
>  tools/libxl/libxl.c | 55 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index f7961f6..6671b08 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -359,6 +359,9 @@ int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
>      uint32_t stub_dm_domid;
>      const char *stub_dm_old_name = NULL, *stub_dm_new_name = NULL;
>      int rc;
> +    const char *vm_path, *vm_name_path;
> +    char** be_dev = NULL;
> +    unsigned int ndevs = 0;
>  
>      dom_path = libxl__xs_get_dompath(gc, domid);
>      if (!dom_path) goto x_nomem;
> @@ -429,6 +432,58 @@ int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
>          goto x_fail;
>      }
>  
> +    /* update /vm/<uuid>/name */
> +    vm_path = libxl__xs_read(gc, trans, libxl__sprintf(gc, "%s/vm", 
> dom_path));
> +    vm_name_path = libxl__sprintf(gc, "%s/name", vm_path);
> +    if (libxl__xs_write_checked(gc, trans, vm_name_path, new_name)) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to write new name `%s'"
> +                   " to %s", new_name, vm_name_path);
> +        goto x_fail;
> +    }
> +
> +
> +    /* update backend /local/domain/0/backend/<device>/<domid>/.../domain */
> +    be_dev = libxl__xs_directory(gc, trans, "/local/domain/0/backend", 
> &ndevs);

The hard-coded 0 cannot work if you have driver domain.

At the very least it should be using LIBXL_TOOLSTACK_DOMID.

> +    if (be_dev && ndevs) {
> +        for (int i = 0; i < ndevs; i++, be_dev++) {
> +            /* <device>: vbd, vif, vkbd, ... */
> +            char** be_dom = NULL;
> +            unsigned int ndoms = 0;
> +            be_dom = libxl__xs_directory(gc, trans,
> +                     GCSPRINTF("/local/domain/0/backend/%s", *be_dev), 
> &ndoms);
> +            if (be_dom && ndoms) {
> +                for (int j = 0; j < ndoms; j++, be_dom++) {
> +                    /* <device>/<domid>: vif/1, vif/2, ...*/
> +                    char ** be_devid = NULL;
> +                    unsigned int ndevids = 0;
> +
> +                    if (strcmp(*be_dom, GCSPRINTF("%d", domid)))
> +                        continue;
> +
> +                    be_devid = libxl__xs_directory(gc, trans,
> +                                     
> GCSPRINTF("/local/domain/0/backend/%s/%s",
> +                                     *be_dev, *be_dom),
> +                                     &ndevids);
> +                    if (be_devid && ndevids) {
> +                        for (int k = 0; k < ndevids; k++, be_devid++) {
> +                            /* <device>/<domid>/<devid>: vif/1/0, vif/1/1, 
> ... */
> +                            char *tmp = 
> GCSPRINTF("/local/domain/0/backend/%s/%s/%s/domain",
> +                                                  *be_dev, *be_dom, 
> *be_devid);

Same here.

And I would like to request this hunk to be in a separate function if
possible. I got lost when counting the closing "}". ;-)

Wei.

> +                            if (!libxl__xs_read(gc, trans, tmp))
> +                                continue;
> +
> +                            if (libxl__xs_write_checked(gc, trans, tmp, 
> new_name)) {
> +                                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to 
> write new name `%s'"
> +                                           " to %s", new_name, tmp);
> +                                goto x_fail;
> +                            }
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +    }
> +
>      if (stub_dm_domid) {
>          rc = libxl__domain_rename(gc, stub_dm_domid,
>                                    stub_dm_old_name,
> -- 
> 1.8.4.5

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