Xen devel - Sep 2012 - [PATCH v2 1/2] libxl: fix usage of backend parameter and run_hotplug_scripts

vif interfaces allows the user to specify the domain that should run
the backend (also known as driver domain) using the ''backend''
parameter. This is not compatible with run_hotplug_scripts=1, since
libxl can only run the hotplug scripts from the Domain 0.

Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
Changes since v1:

 * Remove references to xl.conf, since it''s a layering violation.

 * Move the docs update to next patch (that includes xl changes).
---
 tools/libxl/libxl.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8ea3478..47b1fb9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2474,6 +2474,8 @@ 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) {
@@ -2503,6 +2505,18 @@ 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)
-- 
1.7.7.5 (Apple Git-26)

Print an error and exit if backend!=0 is used in conjunction with
run_hotplug_scripts. Currently libxl can only execute hotplug scripts
from the toolstack domain (the same domain xl is running from).

Added a description and workaround of this issue on
xl-network-configuration.

Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 docs/misc/xl-network-configuration.markdown |    6 ++++--
 tools/libxl/xl_cmdimpl.c                    |    7 +++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xl-network-configuration.markdown
b/docs/misc/xl-network-configuration.markdown
index 650926c..5e2f049 100644
--- a/docs/misc/xl-network-configuration.markdown
+++ b/docs/misc/xl-network-configuration.markdown
@@ -122,8 +122,10 @@ 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. Specifying another domain requires setting up a
-driver domain which is outside the scope of this document.
+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
+domain which is outside the scope of this document.
 
 ### rate
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 138cd72..02b9ce8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1000,6 +1000,13 @@ 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.7.7.5 (Apple Git-26)

On Tue, 2012-09-04 at 16:16 +0100, Roger Pau Monne
wrote:
> vif interfaces allows the user to specify the domain that should run
> the backend (also known as driver domain) using the
''backend''
> parameter. This is not compatible with run_hotplug_scripts=1, since
> libxl can only run the hotplug scripts from the Domain 0.
Sorry but I think in the end this has missed the cut-off for 4.2.0 (we
are cutting the final RC tomorrow).

We should be branching soon after the final RC which means 4.3
development will open in unstable shortly. We should take this into
unstable then and consider it for 4.2.1.

In the meantime I suppose we should mention this in the release notes.

Ian Campbell wrote:
> On Tue, 2012-09-04 at 16:16 +0100, Roger Pau Monne wrote:
>> vif interfaces allows the user to specify the domain that should run
>> the backend (also known as driver domain) using the
''backend''
>> parameter. This is not compatible with run_hotplug_scripts=1, since
>> libxl can only run the hotplug scripts from the Domain 0.
> 
> Sorry but I think in the end this has missed the cut-off for 4.2.0 (we
> are cutting the final RC tomorrow).
> 
> We should be branching soon after the final RC which means 4.3
> development will open in unstable shortly. We should take this into
> unstable then and consider it for 4.2.1.
> 
> In the meantime I suppose we should mention this in the release notes.
Do you think we could commit the docs change
(xl-network-configuration.markdown) only? So people can know that this
won''t work in current release.

Sorry for the delay.

On Thu, 2012-09-06 at 09:30 +0100, Roger Pau Monne
wrote:
> Ian Campbell wrote:
> > On Tue, 2012-09-04 at 16:16 +0100, Roger Pau Monne wrote:
> >> vif interfaces allows the user to specify the domain that should
run
> >> the backend (also known as driver domain) using the
''backend''
> >> parameter. This is not compatible with run_hotplug_scripts=1,
since
> >> libxl can only run the hotplug scripts from the Domain 0.
> > 
> > Sorry but I think in the end this has missed the cut-off for 4.2.0 (we
> > are cutting the final RC tomorrow).
> > 
> > We should be branching soon after the final RC which means 4.3
> > development will open in unstable shortly. We should take this into
> > unstable then and consider it for 4.2.1.
> > 
> > In the meantime I suppose we should mention this in the release notes.
> 
> Do you think we could commit the docs change
> (xl-network-configuration.markdown) only? So people can know that this
> won''t work in current release.
I think we can take some docs only patches after RC4, I don''t want to
taker any patches before then since we need a test pass before we tag it
tomorrow.
> Sorry for the delay.
S''OK.

Ian.

On Tue, 2012-09-04 at 16:16 +0100, Roger Pau Monne
wrote:
> vif interfaces allows the user to specify the domain that should run
> the backend (also known as driver domain) using the
''backend''
> parameter. This is not compatible with run_hotplug_scripts=1, since
> libxl can only run the hotplug scripts from the Domain 0.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
Acked + applied, thanks.
> ---
> Changes since v1:
> 
>  * Remove references to xl.conf, since it''s a layering violation.
> 
>  * Move the docs update to next patch (that includes xl changes).
> ---
>  tools/libxl/libxl.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 8ea3478..47b1fb9 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2474,6 +2474,8 @@ 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) {
> @@ -2503,6 +2505,18 @@ 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)

On Tue, 2012-09-04 at 16:16 +0100, Roger Pau Monne
wrote:
> Print an error and exit if backend!=0 is used in conjunction with
> run_hotplug_scripts. Currently libxl can only execute hotplug scripts
> from the toolstack domain (the same domain xl is running from).
> 
> Added a description and workaround of this issue on
> xl-network-configuration.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
Acked + applied, thanks.