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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 16 December 2011 12:02
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 1 of 3] Make ro_paths and rw_paths
> dynamic
> 
> 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.
> 

Actually, yes, the helper function would be a much neater solution. I'll go for 
that.

  Paul

> > @@ -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