Xen devel - Apr 2013 - [PATCH] libxl: ignore run_hotplug_scripts setting when backend_domid!=0

Instead of forcing the user to change run_hotplug_scripts setting
globally, just ignore it for backends outside of dom0.

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 docs/man/xl.conf.pod.5                      |  2 ++
 docs/misc/xl-network-configuration.markdown |  4 +---
 tools/libxl/libxl.c                         | 14 --------------
 tools/libxl/libxl_device.c                  | 10 ++++++++++
 tools/libxl/xl_cmdimpl.c                    |  7 -------
 5 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 7b9fcac..c51ab39 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -60,6 +60,8 @@ Default: C<1>
 If disabled hotplug scripts will be called from udev, as it used to
 be in the previous releases. With the default option, hotplug scripts
 will be launched by xl directly.
+This option applies only to backends in dom0. For backends in other domains
+scripts are always called from udev.
 
 Default: C<1>
 
diff --git a/docs/misc/xl-network-configuration.markdown
b/docs/misc/xl-network-configuration.markdown
index e0d3d2a..9c13532 100644
--- a/docs/misc/xl-network-configuration.markdown
+++ b/docs/misc/xl-network-configuration.markdown
@@ -131,9 +131,7 @@ specified IP address to be used by the guest (blocking all
others).
 ### backend
 
 Specifies the backend domain which this device should attach to. This
