Re: [RFC] support more qdisk types

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Feb 02, 2016 at 03:06:01PM -0700, Jim Fehlig wrote:
> Wei Liu wrote:
> > On Mon, Jan 25, 2016 at 05:25:02PM -0700, Jim Fehlig wrote:
> >> I would like to hear the community's opinion on supporting more qdisk types in
> >> xl/libxl, e.g. nbd, rbd, iSCSI, etc. I prefer supporting additional qdisk types
> >> in libxl over apps like xl or libvirt doing all the setup, producing a block
> >> device, and then passing that to libxl. Each libxl app would have to
> >> re-implement functionality already provided by qdisk. libxl already supports
> >> IDE, AHCI, SCSI, and Xen PV qdisks. My suggestion is to extend that to initially
> >> include nbd, rbd, and iSCSI. Sheepdog, ssh, etc. could be added in the future.
> >>
> > 
> > This is a good idea in general.
> > 
> >> I considered several approaches to supporting additional qdisk types, based
> >> primarily on changes to the disk cfg and interface. At one extreme is to change
> >> nothing and use the existing 'target=' to encode all required config for the
> >> additional qdisk types. libxl would need to be taught how to turn the blob into
> >> an appropriate qdisk. 
> > 
> > I think you mean "libxlu would need to be taught how to turn that blob
> > into an appropriate libxl_device_disk" -- libxl doesn't parse user
> > configuration, it deals with the libxl_device_disk directly to construct
> > arguments for QEMU.
> 
> Right. But if the configuration is all encoded in 'target=', libxlu already
> parses that and puts it in libxl_device_disk.pdev_path.
> 
> > Either it is done by extending target= or adding discrete options, the
> > heavy lifting is going to be in libxlu.  I don't think we want to parse
> > a string inside libxl.
> 
> Ok. So in your opinion, even if any new disk config is encoded in 'target=',
> libxlu should split that up into (new) members of libxl_device_disk, not just
> plop it into libxl_device_disk.pdev_path?
> 

No, not necessarily. I didn't look closely in the code yesterday when
replying, sorry.

If  target= has always been shoveled into pdev_path, using that would be
fine. We already have mechanism to parse target= outside of libxl in
hotplug script.

Are you aware of all those hotplug scripts living under tools/hotplug ?
Does using hotplug script sound plausible to you?

Currently hotplug script for QEMU is broken and needs fixing though, but
I'm sure we can figure it out.

Wei.

> >> At the other extreme is extending xl-disk-configuration
> >> with discrete knobs for each possible config item and making the
> >> libxl_device_disk structure more hierarchical. E.g.
> >>
> > 
> > I don't think the second half (hierarchical structure) is closely tied
> > to whether target= is extended or discrete knobs are used.
> 
> Nod.
> 
> > And extending
> > the structure seems to be the right thing to do.
> 
> So what do you think of the approach in the RFC patch? It adds discrete knobs in
> the disk config and extends the disk structure similarly. Before I can make
> progress on this we need to agree on extending the config and libxl_device_disk
> structure.
> 
> Regards,
> Jim
>