Re: [Xen-devel] [PATCH 1 of 3] Make ro_paths and rw_paths dynamic

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2011-12-16 at 11:47 +0000, Paul Durrant wrote:
> +    nr_ro_paths = 0;
> +    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
> +        ro_paths = libxl__calloc(gc, 5, sizeof(char *));
> +        ro_paths[nr_ro_paths++] = "hvmloader";
> +    } else {
> +        ro_paths = libxl__calloc(gc, 4, sizeof(char *));
> +    }
> +
> +    ro_paths[nr_ro_paths++] = "cpu";
> +    ro_paths[nr_ro_paths++] = "memory";
> +    ro_paths[nr_ro_paths++] = "device";
> +    ro_paths[nr_ro_paths++] = "control"; 

The flexarray stuff allows you do to this sort of thing without worrying
about running off the end of the allocated array etc.

Part of me thinks that if the arrays aren't static any more you might as
well just do the create in an open coded list, instead of open coding
the creation of a list and then iterating over it.

A helper function like libxl__xs_mkdir(gc, t, path, perm) would reduce
the amount of boilerplate.

> @@ -414,16 +440,16 @@ retry_transaction:
>      if (rc)
>          goto out;
>  
> -    for (i = 0; i < ARRAY_SIZE(rw_paths); i++) {
> -        char *path = libxl__sprintf(gc, "%s/%s", dom_path, rw_paths[i]);
> -        xs_mkdir(ctx->xsh, t, path);
> -        xs_set_permissions(ctx->xsh, t, path, rwperm, ARRAY_SIZE(rwperm));
> -    }
>      for (i = 0; i < ARRAY_SIZE(ro_paths); i++) {
>          char *path = libxl__sprintf(gc, "%s/%s", dom_path, ro_paths[i]);
>          xs_mkdir(ctx->xsh, t, path);
>          xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));
>      }
> +    for (i = 0; i < nr_rw_paths; i++) {
> +        char *path = libxl__sprintf(gc, "%s/%s", dom_path, rw_paths[i]);
> +        xs_mkdir(ctx->xsh, t, path);
> +        xs_set_permissions(ctx->xsh, t, path, rwperm, ARRAY_SIZE(rwperm));
> +    }

What does "xenstore-ls -fp" show before and after this re-ordering?

>      xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/uuid", vm_path), 
> uuid_string, strlen(uuid_string));
>      xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/name", vm_path), 
> info->name, strlen(info->name));


> diff -r 03138a08366b -r 24fc8670dfca tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c    Fri Dec 09 16:19:36 2011 +0000
> +++ b/tools/libxl/libxl_dm.c    Fri Dec 16 11:43:52 2011 +0000
> @@ -821,9 +821,7 @@ int libxl__create_device_model(libxl__gc
>          goto out;
>      }
>  
> -    path = libxl__sprintf(gc, "/local/domain/%d/hvmloader", info->domid);
> -    xs_mkdir(ctx->xsh, XBT_NULL, path);
> -    libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "%s/bios", path),
> +    libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, 
> "/local/domain/%d/hvmloader/bios", info->domid),
>                      "%s", libxl__domain_bios(gc, info));
>  
>      path = libxl__sprintf(gc, "/local/domain/0/device-model/%d", 
> info->domid);

Pre-existing problem but this should be libxl__xs_get_dompath.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel