Re: [PATCH 5/6] libxl: introduce libxl__alloc_vdev

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

On Fri, 2012-03-30 at 12:43 +0100, Stefano Stabellini wrote:
> On Tue, 27 Mar 2012, Ian Campbell wrote:
> > > +static char * libxl__alloc_vdev(libxl__gc *gc, uint32_t domid, xs_transaction_t t,
> > > +        libxl_device_disk *disk)
> > > +{
> > > +    int rc = 0;
> > > +    libxl_device_disk *disks = NULL;
> > > +    int num = 0, idx = -1, max_idx = -1, i = 0;
> > > +
> > > +    rc = libxl__append_disk_list_of_type(gc, domid, t, "vbd", &disks, &num);
> > > +    if (rc) goto out;
> > > +
> > > +    rc = libxl__append_disk_list_of_type(gc, domid, t, "tap", &disks, &num);
> > > +    if (rc) goto out;
> > > +
> > > +    rc = libxl__append_disk_list_of_type(gc, domid, t, "qdisk", &disks, &num);
> > > +    if (rc) goto out;
> > 
> > This is basically an open-coded version of libxl_disk_list, isn't it?
> 
> yes it is similar, but as you can see the "smart" bit is in
> libxl__append_disk_list_of_type anyway

It's also in the list "vbd", "tap", "qdisk" which now need to be sync'd
in more than one place if it changes.

> > For this use though wouldn't it be as easy to simply iterate over idx
> > checking if a frontend dir exists?
> 
> We could try one at a time: 
> 
> xvd(a + idx) -> dev_number -> xs_read * 3
> 
> this could work, even though we would still need to read 3 possible
> paths (tap, qdisk, vbd) for each one.

You can check for the existence of the frontend dir here. I think this
is safe since only the content of the dir is(/should be) writeable by
the guest, the existence ofd that node is completely controlled by the
toolstack.

Ian.

> It would be more efficient than calling libxl__append_disk_list_of_type
> three times, but is it actually simpler?