Roger Pau Monne
2012-Aug-14 18:15 UTC
[PATCH] 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>
---
docs/misc/xl-network-configuration.markdown | 6 ++++--
tools/libxl/libxl.c | 14 ++++++++++++++
2 files changed, 18 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/libxl.c b/tools/libxl/libxl.c
index 8ea3478..6b85cdc 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, "the vif ''backend='' option cannot be
used in conjunction "
+ "with run_hotplug_scripts, please set
run_hotplug_scripts "
+ "to 0 in xl.conf");
+ return ERROR_FAIL;
+ }
+
switch (libxl__domain_type(gc, domid)) {
case LIBXL_DOMAIN_TYPE_HVM:
if (!nic->nictype)
--
1.7.7.5 (Apple Git-26)
Ian Campbell
2012-Aug-15 08:54 UTC
Re: [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
On Tue, 2012-08-14 at 19:15 +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> > --- > docs/misc/xl-network-configuration.markdown | 6 ++++-- > tools/libxl/libxl.c | 14 ++++++++++++++ > 2 files changed, 18 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/libxl.c b/tools/libxl/libxl.c > index 8ea3478..6b85cdc 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");Include the error value?> + return run_hotplug_scripts; > + } > + if (nic->backend_domid != LIBXL_TOOLSTACK_DOMID && run_hotplug_scripts) { > + LOG(ERROR, "the vif ''backend='' option cannot be used in conjunction " > + "with run_hotplug_scripts, please set run_hotplug_scripts " > + "to 0 in xl.conf");This mention of xl.conf in libxl is a layering violation. I think it would be fine for libxl to log something generic about hotplug scripts and return an error and to have a more specific check t parse time in xl which errors out with a reference to the config option.> + return ERROR_FAIL; > + } > + > switch (libxl__domain_type(gc, domid)) { > case LIBXL_DOMAIN_TYPE_HVM: > if (!nic->nictype)
Roger Pau Monne
2012-Aug-23 17:16 UTC
Re: [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
Ian Campbell wrote:> On Tue, 2012-08-14 at 19:15 +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> >> --- >> docs/misc/xl-network-configuration.markdown | 6 ++++-- >> tools/libxl/libxl.c | 14 ++++++++++++++ >> 2 files changed, 18 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/libxl.c b/tools/libxl/libxl.c >> index 8ea3478..6b85cdc 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"); > > Include the error value?libxl__hotplug_settings already prints an error message, that contains the errno value. I think that''s enough.>> + return run_hotplug_scripts; >> + } >> + if (nic->backend_domid != LIBXL_TOOLSTACK_DOMID && run_hotplug_scripts) { >> + LOG(ERROR, "the vif ''backend='' option cannot be used in conjunction " >> + "with run_hotplug_scripts, please set run_hotplug_scripts " >> + "to 0 in xl.conf"); > > This mention of xl.conf in libxl is a layering violation. > > I think it would be fine for libxl to log something generic about > hotplug scripts and return an error and to have a more specific check t > parse time in xl which errors out with a reference to the config option.Ok, I will probably split this in two patches, one for libxl and one for the parser. Thanks for the review.
Ian Campbell
2012-Aug-31 09:27 UTC
Re: [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
On Thu, 2012-08-23 at 18:16 +0100, Roger Pau Monne wrote:> Ok, I will probably split this in two patches, one for libxl and one > for the parser.Are you still indenting to do this for 4.2.0? It''s fast approaching... Ian.
Roger Pau Monne
2012-Aug-31 18:22 UTC
Re: [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
Ian Campbell wrote:> On Thu, 2012-08-23 at 18:16 +0100, Roger Pau Monne wrote: >> Ok, I will probably split this in two patches, one for libxl and one >> for the parser. > > Are you still indenting to do this for 4.2.0? It''s fast approaching...Yes, sorry for the delay. I will try to submit a patch for it on Tuesday.