Support for vif-route was missing in libxl, this patch series adds a gatewaydev/netdev vif option and also allows setting a default gatewaydev in xl.conf global configuration file. vif-route doesn''t support HVM domains because it doesn''t support the "add" or "remove" actions. I would like to request some testing from people that actually use this network mode.
Roger Pau Monne
2013-Feb-06 18:04 UTC
[PATCH v2 1/4] xl/libxl: add gatewaydev/netdev to vif specification
This option is used by the vif-route hotplug script. A new more
descriptive name is used, "gatewaydev", but "netdev" is also
supported
as a deprecated backwards compatible option.
This option was supported in the past, according to
http://wiki.xen.org/wiki/Vif-route, so we should also support it in
libxl.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ulf Kreutzberg <ulf.kreutzberg@hosteurope.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
Changes since v1:
* Don't pass a NULL as a value of a env variable
* Change netdev to gatewaydev
---
docs/misc/xl-network-configuration.markdown | 10 ++++++++++
tools/libxl/libxl.c | 6 +++++-
tools/libxl/libxl_linux.c | 9 ++++++++-
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 14 ++++++++++++++
5 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/docs/misc/xl-network-configuration.markdown
b/docs/misc/xl-network-configuration.markdown
index 5e2f049..e0d3d2a 100644
--- a/docs/misc/xl-network-configuration.markdown
+++ b/docs/misc/xl-network-configuration.markdown
@@ -67,6 +67,15 @@ added to. The default is `xenbr0`. The bridge must be
configured using
your distribution's network configuration tools. See the [wiki][net]
for guidance and examples.
+### gatewaydev
+
+Specifies the name of the network interface which has an IP and which
+is in the network the VIF should communicate with. This is used in the host
+by the vif-route hotplug script. See [wiki][vifroute] for guidance and
+examples.
+
+NOTE: netdev is a deprecated alias of this option.
+
### type
This keyword is valid for HVM guests only.
@@ -158,3 +167,4 @@ on the underlying netback implementation.
[oui]: http://en.wikipedia.org/wiki/Organizationally_Unique_Identifier
[net]: http://wiki.xen.org/wiki/HostConfiguration/Networking
+[vifroute]: http://wiki.xen.org/wiki/Vif-route
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 73e0dc3..5d590f1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2826,7 +2826,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
if (rc) goto out;
front = flexarray_make(gc, 16, 1);
- back = flexarray_make(gc, 16, 1);
+ back = flexarray_make(gc, 18, 1);
if (nic->devid == -1) {
if ((nic->devid = libxl__device_nextid(gc, domid, "vif")
< 0)) {
@@ -2862,6 +2862,10 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t
domid,
flexarray_append(back, "ip");
flexarray_append(back, libxl__strdup(gc, nic->ip));
}
+ if (nic->gatewaydev) {
+ flexarray_append(back, "gatewaydev");
+ flexarray_append(back, libxl__strdup(gc, nic->gatewaydev));
+ }
if (nic->rate_interval_usecs > 0) {
flexarray_append(back, "rate");
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 1fed3cd..60fc533 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -84,11 +84,16 @@ static char **get_hotplug_env(libxl__gc *gc,
char *script, libxl__device *dev)
{
const char *type = libxl__device_kind_to_string(dev->backend_kind);
+ char *be_path = libxl__device_backend_path(gc, dev);
char **env;
+ char *gatewaydev;
int nr = 0;
libxl_nic_type nictype;
- const int arraysize = 13;
+ gatewaydev = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s",
be_path,
+
"gatewaydev"));
+
+ const int arraysize = 15;
GCNEW_ARRAY(env, arraysize);
env[nr++] = "script";
env[nr++] = script;
@@ -98,6 +103,8 @@ static char **get_hotplug_env(libxl__gc *gc,
env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid,
dev->devid);
env[nr++] = "XENBUS_BASE_PATH";
env[nr++] = "backend";
+ env[nr++] = "netdev";
+ env[nr++] = gatewaydev ? : "";
if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) {
if (libxl__nic_type(gc, dev, &nictype)) {
LOG(ERROR, "unable to get nictype");
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index acc4bc9..0112a7a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -382,6 +382,7 @@ libxl_device_nic = Struct("device_nic", [
("nictype", libxl_nic_type),
("rate_bytes_per_interval", uint64),
("rate_interval_usecs", uint32),
+ ("gatewaydev", string),
])
libxl_device_pci = Struct("device_pci", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 080bbd8..dc1788e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1205,6 +1205,14 @@ static void parse_config_data(const char *config_source,
parse_vif_rate(&config, (p2 + 1), nic);
} else if (!strcmp(p, "accel")) {
fprintf(stderr, "the accel parameter for vifs is
currently not supported\n");
+ } else if (!strcmp(p, "netdev")) {
+ fprintf(stderr, "the netdev parameter is deprecated,
"
+ "please use gatewaydev
instead\n");
+ free(nic->gatewaydev);
+ nic->gatewaydev = strdup(p2 + 1);
+ } else if (!strcmp(p, "gatewaydev")) {
+ free(nic->gatewaydev);
+ nic->gatewaydev = strdup(p2 + 1);
}
} while ((p = strtok(NULL, ",")) != NULL);
skip_nic:
@@ -5471,6 +5479,12 @@ int main_networkattach(int argc, char **argv)
}
} else if (MATCH_OPTION("bridge", *argv, oparg)) {
replace_string(&nic.bridge, oparg);
+ } else if (MATCH_OPTION("netdev", *argv, oparg)) {
+ fprintf(stderr, "the netdev parameter is deprecated, "
+ "please use gatewaydev instead\n");
+ replace_string(&nic.gatewaydev, oparg);
+ } else if (MATCH_OPTION("gatewaydev", *argv, oparg)) {
+ replace_string(&nic.gatewaydev, oparg);
} else if (MATCH_OPTION("ip", *argv, oparg)) {
replace_string(&nic.ip, oparg);
} else if (MATCH_OPTION("script", *argv, oparg)) {
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Roger Pau Monne
2013-Feb-06 18:04 UTC
[PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
This adds a new global option in the xl configuration file called
"vif.default.gatewaydev", that is used to specify the default
gatewaydev to use when none is passed in the vif specification.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Ulf Kreutzberg <ulf.kreutzberg@hosteurope.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
---
Changes since v1:
* Rename defaultnetdev to vif.default.gatewaydev
---
docs/man/xl.conf.pod.5 | 6 ++++++
tools/examples/xl.conf | 3 +++
tools/libxl/xl.c | 4 ++++
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 5 +++++
5 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 82c6b20..f8f7e7f 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -82,6 +82,12 @@ Configures the default bridge to set for virtual network
devices.
Default: C<xenbr0>
+=item B<vif.default.gatewaydev="NAME">
+
+Configures the default gateway device to set for virtual network devices.
+
+Default: C<None>
+
=item B<output_format="json|sxp">
Configures the default output format used by xl when printing "machine
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 28ab796..9a03fff 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -20,3 +20,6 @@
# if disabled the old behaviour will be used, and hotplug scripts will be
# launched by udev.
#run_hotplug_scripts=1
+
+# 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 ecbcd3b..f657216 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -43,6 +43,7 @@ int run_hotplug_scripts = 1;
char *lockfile;
char *default_vifscript = NULL;
char *default_bridge = NULL;
+char *default_gatewaydev = NULL;
enum output_format default_output_format = OUTPUT_FORMAT_JSON;
static xentoollog_level minmsglevel = XTL_PROGRESS;
@@ -91,6 +92,9 @@ static void parse_global_config(const char *configfile,
if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0))
default_bridge = strdup(buf);
+ if (!xlu_cfg_get_string (config, "vif.default.gatewaydev",
&buf, 0))
+ default_gatewaydev = 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 be6f38b..b881f92 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -148,6 +148,7 @@ extern int dryrun_only;
extern char *lockfile;
extern char *default_vifscript;
extern char *default_bridge;
+extern char *default_gatewaydev;
extern char *blkdev_start;
enum output_format {
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index dc1788e..7796498 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1141,6 +1141,11 @@ static void parse_config_data(const char *config_source,
nic->bridge = strdup(default_bridge);
}
+ if (default_gatewaydev) {
+ free(nic->gatewaydev);
+ nic->gatewaydev = strdup(default_gatewaydev);
+ }
+
p = strtok(buf2, ",");
if (!p)
goto skip_nic;
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
This is a replacement for defaultbridge xl.conf option. The now
deprecated defaultbridge is still supported.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
tools/examples/xl.conf | 3 +++
tools/libxl/xl.c | 13 +++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 9a03fff..4451176 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -23,3 +23,6 @@
# default gateway device to use with vif-route hotplug script
#vif.default.gatewaydev="eth0"
+
+# default bridge device to use with vif-bridge hotplug scripts
+#vif.default.bridge="bridge0"
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index f657216..100ab32 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -89,8 +89,17 @@ static void parse_global_config(const char *configfile,
if (!xlu_cfg_get_string (config, "vifscript", &buf, 0))
default_vifscript = strdup(buf);
- if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0))
- default_bridge = strdup(buf);
+ if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) {
+ fprintf(stderr, "the global config option defaultbridge is
deprecated, "
+ "please switch to vif.default.bridge\n");
+ free(default_bridge);
+ default_bridge = strdup(buf);
+ }
+
+ if (!xlu_cfg_get_string (config, "vif.default.bridge", &buf,
0)) {
+ free(default_bridge);
+ default_bridge = strdup(buf);
+ }
if (!xlu_cfg_get_string (config, "vif.default.gatewaydev",
&buf, 0))
default_gatewaydev = strdup(buf);
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Replace vifscript with vif.default.script. The old config option is
kept for backwards compatibility.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 100ab32..92565d1 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -86,8 +86,17 @@ static void parse_global_config(const char *configfile,
exit(1);
}
- if (!xlu_cfg_get_string (config, "vifscript", &buf, 0))
+ if (!xlu_cfg_get_string (config, "vifscript", &buf, 0)) {
+ fprintf(stderr, "the global config option vifscript is deprecated,
"
+ "please switch to vif.default.script\n");
+ free(default_vifscript);
default_vifscript = strdup(buf);
+ }
+
+ if (!xlu_cfg_get_string (config, "vif.default.script", &buf,
0)) {
+ free(default_vifscript);
+ default_vifscript = strdup(buf);
+ }
if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) {
fprintf(stderr, "the global config option defaultbridge is
deprecated, "
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
On 06/02/13 18:04, Roger Pau Monne wrote:> Replace vifscript with vif.default.script. The old config option is > kept for backwards compatibility. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: George Dunlap <george.dunlap@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com>I haven't done a detailed review of the code, but the naming looks fine to me. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ulf Kreutzberg
2013-Feb-14 15:09 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
Roger,
after applying patch v2 1/4 and 2/4 (are there missing some?)
to git checkout c23ea051ccee613e668b2a87817d49a28215ac8b of
git://xenbits.xen.org/xen.git xen
I get the following error(s) during compiling with ''make
tools'':
In file included from libxl.h:355,
from xl_cmdtable.c:17:
_libxl_types.h:437: error: duplicate member ‘gatewaydev’
make[3]: *** [xl_cmdtable.o] Error 1
On 06.02.2013 19:04, Roger Pau Monne wrote:> This adds a new global option in the xl configuration file called
> "vif.default.gatewaydev", that is used to specify the default
> gatewaydev to use when none is passed in the vif specification.
>
Thanks and best regards,
Ulf
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Roger Pau Monné
2013-Feb-14 15:17 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
On 14/02/13 16:09, Ulf Kreutzberg wrote:> Roger, > > > after applying patch v2 1/4 and 2/4 (are there missing some?)Well, you might want to try 3/4 and 4/4 also, but they are not mandatory.> to git checkout c23ea051ccee613e668b2a87817d49a28215ac8b of > git://xenbits.xen.org/xen.git xen > > I get the following error(s) during compiling with 'make tools': > > In file included from libxl.h:355, > from xl_cmdtable.c:17: > _libxl_types.h:437: error: duplicate member ‘gatewaydev’ > make[3]: *** [xl_cmdtable.o] Error 1Could you take a look at tools/libxl/libxl_types.idl? You should have something like the following chunk: libxl_device_nic = Struct("device_nic", [ ("backend_domid", libxl_domid), ("devid", libxl_devid), ("mtu", integer), ("model", string), ("mac", libxl_mac), ("ip", string), ("bridge", string), ("ifname", string), ("script", string), ("nictype", libxl_nic_type), ("rate_bytes_per_interval", uint64), ("rate_interval_usecs", uint32), ("gatewaydev", string), ]) (With only one "gatewaydev"). _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ulf Kreutzberg
2013-Feb-15 14:06 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
Hello Roger, hello everybody, sorry for the late reply. On 14.02.2013 16:17, Roger Pau Monné wrote:> Could you take a look at tools/libxl/libxl_types.idl? > > You should have something like the following chunk: > > libxl_device_nic = Struct("device_nic", [ > ("backend_domid", libxl_domid), > ("devid", libxl_devid), > ("mtu", integer), > ("model", string), > ("mac", libxl_mac), > ("ip", string), > ("bridge", string), > ("ifname", string), > ("script", string), > ("nictype", libxl_nic_type), > ("rate_bytes_per_interval", uint64), > ("rate_interval_usecs", uint32), > ("gatewaydev", string), > ]) > > (With only one "gatewaydev"). >That was my copy''n''paste mistake - fixed. I could see that after applying patches V1 the parameter "netdev" is parsed correctly and the main ip is passed to xen scripts. However, I could not get xen 4.3 running so I had to test 4.3 tools on 4.2.1 hypervisor, which could not start the domU, but debug output of the scripts showed me everything is parsed as expected. The mac address still is not parsed if digits are not padded with leading zero, like "mac=de:ad:a:1e:42:3" - the rest of the line is ignored without any error. However, I was not able to apply patches V2 after that, they do not seem to work together with the first patch series. Only patching with V2 resulted in missing "netdev" member in libxl_device_nic (libxl_types.idl). After editing the file and adding the definition, it compiled so far. xl with only patches V2 applied does parse neither netdev nor gatewaydev as it seems. Best regards, Ulf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monné
2013-Feb-18 07:38 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
On 15/02/13 15:06, Ulf Kreutzberg wrote:> Hello Roger, > hello everybody, > > sorry for the late reply. > > On 14.02.2013 16:17, Roger Pau Monné wrote: >> Could you take a look at tools/libxl/libxl_types.idl? >> >> You should have something like the following chunk: >> >> libxl_device_nic = Struct("device_nic", [ >> ("backend_domid", libxl_domid), >> ("devid", libxl_devid), >> ("mtu", integer), >> ("model", string), >> ("mac", libxl_mac), >> ("ip", string), >> ("bridge", string), >> ("ifname", string), >> ("script", string), >> ("nictype", libxl_nic_type), >> ("rate_bytes_per_interval", uint64), >> ("rate_interval_usecs", uint32), >> ("gatewaydev", string), >> ]) >> >> (With only one "gatewaydev"). >> > That was my copy'n'paste mistake - fixed. > > I could see that after applying patches V1 the parameter "netdev" is > parsed correctly and the main ip is passed to xen scripts. > However, I could not get xen 4.3 running so I had to test 4.3 tools on > 4.2.1 hypervisor, which could not start the domU, but debug output of > the scripts showed me everything is parsed as expected. > > The mac address still is not parsed if digits are not padded with > leading zero, like "mac=de:ad:a:1e:42:3" - the rest of the line is > ignored without any error. > > However, I was not able to apply patches V2 after that, they do not seem > to work together with the first patch series.Hello Ulf, Version v2 should be applied without v1, they are not additional patches, it's a revision of the same patches (trying to apply both will lead to errors). To simplify it I've pushed the patches to my git repo, so you can fetch them easily: $ git clone -b vif_route_v2 git://xenbits.xen.org/people/royger/xen.git More information about how to build Xen from source can be found at http://wiki.xen.org/wiki/Compiling_Xen_From_Source#Building_from_Source. Hope this helps, Roger.> Only patching with V2 resulted in missing "netdev" member in > libxl_device_nic (libxl_types.idl). After editing the file and adding > the definition, it compiled so far. > > xl with only patches V2 applied does parse neither netdev nor gatewaydev > as it seems. > > > Best regards, > Ulf > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ulf Kreutzberg
2013-Feb-19 16:04 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
Hello Roger, hi all, On 18.02.2013 08:38, Roger Pau Monné wrote:>> The mac address still is not parsed if digits are not padded with >> leading zero, like "mac=de:ad:a:1e:42:3" - the rest of the line is >> ignored without any error. >> >> However, I was not able to apply patches V2 after that, they do not seem >> to work together with the first patch series. > > Hello Ulf, > > Version v2 should be applied without v1, they are not additional > patches, it''s a revision of the same patches (trying to apply both will > lead to errors). To simplify it I''ve pushed the patches to my git repo, > so you can fetch them easily: > > $ git clone -b vif_route_v2 git://xenbits.xen.org/people/royger/xen.git >Many thanks, building that tree definitely works. (Still could not get xen 4.3 running, but that is not the issue here). I have tested the utils with Xen 4.2.1 and parsing of domu configs with netdev= and gatewaydev= parameter works perfectly. Domu does start up and is reacheable. I wonder if you could backport these changes to 4.2? One caveat: the mac address and therfore the rest of the domu config still is not parsed if not padded with leading zeros, as mentioned before. Best regards, Ulf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monné
2013-Mar-04 11:02 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
On 19/02/13 17:04, Ulf Kreutzberg wrote:> Hello Roger, > hi all, > > On 18.02.2013 08:38, Roger Pau Monné wrote: >>> The mac address still is not parsed if digits are not padded with >>> leading zero, like "mac=de:ad:a:1e:42:3" - the rest of the line is >>> ignored without any error. >>> >>> However, I was not able to apply patches V2 after that, they do not seem >>> to work together with the first patch series. >> >> Hello Ulf, >> >> Version v2 should be applied without v1, they are not additional >> patches, it's a revision of the same patches (trying to apply both will >> lead to errors). To simplify it I've pushed the patches to my git repo, >> so you can fetch them easily: >> >> $ git clone -b vif_route_v2 git://xenbits.xen.org/people/royger/xen.git >> > > Many thanks, building that tree definitely works. (Still could not get > xen 4.3 running, but that is not the issue here). I have tested the > utils with Xen 4.2.1 and parsing of domu configs with netdev= and > gatewaydev= parameter works perfectly. Domu does start up and is reacheable. > I wonder if you could backport these changes to 4.2?Are you OK if I add your "Tested-by"? Could you please take a look at the patches when you can IanJ? I would like to get this off my patch queue :)> One caveat: the mac address and therfore the rest of the domu config > still is not parsed if not padded with leading zeros, as mentioned before. > > Best regards, > Ulf >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2013-Mar-12 16:20 UTC
Re: [PATCH v2 1/4] xl/libxl: add gatewaydev/netdev to vif specification
On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote:> This option is used by the vif-route hotplug script. A new more > descriptive name is used, "gatewaydev", but "netdev" is also supported > as a deprecated backwards compatible option. > > This option was supported in the past, according to > http://wiki.xen.org/wiki/Vif-route, so we should also support it in > libxl. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: Ulf Kreutzberg <ulf.kreutzberg@hosteurope.de>Acked-by: Ian Campbell <ian.campbell@citrix.com>> Cc: George Dunlap <george.dunlap@citrix.com> > --- > Changes since v1: > * Don't pass a NULL as a value of a env variable > * Change netdev to gatewaydev > --- > docs/misc/xl-network-configuration.markdown | 10 ++++++++++ > tools/libxl/libxl.c | 6 +++++- > tools/libxl/libxl_linux.c | 9 ++++++++- > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c | 14 ++++++++++++++ > 5 files changed, 38 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/xl-network-configuration.markdown b/docs/misc/xl-network-configuration.markdown > index 5e2f049..e0d3d2a 100644 > --- a/docs/misc/xl-network-configuration.markdown > +++ b/docs/misc/xl-network-configuration.markdown > @@ -67,6 +67,15 @@ added to. The default is `xenbr0`. The bridge must be configured using > your distribution's network configuration tools. See the [wiki][net] > for guidance and examples. > > +### gatewaydev > + > +Specifies the name of the network interface which has an IP and which > +is in the network the VIF should communicate with. This is used in the host > +by the vif-route hotplug script. See [wiki][vifroute] for guidance and > +examples. > + > +NOTE: netdev is a deprecated alias of this option. > + > ### type > > This keyword is valid for HVM guests only. > @@ -158,3 +167,4 @@ on the underlying netback implementation. > > [oui]: http://en.wikipedia.org/wiki/Organizationally_Unique_Identifier > [net]: http://wiki.xen.org/wiki/HostConfiguration/Networking > +[vifroute]: http://wiki.xen.org/wiki/Vif-route > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 73e0dc3..5d590f1 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -2826,7 +2826,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, > if (rc) goto out; > > front = flexarray_make(gc, 16, 1); > - back = flexarray_make(gc, 16, 1); > + back = flexarray_make(gc, 18, 1); > > if (nic->devid == -1) { > if ((nic->devid = libxl__device_nextid(gc, domid, "vif") < 0)) { > @@ -2862,6 +2862,10 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, > flexarray_append(back, "ip"); > flexarray_append(back, libxl__strdup(gc, nic->ip)); > } > + if (nic->gatewaydev) { > + flexarray_append(back, "gatewaydev"); > + flexarray_append(back, libxl__strdup(gc, nic->gatewaydev)); > + } > > if (nic->rate_interval_usecs > 0) { > flexarray_append(back, "rate"); > diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c > index 1fed3cd..60fc533 100644 > --- a/tools/libxl/libxl_linux.c > +++ b/tools/libxl/libxl_linux.c > @@ -84,11 +84,16 @@ static char **get_hotplug_env(libxl__gc *gc, > char *script, libxl__device *dev) > { > const char *type = libxl__device_kind_to_string(dev->backend_kind); > + char *be_path = libxl__device_backend_path(gc, dev); > char **env; > + char *gatewaydev; > int nr = 0; > libxl_nic_type nictype; > > - const int arraysize = 13; > + gatewaydev = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s", be_path, > + "gatewaydev")); > + > + const int arraysize = 15; > GCNEW_ARRAY(env, arraysize); > env[nr++] = "script"; > env[nr++] = script; > @@ -98,6 +103,8 @@ static char **get_hotplug_env(libxl__gc *gc, > env[nr++] = GCSPRINTF("backend/%s/%u/%d", type, dev->domid, dev->devid); > env[nr++] = "XENBUS_BASE_PATH"; > env[nr++] = "backend"; > + env[nr++] = "netdev"; > + env[nr++] = gatewaydev ? : ""; > if (dev->backend_kind == LIBXL__DEVICE_KIND_VIF) { > if (libxl__nic_type(gc, dev, &nictype)) { > LOG(ERROR, "unable to get nictype"); > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index acc4bc9..0112a7a 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -382,6 +382,7 @@ libxl_device_nic = Struct("device_nic", [ > ("nictype", libxl_nic_type), > ("rate_bytes_per_interval", uint64), > ("rate_interval_usecs", uint32), > + ("gatewaydev", string), > ]) > > libxl_device_pci = Struct("device_pci", [ > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 080bbd8..dc1788e 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1205,6 +1205,14 @@ static void parse_config_data(const char *config_source, > parse_vif_rate(&config, (p2 + 1), nic); > } else if (!strcmp(p, "accel")) { > fprintf(stderr, "the accel parameter for vifs is currently not supported\n"); > + } else if (!strcmp(p, "netdev")) { > + fprintf(stderr, "the netdev parameter is deprecated, " > + "please use gatewaydev instead\n"); > + free(nic->gatewaydev); > + nic->gatewaydev = strdup(p2 + 1); > + } else if (!strcmp(p, "gatewaydev")) { > + free(nic->gatewaydev); > + nic->gatewaydev = strdup(p2 + 1); > } > } while ((p = strtok(NULL, ",")) != NULL); > skip_nic: > @@ -5471,6 +5479,12 @@ int main_networkattach(int argc, char **argv) > } > } else if (MATCH_OPTION("bridge", *argv, oparg)) { > replace_string(&nic.bridge, oparg); > + } else if (MATCH_OPTION("netdev", *argv, oparg)) { > + fprintf(stderr, "the netdev parameter is deprecated, " > + "please use gatewaydev instead\n"); > + replace_string(&nic.gatewaydev, oparg); > + } else if (MATCH_OPTION("gatewaydev", *argv, oparg)) { > + replace_string(&nic.gatewaydev, oparg); > } else if (MATCH_OPTION("ip", *argv, oparg)) { > replace_string(&nic.ip, oparg); > } else if (MATCH_OPTION("script", *argv, oparg)) {_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2013-Mar-12 16:22 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote:> This adds a new global option in the xl configuration file called > "vif.default.gatewaydev", that is used to specify the default > gatewaydev to use when none is passed in the vif specification. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: Ulf Kreutzberg <ulf.kreutzberg@hosteurope.de> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: George Dunlap <george.dunlap@citrix.com> > --- > Changes since v1: > * Rename defaultnetdev to vif.default.gatewaydevThat whole file is a bit uncomfortable ad-hoc in its naming, but oh well. Acked-by: Ian Campbell <ian.campbell@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote:> This is a replacement for defaultbridge xl.conf option. The now > deprecated defaultbridge is still supported. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > --- > tools/examples/xl.conf | 3 +++ > tools/libxl/xl.c | 13 +++++++++++--No change to docs/man/xl.conf.pod.5? 2 files changed, 14 insertions(+), 2 deletions(-)> > diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf > index 9a03fff..4451176 100644 > --- a/tools/examples/xl.conf > +++ b/tools/examples/xl.conf > @@ -23,3 +23,6 @@ > > # default gateway device to use with vif-route hotplug script > #vif.default.gatewaydev="eth0" > + > +# default bridge device to use with vif-bridge hotplug scripts > +#vif.default.bridge="bridge0"Common names (at least in Linux-land) are "xenbr0" and "br0". Unless this conflicts massively with *BSD perhaps using one of those would be useful?> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c > index f657216..100ab32 100644 > --- a/tools/libxl/xl.c > +++ b/tools/libxl/xl.c > @@ -89,8 +89,17 @@ static void parse_global_config(const char *configfile, > if (!xlu_cfg_get_string (config, "vifscript", &buf, 0)) > default_vifscript = strdup(buf); > > - if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) > - default_bridge = strdup(buf); > + if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) { > + fprintf(stderr, "the global config option defaultbridge is deprecated, " > + "please switch to vif.default.bridge\n"); > + free(default_bridge); > + default_bridge = strdup(buf); > + } > + > + if (!xlu_cfg_get_string (config, "vif.default.bridge", &buf, 0)) { > + free(default_bridge); > + default_bridge = strdup(buf); > + }Put else if (!xlu.... ("defaultbridge")... ) here instead of above so we only warn if the user gave defaultbridge but not vif.default.bridge? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote:> Replace vifscript with vif.default.script. The old config option is > kept for backwards compatibility. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: George Dunlap <george.dunlap@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > --- > tools/libxl/xl.c | 11 ++++++++++-Needs docs changes too I expect.> 1 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c > index 100ab32..92565d1 100644 > --- a/tools/libxl/xl.c > +++ b/tools/libxl/xl.c > @@ -86,8 +86,17 @@ static void parse_global_config(const char *configfile, > exit(1); > } > > - if (!xlu_cfg_get_string (config, "vifscript", &buf, 0)) > + if (!xlu_cfg_get_string (config, "vifscript", &buf, 0)) { > + fprintf(stderr, "the global config option vifscript is deprecated, " > + "please switch to vif.default.script\n"); > + free(default_vifscript); > default_vifscript = strdup(buf); > + } > + > + if (!xlu_cfg_get_string (config, "vif.default.script", &buf, 0)) { > + free(default_vifscript); > + default_vifscript = strdup(buf); > + }Same comment as 3/4 about checking the deprecated name second. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2013-Mar-12 16:27 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
On Tue, 2013-03-12 at 16:22 +0000, Ian Campbell wrote:> On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote: > > This adds a new global option in the xl configuration file called > > "vif.default.gatewaydev", that is used to specify the default > > gatewaydev to use when none is passed in the vif specification. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > Cc: Ulf Kreutzberg <ulf.kreutzberg@hosteurope.de> > > Cc: Ian Campbell <ian.campbell@citrix.com> > > Cc: George Dunlap <george.dunlap@citrix.com> > > --- > > Changes since v1: > > * Rename defaultnetdev to vif.default.gatewaydev > > That whole file is a bit uncomfortable ad-hoc in its naming, but oh > well.Although if you fancied writing down some sort of simple "schema" (which is far too grand a word for what I'm thinking of) which fits the scheme you are using here and could serve as guidance for future new variable names, that would be very nice. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ulf Kreutzberg
2013-Mar-12 16:39 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
Roger, sorry for the very-late reply... On 04.03.2013 12:02, Roger Pau Monné wrote:> > Are you OK if I add your "Tested-by"? >That is OK, of course. Thanks and regards, Ulf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Roger Pau Monné
2013-Mar-13 16:29 UTC
Re: [PATCH v2 2/4] xl: allow specifying a default gatewaydev in xl.conf
On 12/03/13 17:27, Ian Campbell wrote:> On Tue, 2013-03-12 at 16:22 +0000, Ian Campbell wrote: >> On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote: >>> This adds a new global option in the xl configuration file called >>> "vif.default.gatewaydev", that is used to specify the default >>> gatewaydev to use when none is passed in the vif specification. >>> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> Cc: Ulf Kreutzberg <ulf.kreutzberg@hosteurope.de> >>> Cc: Ian Campbell <ian.campbell@citrix.com> >>> Cc: George Dunlap <george.dunlap@citrix.com> >>> --- >>> Changes since v1: >>> * Rename defaultnetdev to vif.default.gatewaydev >> >> That whole file is a bit uncomfortable ad-hoc in its naming, but oh >> well. > > Although if you fancied writing down some sort of simple "schema" (which > is far too grand a word for what I'm thinking of) which fits the scheme > you are using here and could serve as guidance for future new variable > names, that would be very nice.Thanks for the review, I guess the best place for this would be a comment on the code next to the section that parses the global options. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On 12/03/13 17:25, Ian Campbell wrote:> On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote: >> This is a replacement for defaultbridge xl.conf option. The now >> deprecated defaultbridge is still supported. >> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> Cc: George Dunlap <George.Dunlap@eu.citrix.com> >> Cc: Ian Campbell <ian.campbell@citrix.com> >> --- >> tools/examples/xl.conf | 3 +++ >> tools/libxl/xl.c | 13 +++++++++++-- > > No change to docs/man/xl.conf.pod.5?My bad, I'm going to add it now.> 2 files changed, 14 insertions(+), > 2 deletions(-) >> >> diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf >> index 9a03fff..4451176 100644 >> --- a/tools/examples/xl.conf >> +++ b/tools/examples/xl.conf >> @@ -23,3 +23,6 @@ >> >> # default gateway device to use with vif-route hotplug script >> #vif.default.gatewaydev="eth0" >> + >> +# default bridge device to use with vif-bridge hotplug scripts >> +#vif.default.bridge="bridge0" > > Common names (at least in Linux-land) are "xenbr0" and "br0". Unless > this conflicts massively with *BSD perhaps using one of those would be > useful?I use bridgeX on both Linux and BSD, but that's just my choice. br0 or xenbr0 is probably more common, so I'm going to change it.> > >> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c >> index f657216..100ab32 100644 >> --- a/tools/libxl/xl.c >> +++ b/tools/libxl/xl.c >> @@ -89,8 +89,17 @@ static void parse_global_config(const char *configfile, >> if (!xlu_cfg_get_string (config, "vifscript", &buf, 0)) >> default_vifscript = strdup(buf); >> >> - if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) >> - default_bridge = strdup(buf); >> + if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) { >> + fprintf(stderr, "the global config option defaultbridge is deprecated, " >> + "please switch to vif.default.bridge\n"); >> + free(default_bridge); >> + default_bridge = strdup(buf); >> + } >> + >> + if (!xlu_cfg_get_string (config, "vif.default.bridge", &buf, 0)) { >> + free(default_bridge); >> + default_bridge = strdup(buf); >> + } > > Put else if (!xlu.... ("defaultbridge")... ) here instead of above so we > only warn if the user gave defaultbridge but not vif.default.bridge?Since "defaultbridge" is now deprecated I think we should warn the user about it, even if vif.default.bridge is also specified (which also doesn't make much sense to use both) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel