Xen devel - May 2013 - [PATCH 0/3] libxl: allow usage of storage driver domains

Minor fixes in order to use driver domains for block devices (phy 
backends). I''m CCing George in order to see if this is suitable for 
inclusion into 4.3. IMHO I think this should be considered as a 
bug-fix, that allows the storage driver domain features already 
present in libxl to be usable for users. I''ve also created a wiki page 
that shows how to create and use storage driver domains with xl:

http://wiki.xen.org/wiki/Storage_driver_domains

Thanks, Roger.

Don't try to check physical devices if they belong to a domain
different than the one where the toolstack is running. This prevents
the following error when trying to use storage driver domains:

libxl: debug: libxl_create.c:1246:do_domain_create: ao 0x1819240: create:
how=(nil) callback=(nil) poller=0x1818fa0
libxl: debug: libxl_device.c:235:libxl__device_disk_set_backend: Disk vdev=xvda
spec.backend=phy
libxl: debug: libxl_device.c:175:disk_try_backend: Disk vdev=xvda, backend phy
unsuitable as phys path not a block device
libxl: error: libxl_device.c:278:libxl__device_disk_set_backend: no suitable
backend for disk xvda

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
 tools/libxl/libxl_device.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index a5cf446..b1fc4ef 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -161,6 +161,12 @@ static int disk_try_backend(disk_try_backend_args *a,
             goto bad_format;
         }
 
