[RFC] support more qdisk types

[RFC] support more qdisk types

[Jim Fehlig]

[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]

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


[-- Attachment #2: RFC-support-more-qdisk-types.patch --]
[-- Type: text/x-patch, Size: 3405 bytes --]

>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


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Konrad Rzeszutek Wilk]

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?

Re: [RFC] support more qdisk types

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 1715 bytes --]

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?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Konrad Rzeszutek Wilk]

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
> 

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Konrad Rzeszutek Wilk]

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
> 

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Konrad Rzeszutek Wilk]

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
> 

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 2420 bytes --]

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.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Wei Liu]

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.

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Ian Campbell]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Ian Campbell]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Jim Fehlig]

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

Re: [RFC] support more qdisk types

[Ian Campbell]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Ian Jackson]

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.

Re: [RFC] support more qdisk types

[Wei Liu]

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
> 

Re: [RFC] support more qdisk types

[Ian Campbell]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Wei Liu]

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.

Re: [RFC] support more qdisk types

[Ian Campbell]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [RFC] support more qdisk types

[Wei Liu]

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.

Re: [RFC] support more qdisk types

[Roger Pau Monné]

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