[PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set

[PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set

[George Dunlap]

As of commit 05bfd984dfe7014f1f5ea1133608b9bab589c120, hotplug scripts
are not run if backend_domid != LIBXL_TOOSTACK_DOMID; so there is no reason
to restrict this for network driver domains any more.

This is a candidate for backporting to 4.3.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 docs/misc/xl-network-configuration.markdown |    6 ++----
 tools/libxl/libxl.c                         |    7 -------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/docs/misc/xl-network-configuration.markdown b/docs/misc/xl-network-configuration.markdown
index e0d3d2a..3c439d4 100644
--- a/docs/misc/xl-network-configuration.markdown
+++ b/docs/misc/xl-network-configuration.markdown
@@ -131,10 +131,8 @@ 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
-domain which is outside the scope of this document.
+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 bd63a30..81785df 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2820,13 +2820,6 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
     rc = libxl__resolve_domid(gc, nic->backend_domname, &nic->backend_domid);
     if (rc < 0) return rc;
 
-    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.9.5

[PATCH 2/2] libxl: Add vif.default.backend to xl.conf

[George Dunlap]

This will allow a user to default to a network driver domain
system-wide.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
---
 docs/man/xl.conf.pod.5   |    6 ++++++
 tools/examples/xl.conf   |    4 ++++
 tools/libxl/xl.c         |    5 +++++
 tools/libxl/xl.h         |    1 +
 tools/libxl/xl_cmdimpl.c |    6 ++++++
 5 files changed, 22 insertions(+)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 1229c8a..d8d1bb1 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -93,6 +93,12 @@ The old B<defaultbridge> option is deprecated and should not be used.
 
 Default: C<xenbr0>
 
+=item B<vif.default.backend="NAME">
+
+Configures the default backend to set for virtual network devices.
+
+Default: C<0>
+
 =item B<vif.default.gatewaydev="NAME">
 
 Configures the default gateway device to set for virtual network devices.
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 9c037a6..7b081d2 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -19,6 +19,10 @@
 # launched by udev.
 #run_hotplug_scripts=1
 
+# default backend domain to connect guest vifs to.  This can be any
+# valid domain identifier.
+#vif.default.backend="0"
+
 # default gateway device to use with vif-route hotplug script
 #vif.default.gatewaydev="eth0"
 
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 1ce820c..c694e8f 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -45,6 +45,7 @@ char *lockfile;
 char *default_vifscript = NULL;
 char *default_bridge = NULL;
 char *default_gatewaydev = NULL;
+char *default_vifbackend = NULL;
 enum output_format default_output_format = OUTPUT_FORMAT_JSON;
 int claim_mode = 0;
 
@@ -158,6 +159,10 @@ static void parse_global_config(const char *configfile,
     if (!xlu_cfg_get_string (config, "vif.default.gatewaydev", &buf, 0))
         default_gatewaydev = strdup(buf);
 
+
+    if (!xlu_cfg_get_string (config, "vif.default.backend", &buf, 0))
+        default_vifbackend = strdup(buf);
+
     if (!xlu_cfg_get_string (config, "output_format", &buf, 0)) {
         if (!strcmp(buf, "json"))
             default_output_format = OUTPUT_FORMAT_JSON;
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5ad3e17..e72a7d2 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -151,6 +151,7 @@ extern char *lockfile;
 extern char *default_vifscript;
 extern char *default_bridge;
 extern char *default_gatewaydev;
+extern char *default_vifbackend;
 extern char *blkdev_start;
 
 enum output_format {
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8a478ba..f670c6c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1125,6 +1125,11 @@ static void parse_config_data(const char *config_source,
                 nic->gatewaydev = strdup(default_gatewaydev);
             }
 
+            if (default_vifbackend) {
+                free(nic->backend_domname);
+                nic->backend_domname = strdup(default_vifbackend);
+            }
+
             p = strtok(buf2, ",");
             if (!p)
                 goto skip_nic;
@@ -1174,6 +1179,7 @@ static void parse_config_data(const char *config_source,
                     free(nic->ifname);
                     nic->ifname = strdup(p2 + 1);
                 } else if (!strcmp(p, "backend")) {
+                    free(nic->backend_domname);
                     nic->backend_domname = strdup(p2 + 1);
                 } else if (!strcmp(p, "rate")) {
                     parse_vif_rate(&config, (p2 + 1), nic);
-- 
1.7.9.5

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set

[Roger Pau Monné]

On 05/07/13 13:13, George Dunlap wrote:
> As of commit 05bfd984dfe7014f1f5ea1133608b9bab589c120, hotplug scripts
> are not run if backend_domid != LIBXL_TOOSTACK_DOMID; so there is no reason
> to restrict this for network driver domains any more.
> 
> This is a candidate for backporting to 4.3.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Re: [PATCH 2/2] libxl: Add vif.default.backend to xl.conf

[Roger Pau Monné]

On 05/07/13 13:13, George Dunlap wrote:
> This will allow a user to default to a network driver domain
> system-wide.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

(Just one nit below)

> ---
>  docs/man/xl.conf.pod.5   |    6 ++++++
>  tools/examples/xl.conf   |    4 ++++
>  tools/libxl/xl.c         |    5 +++++
>  tools/libxl/xl.h         |    1 +
>  tools/libxl/xl_cmdimpl.c |    6 ++++++
>  5 files changed, 22 insertions(+)
> 
> diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
> index 1229c8a..d8d1bb1 100644
> --- a/docs/man/xl.conf.pod.5
> +++ b/docs/man/xl.conf.pod.5
> @@ -93,6 +93,12 @@ The old B<defaultbridge> option is deprecated and should not be used.
>  
>  Default: C<xenbr0>
>  
> +=item B<vif.default.backend="NAME">
> +
> +Configures the default backend to set for virtual network devices.
> +
> +Default: C<0>
> +
>  =item B<vif.default.gatewaydev="NAME">
>  
>  Configures the default gateway device to set for virtual network devices.
> diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
> index 9c037a6..7b081d2 100644
> --- a/tools/examples/xl.conf
> +++ b/tools/examples/xl.conf
> @@ -19,6 +19,10 @@
>  # launched by udev.
>  #run_hotplug_scripts=1
>  
> +# default backend domain to connect guest vifs to.  This can be any
> +# valid domain identifier.
> +#vif.default.backend="0"
> +
>  # default gateway device to use with vif-route hotplug script
>  #vif.default.gatewaydev="eth0"
>  
> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
> index 1ce820c..c694e8f 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -45,6 +45,7 @@ char *lockfile;
>  char *default_vifscript = NULL;
>  char *default_bridge = NULL;
>  char *default_gatewaydev = NULL;
> +char *default_vifbackend = NULL;
>  enum output_format default_output_format = OUTPUT_FORMAT_JSON;
>  int claim_mode = 0;
>  
> @@ -158,6 +159,10 @@ static void parse_global_config(const char *configfile,
>      if (!xlu_cfg_get_string (config, "vif.default.gatewaydev", &buf, 0))
>          default_gatewaydev = strdup(buf);
>  
> +

There's an extra new line here.

> +    if (!xlu_cfg_get_string (config, "vif.default.backend", &buf, 0))
> +        default_vifbackend = strdup(buf);
> +
>      if (!xlu_cfg_get_string (config, "output_format", &buf, 0)) {
>          if (!strcmp(buf, "json"))
>              default_output_format = OUTPUT_FORMAT_JSON;
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 5ad3e17..e72a7d2 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -151,6 +151,7 @@ extern char *lockfile;
>  extern char *default_vifscript;
>  extern char *default_bridge;
>  extern char *default_gatewaydev;
> +extern char *default_vifbackend;
>  extern char *blkdev_start;
>  
>  enum output_format {
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 8a478ba..f670c6c 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1125,6 +1125,11 @@ static void parse_config_data(const char *config_source,
>                  nic->gatewaydev = strdup(default_gatewaydev);
>              }
>  
> +            if (default_vifbackend) {
> +                free(nic->backend_domname);
> +                nic->backend_domname = strdup(default_vifbackend);
> +            }
> +
>              p = strtok(buf2, ",");
>              if (!p)
>                  goto skip_nic;
> @@ -1174,6 +1179,7 @@ static void parse_config_data(const char *config_source,
>                      free(nic->ifname);
>                      nic->ifname = strdup(p2 + 1);
>                  } else if (!strcmp(p, "backend")) {
> +                    free(nic->backend_domname);
>                      nic->backend_domname = strdup(p2 + 1);
>                  } else if (!strcmp(p, "rate")) {
>                      parse_vif_rate(&config, (p2 + 1), nic);
> 

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set

[George Dunlap]

Ping?

On Fri, Jul 5, 2013 at 12:13 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> As of commit 05bfd984dfe7014f1f5ea1133608b9bab589c120, hotplug scripts
> are not run if backend_domid != LIBXL_TOOSTACK_DOMID; so there is no reason
> to restrict this for network driver domains any more.
>
> This is a candidate for backporting to 4.3.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> ---
>  docs/misc/xl-network-configuration.markdown |    6 ++----
>  tools/libxl/libxl.c                         |    7 -------
>  2 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/docs/misc/xl-network-configuration.markdown b/docs/misc/xl-network-configuration.markdown
> index e0d3d2a..3c439d4 100644
> --- a/docs/misc/xl-network-configuration.markdown
> +++ b/docs/misc/xl-network-configuration.markdown
> @@ -131,10 +131,8 @@ 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
> -domain which is outside the scope of this document.
> +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 bd63a30..81785df 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2820,13 +2820,6 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
>      rc = libxl__resolve_domid(gc, nic->backend_domname, &nic->backend_domid);
>      if (rc < 0) return rc;
>
> -    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.9.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]

[Ian Jackson]

George Dunlap writes ("[PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set"):
> As of commit 05bfd984dfe7014f1f5ea1133608b9bab589c120, hotplug scripts
> are not run if backend_domid != LIBXL_TOOSTACK_DOMID; so there is no reason
> to restrict this for network driver domains any more.

Applied, thanks.

> This is a candidate for backporting to 4.3.

As usual, we should consider this after it's been in master for a bit.

Ian.

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]

[George Dunlap]

On 17/07/13 12:19, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set"):
>> As of commit 05bfd984dfe7014f1f5ea1133608b9bab589c120, hotplug scripts
>> are not run if backend_domid != LIBXL_TOOSTACK_DOMID; so there is no reason
>> to restrict this for network driver domains any more.
> Applied, thanks.
>
>> This is a candidate for backporting to 4.3.
> As usual, we should consider this after it's been in master for a bit.

What is meant to trigger that consideration?  Do you have a list 
somewhere, or are you relying on the submitter to ping after a certain 
amount of time?

I obviously prefer the first, so I can cross this off my list entirely...

  -George

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]

[Ian Jackson]

George Dunlap writes ("Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]"):
> What is meant to trigger that consideration?  Do you have a list 
> somewhere, or are you relying on the submitter to ping after a certain 
> amount of time?
> 
> I obviously prefer the first, so I can cross this off my list entirely...

I don't keep such a list.  I probably should.

...

^P^P^P^K

I keep such a list and I have put this commit on it.

Ian.

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]

[George Dunlap]

On 18/07/13 12:20, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]"):
>> What is meant to trigger that consideration?  Do you have a list
>> somewhere, or are you relying on the submitter to ping after a certain
>> amount of time?
>>
>> I obviously prefer the first, so I can cross this off my list entirely...
> I don't keep such a list.  I probably should.
>
> ...
>
> ^P^P^P^K
>
> I keep such a list and I have put this commit on it.

:-)

Re: [PATCH 2/2] libxl: Add vif.default.backend to xl.conf

[Ian Campbell]

On Fri, 2013-07-05 at 13:47 +0200, Roger Pau Monné wrote:
> On 05/07/13 13:13, George Dunlap wrote:
> > This will allow a user to default to a network driver domain
> > system-wide.
> > 
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Ian Jackson <ian.jackson@citrix.com>
> > CC: Roger Pau Monne <roger.pau@citrix.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> (Just one nit below)

Acked + applied (with nit fixed, which I just realised I forgot to note
in the commit log, sorry)

Ian.


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

Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]

[Ian Jackson]

George Dunlap writes ("Re: [PATCH 1/2] libxl: Allow network driver domains when run_hotplug_scritps is set [and 1 more messages]"):
> On 18/07/13 12:20, Ian Jackson wrote:
...
> > I keep such a list and I have put this commit on it.
> 
> :-)

I have cherry picked this onto 4.3.

It didn't apply cleanly to 4.2, and the commit 05bfd984dfe7 which it
depends on doesn't appear to be in 4.2 either.

Thanks,
Ian.