+        if (a->disk->backend_domid != LIBXL_TOOLSTACK_DOMID) {
+            LOG(DEBUG, "Disk vdev=%s, is using a storage driver domain,
"
+                       "skipping physical device check",
a->disk->vdev);
+            return backend;
+        }
+
         if (a->disk->script) {
             LOG(DEBUG, "Disk vdev=%s, uses script=... assuming phy
backend",
                 a->disk->vdev);
-- 
1.7.7.5 (Apple Git-26)


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

Prevent hotplug script execution from libxl if device is on a
different domain.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
 tools/libxl/libxl_device.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index b1fc4ef..bc86648 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -903,6 +903,13 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
     int hotplug;
     pid_t pid;
 
+    /*
+     * If device is attached from a driver domain don't try to execute
+     * hotplug scripts
+     */
+    if (aodev->dev->backend_domid != LIBXL_TOOLSTACK_DOMID)
+        goto out;
+
     /* Check if we have to execute hotplug scripts for this device
      * and return the necessary args/env vars for execution */
     hotplug = libxl__get_hotplug_script_info(gc, aodev->dev, &args,
&env,
-- 
1.7.7.5 (Apple Git-26)


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

This will be handled by the driver domain itself, since the toolstack
does not have access to the physical device because it is in a
different domain.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
 tools/libxl/libxl.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 87bda72..bc91fd5 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2104,7 +2104,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
                  * responsible for this since the block device may not
                  * exist yet.
                  */
-                if (!disk->script) {
+                if (!disk->script &&
+                    disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
                     int major, minor;
                     libxl__device_physdisk_major_minor(dev, &major,
&minor);
                     flexarray_append_pair(back, "physical-device",
-- 
1.7.7.5 (Apple Git-26)


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

On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne
wrote:
> Don't try to check physical devices if they belong to a domain
> different than the one where the toolstack is running. This prevents
> the following error when trying to use storage driver domains:
> 
> libxl: debug: libxl_create.c:1246:do_domain_create: ao 0x1819240: create:
how=(nil) callback=(nil) poller=0x1818fa0
> libxl: debug: libxl_device.c:235:libxl__device_disk_set_backend: Disk
vdev=xvda spec.backend=phy
> libxl: debug: libxl_device.c:175:disk_try_backend: Disk vdev=xvda, backend
phy unsuitable as phys path not a block device
> libxl: error: libxl_device.c:278:libxl__device_disk_set_backend: no
suitable backend for disk xvda
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>



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

On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne
wrote:
> Prevent hotplug script execution from libxl if device is on a
> different domain.
The assumption being that the user has enabled udev in the driver
domain?

Is a log message going to be worthwhile?

Does this obsolete run_hotplug_scripts somewhat?
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@citrix.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
> ---
>  tools/libxl/libxl_device.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index b1fc4ef..bc86648 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -903,6 +903,13 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
>      int hotplug;
>      pid_t pid;
>  
> +    /*
> +     * If device is attached from a driver domain don't try to execute
> +     * hotplug scripts
> +     */
> +    if (aodev->dev->backend_domid != LIBXL_TOOLSTACK_DOMID)
> +        goto out;
> +
>      /* Check if we have to execute hotplug scripts for this device
>       * and return the necessary args/env vars for execution */
>      hotplug = libxl__get_hotplug_script_info(gc, aodev->dev, &args,
&env,


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

On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne
wrote:
> This will be handled by the driver domain itself,
again by udev?
>  since the toolstack
> does not have access to the physical device because it is in a
> different domain.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@citrix.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
> ---
>  tools/libxl/libxl.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 87bda72..bc91fd5 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2104,7 +2104,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
>                   * responsible for this since the block device may not
>                   * exist yet.
>                   */
> -                if (!disk->script) {
> +                if (!disk->script &&
> +                    disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
>                      int major, minor;
>                      libxl__device_physdisk_major_minor(dev, &major,
&minor);
>                      flexarray_append_pair(back,
"physical-device",


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

On 05/03/2013 12:23 PM, Roger Pau Monne wrote:
> Don't try to check physical devices if they belong to a domain
> different than the one where the toolstack is running. This prevents
> the following error when trying to use storage driver domains:
>
> libxl: debug: libxl_create.c:1246:do_domain_create: ao 0x1819240: create:
how=(nil) callback=(nil) poller=0x1818fa0
> libxl: debug: libxl_device.c:235:libxl__device_disk_set_backend: Disk
vdev=xvda spec.backend=phy
> libxl: debug: libxl_device.c:175:disk_try_backend: Disk vdev=xvda, backend
phy unsuitable as phys path not a block device
> libxl: error: libxl_device.c:278:libxl__device_disk_set_backend: no
suitable backend for disk xvda
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@citrix.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
Re the feature freeze, these look like fixes to me;

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>


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

On 03/05/13 13:44, Ian Campbell wrote:
> On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne wrote:
>> Prevent hotplug script execution from libxl if device is on a
>> different domain.
> 
> The assumption being that the user has enabled udev in the driver
> domain?
Yes, although it doesn't have to be udev specifically, for example
something similar to xenbackendd could be used in the driver domain, but
I have only tested it with udev.
> 
> Is a log message going to be worthwhile?
There's already one log message earlier in disk_try_backend, but I guess
it won't hurt to add another one here, specifically saying that hotplug
scripts will not be executed by libxl for this specific device.
> Does this obsolete run_hotplug_scripts somewhat?
Not really, it just prevents libxl from calling hotplug scripts that
will surely fail (because the domain where the toolstack is running
doesn't have access to the device). run_hotplug_scripts tells whether
hotplug scripts will be executed by udev or libxl, so this code just
changes the behaviour when run_hotplug_scripts == 1 and backend_domid
!TOOLSTACK_DOMID.
> 
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Ian Jackson <ian.jackson@citrix.com>
>> Cc: George Dunlap <george.dunlap@citrix.com>
>> ---
>>  tools/libxl/libxl_device.c |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index b1fc4ef..bc86648 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -903,6 +903,13 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
>>      int hotplug;
>>      pid_t pid;
>>  
>> +    /*
>> +     * If device is attached from a driver domain don't try to
execute
>> +     * hotplug scripts
>> +     */
>> +    if (aodev->dev->backend_domid != LIBXL_TOOLSTACK_DOMID)
>> +        goto out;
>> +
>>      /* Check if we have to execute hotplug scripts for this device
>>       * and return the necessary args/env vars for execution */
>>      hotplug = libxl__get_hotplug_script_info(gc, aodev->dev,
&args, &env,
> 
> 

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

On 03/05/13 13:45, Ian Campbell wrote:
> On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne wrote:
>> This will be handled by the driver domain itself,
> 
> again by udev?
Or xenbackendd (or something similar). It doesn't have to be udev
specifically.
> 
>>  since the toolstack
>> does not have access to the physical device because it is in a
>> different domain.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Ian Jackson <ian.jackson@citrix.com>
>> Cc: George Dunlap <george.dunlap@citrix.com>
>> ---
>>  tools/libxl/libxl.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 87bda72..bc91fd5 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -2104,7 +2104,8 @@ static void device_disk_add(libxl__egc *egc,
uint32_t domid,
>>                   * responsible for this since the block device may not
>>                   * exist yet.
>>                   */
>> -                if (!disk->script) {
>> +                if (!disk->script &&
>> +                    disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
>>                      int major, minor;
>>                      libxl__device_physdisk_major_minor(dev,
&major, &minor);
>>                      flexarray_append_pair(back,
"physical-device",
> 
> 

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

On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne
wrote:
> Prevent hotplug script execution from libxl if device is on a
> different domain.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>



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

On Fri, 2013-05-03 at 12:23 +0100, Roger Pau Monne
wrote:
> This will be handled by the driver domain itself, since the toolstack
> does not have access to the physical device because it is in a
> different domain.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@@citrix.com>




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

On Fri, May 3, 2013 at 12:48 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> On 05/03/2013 12:23 PM, Roger Pau Monne wrote:
>>
>> Don''t try to check physical devices if they belong to a domain
>> different than the one where the toolstack is running. This prevents
>> the following error when trying to use storage driver domains:
>>
>> libxl: debug: libxl_create.c:1246:do_domain_create: ao 0x1819240:
create:
>> how=(nil) callback=(nil) poller=0x1818fa0
>> libxl: debug: libxl_device.c:235:libxl__device_disk_set_backend: Disk
>> vdev=xvda spec.backend=phy
>> libxl: debug: libxl_device.c:175:disk_try_backend: Disk vdev=xvda,
backend
>> phy unsuitable as phys path not a block device
>> libxl: error: libxl_device.c:278:libxl__device_disk_set_backend: no
>> suitable backend for disk xvda
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Ian Jackson <ian.jackson@citrix.com>
>> Cc: George Dunlap <george.dunlap@citrix.com>
>
>
> Re the feature freeze, these look like fixes to me;
>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Sorry, I thought I was replying to #0 -- the whole series gets a
"feature freeze exception" ACK.

But *not* a "freeze for RC0" exception I don''t think -- it
will have
to wait until Tuesday.

 -George

On 03/05/13 13:23, Roger Pau Monne wrote:
> Minor fixes in order to use driver domains for block devices (phy 
> backends). I''m CCing George in order to see if this is suitable
for
> inclusion into 4.3. IMHO I think this should be considered as a 
> bug-fix, that allows the storage driver domain features already 
> present in libxl to be usable for users. I''ve also created a wiki
page
> that shows how to create and use storage driver domains with xl:
> 
> http://wiki.xen.org/wiki/Storage_driver_domains
Since the tree is open again, could this patches be applied?

Thanks, Roger.

Roger Pau Monne writes ("[PATCH 1/3] libxl: correctly parse storage devices
on driver domains"):
> Don''t try to check physical devices if they belong to a domain
> different than the one where the toolstack is running. This prevents
> the following error when trying to use storage driver domains:
All three look good to me:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

George, are they OK for 4.3 ?

Ian.

On 07/05/13 18:28, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 1/3] libxl: correctly parse storage
devices on driver domains"):
>> Don''t try to check physical devices if they belong to a domain
>> different than the one where the toolstack is running. This prevents
>> the following error when trying to use storage driver domains:
> All three look good to me:
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> George, are they OK for 4.3 ?
Yes, I gave the series an "OK for 4.3 but not for the RC mini-freeze" 
ack on Friday.

  -George

On Wed, 2013-05-08 at 09:47 +0100, George Dunlap wrote:
> On 07/05/13 18:28, Ian Jackson wrote:
> > Roger Pau Monne writes ("[PATCH 1/3] libxl: correctly parse
storage devices on driver domains"):
> >> Don''t try to check physical devices if they belong to a
domain
> >> different than the one where the toolstack is running. This
prevents
> >> the following error when trying to use storage driver domains:
> > All three look good to me:
> >
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> >
> > George, are they OK for 4.3 ?
> 
> Yes, I gave the series an "OK for 4.3 but not for the RC
mini-freeze"
> ack on Friday.
Applied.