| Message ID | 56A6BCDE.6040900@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. ssh? > > 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. At the other extreme is extending xl-disk-configuration Either way - new backends would require changes in both libxl and libvirt right? The libxl would need to understand the new 'target=' blob to parse it out?
On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. > > ssh? >> >> 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. At the other extreme is extending xl-disk-configuration > > Either way - new backends would require changes in both libxl and libvirt right? > The libxl would need to understand the new 'target=' blob to parse it out? > libvirt would probably just do what its doing now. Since it can setup the connection and pass the file descriptor into libxl. Honestly I don't see the advantage here because libvirt does a better job from a security standpoint and unless the goal is to have everything and the kitchen sink in libxl/xl. There's already a number of ways to skin the cat (xl, libvirt, xapi, openstack), why another one?
On Wed, Jan 27, 2016 at 02:25:51PM -0600, Doug Goldstein wrote: > On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. > > > > ssh? > >> > >> 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. At the other extreme is extending xl-disk-configuration > > > > Either way - new backends would require changes in both libxl and libvirt right? > > The libxl would need to understand the new 'target=' blob to parse it out? > > > > libvirt would probably just do what its doing now. Since it can setup > the connection and pass the file descriptor into libxl. Honestly I don't > see the advantage here because libvirt does a better job from a security > standpoint and unless the goal is to have everything and the kitchen > sink in libxl/xl. There's already a number of ways to skin the cat (xl, > libvirt, xapi, openstack), why another one? I believe what Jim is saying that there needs to be some parsing in libxl so that it can pass the right information to QEMU. But that is an assumption and it may be that we do not need it as you suggest? > > -- > Doug Goldstein >
On 01/27/2016 11:32 AM, Konrad Rzeszutek Wilk 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. > ssh? http://qemu.weilnetz.de/qemu-doc.html#disk_005fimages_005fssh >> 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. At the other extreme is extending xl-disk-configuration > Either way - new backends would require changes in both libxl and libvirt right? The backend is not new, it's qdisk. I'm suggesting to support more qdisk types. But yes, it requires changes to libxl, xl, and libvirt. > The libxl would need to understand the new 'target=' blob to parse it out? Right, that's one approach. The config info is encoded in 'target=' in a specified format, and libxl parses it out. Another approach is libxl provides more settings for specifying these qdisk types. Regards, Jim
On 01/27/2016 01:25 PM, Doug Goldstein wrote: > On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. >> ssh? >>> 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. At the other extreme is extending xl-disk-configuration >> Either way - new backends would require changes in both libxl and libvirt right? >> The libxl would need to understand the new 'target=' blob to parse it out? >> > libvirt would probably just do what its doing now. Since it can setup > the connection and pass the file descriptor into libxl. Hmm, I'm not sure I understand. AFAIK, there is no way to pass libxl a file descriptor opened from an image file or block device. > Honestly I don't > see the advantage here because libvirt does a better job from a security > standpoint and unless the goal is to have everything and the kitchen > sink in libxl/xl. There's already a number of ways to skin the cat (xl, > libvirt, xapi, openstack), why another one? I'm simply proposing to extend the types of devices supported by and already supported backend - qdisk. libxl configures qdisk based on the contents of the libxl_device_disk struct. This proposal extends the struct with info necessary to support the additional qdisk types. Regards, Jim
On 01/27/2016 02:09 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Jan 27, 2016 at 02:25:51PM -0600, Doug Goldstein wrote: >> On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. >>> ssh? >>>> 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. At the other extreme is extending xl-disk-configuration >>> Either way - new backends would require changes in both libxl and libvirt right? >>> The libxl would need to understand the new 'target=' blob to parse it out? >>> >> libvirt would probably just do what its doing now. Since it can setup >> the connection and pass the file descriptor into libxl. Honestly I don't >> see the advantage here because libvirt does a better job from a security >> standpoint and unless the goal is to have everything and the kitchen >> sink in libxl/xl. There's already a number of ways to skin the cat (xl, >> libvirt, xapi, openstack), why another one? > I believe what Jim is saying that there needs to be some parsing in libxl > so that it can pass the right information to QEMU. Correct. The info is also needed when libxl creates the device in xenstore. Regards, Jim
On Wed, Jan 27, 2016 at 07:42:49PM -0700, Jim Fehlig wrote: > On 01/27/2016 02:09 PM, Konrad Rzeszutek Wilk wrote: > > On Wed, Jan 27, 2016 at 02:25:51PM -0600, Doug Goldstein wrote: > >> On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. > >>> ssh? > >>>> 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. At the other extreme is extending xl-disk-configuration > >>> Either way - new backends would require changes in both libxl and libvirt right? > >>> The libxl would need to understand the new 'target=' blob to parse it out? > >>> > >> libvirt would probably just do what its doing now. Since it can setup > >> the connection and pass the file descriptor into libxl. Honestly I don't > >> see the advantage here because libvirt does a better job from a security > >> standpoint and unless the goal is to have everything and the kitchen > >> sink in libxl/xl. There's already a number of ways to skin the cat (xl, > >> libvirt, xapi, openstack), why another one? > > I believe what Jim is saying that there needs to be some parsing in libxl > > so that it can pass the right information to QEMU. > > Correct. The info is also needed when libxl creates the device in xenstore. I think that would be awesome - especially with the iSCSI and Sheepdog. The one thing that I am worried about is bitrotting. And I would think if test-cases were added for this support - while it is bigger upfront cost - would benefit us long term. Granted I say that - but I hadn't done my share either (xSplice or some other ones I have on mind) so feel free to ignore the recommendation. > > Regards, > Jim >
On 1/27/16 8:37 PM, Jim Fehlig wrote: > On 01/27/2016 01:25 PM, Doug Goldstein wrote: >> On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. >>> ssh? >>>> 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. At the other extreme is extending xl-disk-configuration >>> Either way - new backends would require changes in both libxl and libvirt right? >>> The libxl would need to understand the new 'target=' blob to parse it out? >>> >> libvirt would probably just do what its doing now. Since it can setup >> the connection and pass the file descriptor into libxl. > > Hmm, I'm not sure I understand. AFAIK, there is no way to pass libxl a file > descriptor opened from an image file or block device. > >> Honestly I don't >> see the advantage here because libvirt does a better job from a security >> standpoint and unless the goal is to have everything and the kitchen >> sink in libxl/xl. There's already a number of ways to skin the cat (xl, >> libvirt, xapi, openstack), why another one? > > I'm simply proposing to extend the types of devices supported by and already > supported backend - qdisk. libxl configures qdisk based on the contents of the > libxl_device_disk struct. This proposal extends the struct with info necessary > to support the additional qdisk types. > > Regards, > Jim > Well I apologize for my comments because I clearly didn't understand what you were proposing. Sounds good.
Konrad Rzeszutek Wilk wrote: > On Wed, Jan 27, 2016 at 07:42:49PM -0700, Jim Fehlig wrote: >> On 01/27/2016 02:09 PM, Konrad Rzeszutek Wilk wrote: >>> On Wed, Jan 27, 2016 at 02:25:51PM -0600, Doug Goldstein wrote: >>>> On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. >>>>> ssh? >>>>>> 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. At the other extreme is extending xl-disk-configuration >>>>> Either way - new backends would require changes in both libxl and libvirt right? >>>>> The libxl would need to understand the new 'target=' blob to parse it out? >>>>> >>>> libvirt would probably just do what its doing now. Since it can setup >>>> the connection and pass the file descriptor into libxl. Honestly I don't >>>> see the advantage here because libvirt does a better job from a security >>>> standpoint and unless the goal is to have everything and the kitchen >>>> sink in libxl/xl. There's already a number of ways to skin the cat (xl, >>>> libvirt, xapi, openstack), why another one? >>> I believe what Jim is saying that there needs to be some parsing in libxl >>> so that it can pass the right information to QEMU. >> Correct. The info is also needed when libxl creates the device in xenstore. > > I think that would be awesome - especially with the iSCSI and Sheepdog. > > The one thing that I am worried about is bitrotting. And I would think > if test-cases were added for this support - while it is bigger upfront > cost - would benefit us long term. Agreed. At a minimum I planned to add testing of any new disk config settings to tools/libxl/check-xl-disk-parse. Were you thinking of something more end-to-end like a new OSSTEST case? Regards, Jim
On Fri, Jan 29, 2016 at 10:18:01AM -0700, Jim Fehlig wrote: > Konrad Rzeszutek Wilk wrote: > > On Wed, Jan 27, 2016 at 07:42:49PM -0700, Jim Fehlig wrote: > >> On 01/27/2016 02:09 PM, Konrad Rzeszutek Wilk wrote: > >>> On Wed, Jan 27, 2016 at 02:25:51PM -0600, Doug Goldstein wrote: > >>>> On 1/27/16 12:32 PM, Konrad Rzeszutek Wilk 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. > >>>>> ssh? > >>>>>> 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. At the other extreme is extending xl-disk-configuration > >>>>> Either way - new backends would require changes in both libxl and libvirt right? > >>>>> The libxl would need to understand the new 'target=' blob to parse it out? > >>>>> > >>>> libvirt would probably just do what its doing now. Since it can setup > >>>> the connection and pass the file descriptor into libxl. Honestly I don't > >>>> see the advantage here because libvirt does a better job from a security > >>>> standpoint and unless the goal is to have everything and the kitchen > >>>> sink in libxl/xl. There's already a number of ways to skin the cat (xl, > >>>> libvirt, xapi, openstack), why another one? > >>> I believe what Jim is saying that there needs to be some parsing in libxl > >>> so that it can pass the right information to QEMU. > >> Correct. The info is also needed when libxl creates the device in xenstore. > > > > I think that would be awesome - especially with the iSCSI and Sheepdog. > > > > The one thing that I am worried about is bitrotting. And I would think > > if test-cases were added for this support - while it is bigger upfront > > cost - would benefit us long term. > > Agreed. At a minimum I planned to add testing of any new disk config settings to > tools/libxl/check-xl-disk-parse. Were you thinking of something more end-to-end > like a new OSSTEST case? Oh, hadn't know that existed. I was thinking OSSTest - but if check-xl-disk-parse fits the bit, pick that. > > Regards, > Jim >
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. 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. > 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. And extending the structure seems to be the right thing to do. Wei.
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? >> 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
On Tue, 2016-02-02 at 15:06 -0700, Jim Fehlig wrote: > > > 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. My main concern is that this approach requires us to update libxl for each new possible backend type. The intention of the target= in the disk spec is that it consumes the rest of the line so it can be used to encode pretty much anything. Is it not possible (modulo bugs) to pass all the necessary information to qdisk in this form? I thought Dave S had made it possible to use qdisk in this way back in: commit a8a1f236a2964506a22d1779648d8e1c8668cb1a Author: David Scott < dave.scott@eu.citrix.com > Date: Tue Apr 23 10:59:26 2013 +0100 libxl: Only call stat() when adding a disk if we expect a device to exist. We consider calling stat() a helpful error check in the following circumstances only: 1. the disk backend type must be PHYsical 2. the disk backend domain must be the same as the running libxl code (ie LIBXL_TOOLSTACK_DOMID) 3. there must not be a hotplug script because this would imply that the device won't be created until after the hotplug script has run. With this fix, it is possible to use qemu's built-in block drivers such as ceph/rbd, with a xl config disk spec like this: disk=[ 'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img' ] Signed-off-by: David Scott < dave.scott@eu.citrix.com > Acked-by: Ian Campbell < ian.campbell@citrix.com > Acked-by: Roger Pau Monné < roger.pau@citrix.com > Of course things may have regressed again since then. Since target is basically passed as is to qdisk does libvirt not already have some helpers in the QEMU backend for constructing such things? Ian.
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 >
On Wed, 2016-02-03 at 10:35 +0000, Wei Liu wrote: > > 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. How do hotplug scripts factor into this? Ian.
On Wed, Feb 03, 2016 at 10:51:27AM +0000, Ian Campbell wrote: > On Wed, 2016-02-03 at 10:35 +0000, Wei Liu wrote: > > > 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. > > How do hotplug scripts factor into this? > If supporting all such block devices requires presenting a block device to QEMU? If QEMU directly handles them then hotplug script is not in the picture. Wei. > Ian.
On Wed, 2016-02-03 at 10:55 +0000, Wei Liu wrote: > On Wed, Feb 03, 2016 at 10:51:27AM +0000, Ian Campbell wrote: > > On Wed, 2016-02-03 at 10:35 +0000, Wei Liu wrote: > > > > 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. > > > > How do hotplug scripts factor into this? > > > > If supporting all such block devices requires presenting a block device > to QEMU? If QEMU directly handles them then hotplug script is not in the > picture. Perhaps I've misunderstood what this thread is about. I thought it was about exposing all the various backends which qdisk supports natively, like CEPH, sheepdog, iscsi, nbd etc. Ian.
On Wed, Feb 03, 2016 at 11:05:04AM +0000, Ian Campbell wrote: > On Wed, 2016-02-03 at 10:55 +0000, Wei Liu wrote: > > On Wed, Feb 03, 2016 at 10:51:27AM +0000, Ian Campbell wrote: > > > On Wed, 2016-02-03 at 10:35 +0000, Wei Liu wrote: > > > > > 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. > > > > > > How do hotplug scripts factor into this? > > > > > > > If supporting all such block devices requires presenting a block device > > to QEMU? If QEMU directly handles them then hotplug script is not in the > > picture. > > Perhaps I've misunderstood what this thread is about. I thought it was > about exposing all the various backends which qdisk supports natively, like > CEPH, sheepdog, iscsi, nbd etc. > Good point. It is me who is confused. Hotplug is not in the picture then. Wei. > Ian.
El 3/2/16 a les 12:05, Ian Campbell ha escrit: > On Wed, 2016-02-03 at 10:55 +0000, Wei Liu wrote: >> On Wed, Feb 03, 2016 at 10:51:27AM +0000, Ian Campbell wrote: >>> On Wed, 2016-02-03 at 10:35 +0000, Wei Liu wrote: >>>>> 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. >>> >>> How do hotplug scripts factor into this? >>> >> >> If supporting all such block devices requires presenting a block device >> to QEMU? If QEMU directly handles them then hotplug script is not in the >> picture. > > Perhaps I've misunderstood what this thread is about. I thought it was > about exposing all the various backends which qdisk supports natively, like > CEPH, sheepdog, iscsi, nbd etc. AFAIK QEMU/Qdisk doesn't need a block device or regular file in order to work, it can use as said above things like iSCSI (which we current support using a script + blkback [0]), nbd, ceph... There's been some discussion in the past about whether hotplug scripts should also be supported by QEMU/Qdisk, or even whether the launch of the Qdisk backend itself should be done inside of a hotplug script (abstracting it away from libxl). I think George had some ideas/work on this (or did look into similar issues in the past). IMHO, I don't think we need hotplug scripts support for Qemu/Qdisk handled backends. Roger. [0] http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=bda856e1023868b1e9605e845bedbd8b7ed2944e
On 02/03/2016 02:56 AM, Ian Campbell wrote: > On Tue, 2016-02-02 at 15:06 -0700, Jim Fehlig wrote: >>> 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. > My main concern is that this approach requires us to update libxl for each > new possible backend type. Yes, understood. > > The intention of the target= in the disk spec is that it consumes the rest > of the line so it can be used to encode pretty much anything. Is it not > possible (modulo bugs) to pass all the necessary information to qdisk in > this form? I thought Dave S had made it possible to use qdisk in this way > back in: > > commit a8a1f236a2964506a22d1779648d8e1c8668cb1a > Author: David Scott < dave.scott@eu.citrix.com > > Date: Tue Apr 23 10:59:26 2013 +0100 > > libxl: Only call stat() when adding a disk if we expect a device to exist. > > We consider calling stat() a helpful error check in the following > circumstances only: > 1. the disk backend type must be PHYsical > 2. the disk backend domain must be the same as the running libxl > code (ie LIBXL_TOOLSTACK_DOMID) > 3. there must not be a hotplug script because this would imply that > the device won't be created until after the hotplug script has > run. > > With this fix, it is possible to use qemu's built-in block drivers > such as ceph/rbd, with a xl config disk spec like this: > > disk=[ 'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img' ] I thought I tried disk config along those lines with no success. But I'll certainly take a closer look at using target= to encode the config needed by these qdisk block drivers. Thanks for the pointer. Regards, Jim
On Wed, 2016-02-03 at 19:53 -0700, Jim Fehlig wrote: > > disk=[ 'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img' ] > > I thought I tried disk config along those lines with no success. But I'll > certainly take a closer look at using target= to encode the config needed by > these qdisk block drivers. Thanks for the pointer. It's certainly not impossible that this is buggy/has regressed, but if possible it would be good to just fix it (and add a test case!)... Ian.
On 02/03/2016 07:53 PM, Jim Fehlig wrote: > On 02/03/2016 02:56 AM, Ian Campbell wrote: >> On Tue, 2016-02-02 at 15:06 -0700, Jim Fehlig wrote: >>>> 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. >> My main concern is that this approach requires us to update libxl for each >> new possible backend type. > Yes, understood. > >> The intention of the target= in the disk spec is that it consumes the rest >> of the line so it can be used to encode pretty much anything. Is it not >> possible (modulo bugs) to pass all the necessary information to qdisk in >> this form? I thought Dave S had made it possible to use qdisk in this way >> back in: >> >> commit a8a1f236a2964506a22d1779648d8e1c8668cb1a >> Author: David Scott < dave.scott@eu.citrix.com > >> Date: Tue Apr 23 10:59:26 2013 +0100 >> >> libxl: Only call stat() when adding a disk if we expect a device to exist. >> >> We consider calling stat() a helpful error check in the following >> circumstances only: >> 1. the disk backend type must be PHYsical >> 2. the disk backend domain must be the same as the running libxl >> code (ie LIBXL_TOOLSTACK_DOMID) >> 3. there must not be a hotplug script because this would imply that >> the device won't be created until after the hotplug script has >> run. >> >> With this fix, it is possible to use qemu's built-in block drivers >> such as ceph/rbd, with a xl config disk spec like this: >> >> disk=[ 'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img' ] > I thought I tried disk config along those lines with no success. But I'll > certainly take a closer look at using target= to encode the config needed by > these qdisk block drivers. Thanks for the pointer. I think my previous problem was related to quoting in a disk spec containing several ceph monitors. According to $qemu-src/hw/block/rbd.c, ':' is used to delimit option=value tuples. I was escaping the colon between a ceph monitor server and port with a single '\'. E.g. disk = [ "vdev=xvdb, backendtype=qdisk, target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789" ] What I didn't realize was that libxl omitted the entire colon when writing the string to xenstore xenstore-read /local/domain/0/backend/qdisk/55/51728/params aio:rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.1006789;192.168.0.1016789;192.168.0.1026789 which caused the rbd block driver to barf. Using double backslash worked. E.g. disk = [ "vdev=xvdb, backendtype=qdisk, target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\\:6789;192.168.0.101\\:6789;192.168.0.102\\:6789" ] with corresponding xenstore entry aio:rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789 Note that specifying server:port in nbd doesn't require quoting the colon. The following config works just fine disk = [ "vdev=xvdb, backendtype=qdisk, target=nbd:192.168.0.150:5555" In the end, maybe all that's needed is a few more examples in xl-disk-configuration. Perhaps a paragraph under "Special Syntax" of 'target' doc, along with example config like the above? Regards, Jim
Ian, Some surprising behaviour of the libxlu disk spec parser is below... On Mon, 2016-02-08 at 17:54 -0700, Jim Fehlig wrote: > On 02/03/2016 07:53 PM, Jim Fehlig wrote: > > On 02/03/2016 02:56 AM, Ian Campbell wrote: > > > On Tue, 2016-02-02 at 15:06 -0700, Jim Fehlig wrote: > > > > > 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. > > > My main concern is that this approach requires us to update libxl for > > > each > > > new possible backend type. > > Yes, understood. > > > > > The intention of the target= in the disk spec is that it consumes the > > > rest > > > of the line so it can be used to encode pretty much anything. Is it > > > not > > > possible (modulo bugs) to pass all the necessary information to qdisk > > > in > > > this form? I thought Dave S had made it possible to use qdisk in this > > > way > > > back in: > > > > > > commit a8a1f236a2964506a22d1779648d8e1c8668cb1a > > > Author: David Scott < dave.scott@eu.citrix.com > > > > Date: Tue Apr 23 10:59:26 2013 +0100 > > > > > > libxl: Only call stat() when adding a disk if we expect a > > > device to exist. > > > > > > We consider calling stat() a helpful error check in the > > > following > > > circumstances only: > > > 1. the disk backend type must be PHYsical > > > 2. the disk backend domain must be the same as the running > > > libxl > > > code (ie LIBXL_TOOLSTACK_DOMID) > > > 3. there must not be a hotplug script because this would > > > imply that > > > the device won't be created until after the hotplug > > > script has > > > run. > > > > > > With this fix, it is possible to use qemu's built-in block > > > drivers > > > such as ceph/rbd, with a xl config disk spec like this: > > > > > > disk=[ > > > 'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubunt > > > u1204.img' ] > > I thought I tried disk config along those lines with no success. But > > I'll > > certainly take a closer look at using target= to encode the config > > needed by > > these qdisk block drivers. Thanks for the pointer. > > I think my previous problem was related to quoting in a disk spec containing > several ceph monitors. According to $qemu-src/hw/block/rbd.c, ':' is used to > delimit option=value tuples. I was escaping the colon between a ceph monitor > server and port with a single '\'. E.g. > > disk = [ "vdev=xvdb, backendtype=qdisk, > target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789" > ] > > What I didn't realize was that libxl omitted the entire colon when writing the > string to xenstore It's a bit surprising to me that any escaping is required at all in this context (apart perhaps from escaping any embedded quotes), but it certainly seems like there must be a bug somewhere if "\:" becomes ""! At the very least it should become ":", no? AIUI you really do need the literal "\:" to make it through to the other end, so it's possible that irrespective of the above oddity what you do need is "\\:". > xenstore-read /local/domain/0/backend/qdisk/55/51728/params > aio:rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.1006789;192.168.0.1016789;192.168.0.1026789 > > which caused the rbd block driver to barf. Using double backslash worked. E.g. > > disk = [ "vdev=xvdb, backendtype=qdisk, > target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\\:6789;192.168.0.101\\:6789;192.168.0.102\\:6789" > ] > > with corresponding xenstore entry > > aio:rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789 > > Note that specifying server:port in nbd doesn't require quoting the colon. The > following config works just fine > > disk = [ "vdev=xvdb, backendtype=qdisk, target=nbd:192.168.0.150:5555" > > In the end, maybe all that's needed is a few more examples in > xl-disk-configuration. Perhaps a paragraph under "Special Syntax" of 'target' > doc, along with example config like the above? Irrespective of any bug above once we know what is supposed to work and have fixed it so it does actually work then I agree more examples should be all which is needed. Ian.
Jim Fehlig writes ("Re: [Xen-devel] [RFC] support more qdisk types"):
> I think my previous problem was related to quoting in a disk spec containing
> several ceph monitors. According to $qemu-src/hw/block/rbd.c, ':' is used to
> delimit option=value tuples. I was escaping the colon between a ceph monitor
> server and port with a single '\'. E.g.
>
> disk = [ "vdev=xvdb, backendtype=qdisk,
> target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\:6789;192.168.0.101\:6789;192.168.0.102\:6789"
> ]
I'm not a huge fan of this embedding of a protocol in `target:' for
direct consumption by qemu, but I guess it's a useful escape hatch for
getting something through to qemu which libxl itself doesn't know
about.
But, anyway:
> What I didn't realize was that libxl omitted the entire colon when
> writing the string to xenstore
I think it is a bug that your string above was not rejected. The bug
is in libxlu_cfg.c, where the \-dequoting is done. It has if()
branches for various characters that might follow \, but if a
not-understood character is found, it (and the preceding \) are simply
discarded. Such things should be rejected (using a call to
xlu__cfgl_lexicalerror).
> Using double backslash worked. E.g.
>
> disk = [ "vdev=xvdb, backendtype=qdisk,
> target=rbd:libvirt-pool/new-libvirt-image:auth_supported=none:mon_host=192.168.0.100\\:6789;192.168.0.101\\:6789;192.168.0.102\\:6789"
> ]
Right.
> In the end, maybe all that's needed is a few more examples in
> xl-disk-configuration. Perhaps a paragraph under "Special Syntax" of 'target'
> doc, along with example config like the above?
This backslash unescaping is in fact a feature of all strings in xl
configuration files. (The syntax is borrowed from Python.)
And you are right that it doesn't seem to be documented.
Ian.
From 3a6aeb434506c620dd122b9ff19656bcdd35f081 Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Mon, 25 Jan 2016 16:57:42 -0700 Subject: [PATCH] [RFC] support more qdisk types Extend xl-disk-configuration to include settings needed to support nbd, rbd, and iSCSI qdisks. Add corresponding fields to the libxl_device_disk structure. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- docs/misc/xl-disk-configuration.txt | 43 +++++++++++++++++++++++++++++++++++++ tools/libxl/libxl_types.idl | 12 +++++++++++ 2 files changed, 55 insertions(+) diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt index 6a2118d..87a6560 100644 --- a/docs/misc/xl-disk-configuration.txt +++ b/docs/misc/xl-disk-configuration.txt @@ -168,6 +168,49 @@ Normally this option should not be specified, in which case libxl will automatically determine the most suitable backend. +backendprotocol=<backend-protocol> +---------------------------------- + +Description: Specifies the protocol used by the qdisk backend + when accessing a network-based disk +Supported values: nbd, rbd, iscsi +Mandatory: No + +backendprotocol is only supported by the qdisk backendtype. nbd uses +QEMU's internal network block device implementation. rbd uses QEMU's +integration with librados to natively access block devices on Ceph +clusters. iscsi uses QEMU's integration with libisci to access iSCSI +resources. + + +server=<host:port> +------------------ + +Description: Specifies a host and port providing access to a + network-based disk +Supported values: Valid host:port pairs +Mandatory: No + +server is only supported by the qdisk backendtype. host can be a valid +hostname or host IP address. Some backendprotocol's such as rbd allow +specifying multiple servers for accessing a network-based disk. + + +auth=<user:data> +---------------- + +Description: Specifies authentication information for a + network-based disk +Supported values: backendprotocol dependent +Mandatory: No + +auth is only supported by the qdisk backendtype. rbd supports cephx +authentication, where 'user' would be the Ceph user and 'data' the +user's base64-encoded cephx key obtained with 'ceph auth get-key <user>'. +iscsi supports CHAP username/password, in which case 'data' contains the +user's password. + + script=<script> --------------- diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 9ad7eba..44a6e06 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -123,6 +123,13 @@ libxl_disk_backend = Enumeration("disk_backend", [ (3, "QDISK"), ]) +libxl_disk_backend_protocol = Enumeration("disk_backend_protocol", [ + (0, "UNKNOWN"), + (1, "nbd"), + (2, "rbd"), + (3, "iscsi"), + ]) + libxl_nic_type = Enumeration("nic_type", [ (0, "UNKNOWN"), (1, "VIF_IOEMU"), @@ -568,6 +575,11 @@ libxl_device_disk = Struct("device_disk", [ ("is_cdrom", integer), ("direct_io_safe", bool), ("discard_enable", libxl_defbool), + ("backend_prtocol", libxl_disk_backend_protocol), + ("server", string), + ("port", integer), + ("auth_user", string), + ("auth_data", string), ]) libxl_device_nic = Struct("device_nic", [ -- 2.1.4
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. 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. 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. libxl_device_disk { ... existing libxl_device_disk_source src; } libxl_device_disk_source { libxl_disk_source_protocol protocol; int num_hosts; libxl_disk_source_host hosts; libxl_disk_source_auth auth; } enum libxl_disk_source_protocol { LIBXL_DISK_SOURCE_PROTOCOL_UNKNOWN = 0, LIBXL_DISK_SOURCE_PROTOCOL_NBD = 1, LIBXL_DISK_SOURCE_PROTOCOL_RBD = 2, LIBXL_DISK_SOURCE_PROTOCOL_ISCSI = 3, } libxl_disk_source_host { char *name; int port; } libxl_disk_source_auth { char *user; char *data; } As an initial RFC, I took a stab at something in the middle, adding a few items to the xl-disk-configuration and libxl_device_disk. Attached is a patch to the doc and IDL illustrating the proposal. Suggests, comments, and feedback warmly welcomed. Regards, Jim