-defaults to domain 0. This option does not work if `run_hotplug_scripts`
-is not disabled in xl.conf (see xl.conf(5) man page for more information
-on this option). Specifying another domain requires setting up a driver
+defaults to domain 0. Specifying another domain requires setting up a driver
 domain which is outside the scope of this document.
 
 ### rate
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 572c2c6..a813a88 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2738,8 +2738,6 @@ out:
 int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
                                  uint32_t domid)
 {
-    int run_hotplug_scripts;
-
     if (!nic->mtu)
         nic->mtu = 1492;
     if (!nic->model) {
@@ -2769,18 +2767,6 @@ int libxl__device_nic_setdefault(libxl__gc *gc,
libxl_device_nic *nic,
                                   libxl__xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
 
-    run_hotplug_scripts = libxl__hotplug_settings(gc, XBT_NULL);
-    if (run_hotplug_scripts < 0) {
-        LOG(ERROR, "unable to get current hotplug scripts execution
setting");
-        return run_hotplug_scripts;
-    }
-    if (nic->backend_domid != LIBXL_TOOLSTACK_DOMID &&
run_hotplug_scripts) {
-        LOG(ERROR, "cannot use a backend domain different than %d if"
-                   "hotplug scripts are executed from libxl",
-                   LIBXL_TOOLSTACK_DOMID);
-        return ERROR_FAIL;
-    }
-
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
         if (!nic->nictype)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index eeea9d9..d622826 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -888,6 +888,16 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
     int hotplug;
     pid_t pid;
 
+
+    /* Do not try to execute hotplug scripts for backends in driver domain,
+     * leave this task to udev - regardless of run_hotplug_scripts setting */
+    if (aodev->dev->backend_domid != LIBXL_TOOLSTACK_DOMID) {
+        LOG(INFO, "Not calling hotplug script for backend outside "
+                "of dom0 (domid=%d backend_domid=%d)",
aodev->dev->domid,
+                aodev->dev->backend_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,
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2d40f8f..8364dba 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1204,13 +1204,6 @@ static void parse_config_data(const char *config_source,
                         fprintf(stderr, "Specified backend domain does not
exist, defaulting to Dom0\n");
                         nic->backend_domid = 0;
                     }
-                    if (nic->backend_domid != 0 &&
run_hotplug_scripts) {
-                        fprintf(stderr, "ERROR: the vif
''backend='' option "
-                                "cannot be used in conjunction with "
-                                "run_hotplug_scripts, please set "
-                                "run_hotplug_scripts to 0 in
xl.conf\n");
-                        exit(EXIT_FAILURE);
-                    }
                 } else if (!strcmp(p, "rate")) {
                     parse_vif_rate(&config, (p2 + 1), nic);
                 } else if (!strcmp(p, "accel")) {
-- 
1.8.1.4

On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
<marmarek@invisiblethingslab.com> wrote:
> Instead of forcing the user to change run_hotplug_scripts setting
> globally, just ignore it for backends outside of dom0.
>
> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
This seems like a good idea -- Roger / Ian, any thoughts?

 -George

On 04/07/13 12:42, George Dunlap wrote:
> On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
> <marmarek@invisiblethingslab.com> wrote:
>> Instead of forcing the user to change run_hotplug_scripts setting
>> globally, just ignore it for backends outside of dom0.
>>
>> Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
> 
> This seems like a good idea -- Roger / Ian, any thoughts?
We already have this behaviour in libxl (see libxl_device.c:device_hotplug):

/*
 * 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;

This was introduced by commit 05bfd984dfe7014f1f5ea1133608b9bab589c120

On 04/07/13 12:39, Roger Pau Monné wrote:
> On 04/07/13 12:42, George Dunlap wrote:
>> On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
>> <marmarek@invisiblethingslab.com> wrote:
>>> Instead of forcing the user to change run_hotplug_scripts setting
>>> globally, just ignore it for backends outside of dom0.
>>>
>>> Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
>> This seems like a good idea -- Roger / Ian, any thoughts?
> We already have this behaviour in libxl (see
libxl_device.c:device_hotplug):
>
> /*
>   * 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;
>
> This was introduced by commit 05bfd984dfe7014f1f5ea1133608b9bab589c120
But you still get an error on domain creation if you actually set a 
non-dom0 backend and run_hotplug_scripts is set; so you can''t actually 
run in "mixed mode" anyway.  This patch should, I think, enable you to
have libxl run the scripts in dom0, but udev run them in domu.

  -George

On 04/07/13 14:40, George Dunlap wrote:
> On 04/07/13 12:39, Roger Pau Monné wrote:
>> On 04/07/13 12:42, George Dunlap wrote:
>>> On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
>>> <marmarek@invisiblethingslab.com> wrote:
>>>> Instead of forcing the user to change run_hotplug_scripts
setting
>>>> globally, just ignore it for backends outside of dom0.
>>>>
>>>> Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
>>> This seems like a good idea -- Roger / Ian, any thoughts?
>> We already have this behaviour in libxl (see
>> libxl_device.c:device_hotplug):
>>
>> /*
>>   * 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;
>>
>> This was introduced by commit 05bfd984dfe7014f1f5ea1133608b9bab589c120
> 
> But you still get an error on domain creation if you actually set a
> non-dom0 backend and run_hotplug_scripts is set; so you can''t
actually
> run in "mixed mode" anyway.  This patch should, I think, enable
you to
> have libxl run the scripts in dom0, but udev run them in domu.
Not for block devices that run in driver domains, but that might be true
for network devices (I have not checked in libxl if the flow of network
devices will error at some point if backend != 0).

For block devices you just need to follow
http://wiki.xen.org/wiki/Storage_driver_domains, there''s no need to
perform any kind of obscure tricks to get it running.

On Thu, Jul 04, 2013 at 03:23:51PM +0200, Roger Pau Monné
wrote:
> On 04/07/13 14:40, George Dunlap wrote:
> > On 04/07/13 12:39, Roger Pau Monné wrote:
> >> On 04/07/13 12:42, George Dunlap wrote:
> >>> On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
> >>> <marmarek@invisiblethingslab.com> wrote:
> >>>> Instead of forcing the user to change run_hotplug_scripts
setting
> >>>> globally, just ignore it for backends outside of dom0.
> >>>>
> >>>> Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
> >>> This seems like a good idea -- Roger / Ian, any thoughts?
> >> We already have this behaviour in libxl (see
> >> libxl_device.c:device_hotplug):
> >>
> >> /*
> >>   * 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;
> >>
> >> This was introduced by commit
05bfd984dfe7014f1f5ea1133608b9bab589c120
> > 
What Marek''s patch does is effectively Roger''s commit 05bfd984
plus some
network specific bits. N.B. this patch predates 05bfd984.
> > But you still get an error on domain creation if you actually set a
> > non-dom0 backend and run_hotplug_scripts is set; so you can''t
actually
> > run in "mixed mode" anyway.  This patch should, I think,
enable you to
> > have libxl run the scripts in dom0, but udev run them in domu.
> 
Long time ago Konrad sent me an email about running network backend in
another domain. The first thing to do is to disable hotplug script
globally.

Korand has some experience here and I think this patch *should* work,
because it basically does what Konrad told me to do.


Wei.
> Not for block devices that run in driver domains, but that might be true
> for network devices (I have not checked in libxl if the flow of network
> devices will error at some point if backend != 0).
> 
> For block devices you just need to follow
> http://wiki.xen.org/wiki/Storage_driver_domains, there''s no need
to
> perform any kind of obscure tricks to get it running.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On 04/07/13 14:51, Wei Liu wrote:
> On Thu, Jul 04, 2013 at 03:23:51PM +0200, Roger Pau Monné wrote:
>> On 04/07/13 14:40, George Dunlap wrote:
>>> On 04/07/13 12:39, Roger Pau Monné wrote:
>>>> On 04/07/13 12:42, George Dunlap wrote:
>>>>> On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
>>>>> <marmarek@invisiblethingslab.com> wrote:
>>>>>> Instead of forcing the user to change
run_hotplug_scripts setting
>>>>>> globally, just ignore it for backends outside of dom0.
>>>>>>
>>>>>> Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
>>>>> This seems like a good idea -- Roger / Ian, any thoughts?
>>>> We already have this behaviour in libxl (see
>>>> libxl_device.c:device_hotplug):
>>>>
>>>> /*
>>>>    * 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;
>>>>
>>>> This was introduced by commit
05bfd984dfe7014f1f5ea1133608b9bab589c120
> What Marek''s patch does is effectively Roger''s commit
05bfd984 plus some
> network specific bits. N.B. this patch predates 05bfd984.
>
>>> But you still get an error on domain creation if you actually set a
>>> non-dom0 backend and run_hotplug_scripts is set; so you
can''t actually
>>> run in "mixed mode" anyway.  This patch should, I think,
enable you to
>>> have libxl run the scripts in dom0, but udev run them in domu.
> Long time ago Konrad sent me an email about running network backend in
> another domain. The first thing to do is to disable hotplug script
> globally.
Sure, that''s easy.  The problem is, there''s no reason to
disable it
globally -- you can run udev in the domnet, but have libxl run the 
scripts in dom0.

I''ll try just taking out that check and seeing what happens...

  -George

On Thu, Jul 04, 2013 at 03:05:54PM +0100, George Dunlap
wrote:
> On 04/07/13 14:51, Wei Liu wrote:
> >On Thu, Jul 04, 2013 at 03:23:51PM +0200, Roger Pau Monné wrote:
> >>On 04/07/13 14:40, George Dunlap wrote:
> >>>On 04/07/13 12:39, Roger Pau Monné wrote:
> >>>>On 04/07/13 12:42, George Dunlap wrote:
> >>>>>On Tue, Apr 9, 2013 at 9:49 AM, Marek Marczykowski
> >>>>><marmarek@invisiblethingslab.com> wrote:
> >>>>>>Instead of forcing the user to change
run_hotplug_scripts setting
> >>>>>>globally, just ignore it for backends outside of
dom0.
> >>>>>>
> >>>>>>Signed-off-by: Marek Marczykowski
<marmarek@invisiblethingslab.com>
> >>>>>This seems like a good idea -- Roger / Ian, any
thoughts?
> >>>>We already have this behaviour in libxl (see
> >>>>libxl_device.c:device_hotplug):
> >>>>
> >>>>/*
> >>>>   * 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;
> >>>>
> >>>>This was introduced by commit
05bfd984dfe7014f1f5ea1133608b9bab589c120
> >What Marek''s patch does is effectively Roger''s commit
05bfd984 plus some
> >network specific bits. N.B. this patch predates 05bfd984.
> >
> >>>But you still get an error on domain creation if you actually
set a
> >>>non-dom0 backend and run_hotplug_scripts is set; so you
can''t actually
> >>>run in "mixed mode" anyway.  This patch should, I
think, enable you to
> >>>have libxl run the scripts in dom0, but udev run them in domu.
> >Long time ago Konrad sent me an email about running network backend in
> >another domain. The first thing to do is to disable hotplug script
> >globally.
> 
> Sure, that''s easy.  The problem is, there''s no reason to
disable it
> globally -- you can run udev in the domnet, but have libxl run the
> scripts in dom0.
> 
> I''ll try just taking out that check and seeing what happens...
> 
I''m not suggesting we disable it globally. I''m suggesting
disabling
hotplug script makes domnet work (whether globally or locally).


Wei.
>  -George