* [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
@ 2012-08-14 18:15 Roger Pau Monne
2012-08-15 8:54 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Roger Pau Monne @ 2012-08-14 18:15 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne
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)
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
2012-08-14 18:15 [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts Roger Pau Monne
@ 2012-08-15 8:54 ` Ian Campbell
2012-08-23 17:16 ` Roger Pau Monne
0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2012-08-15 8:54 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel@lists.xen.org
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)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts
2012-08-15 8:54 ` Ian Campbell
@ 2012-08-23 17:16 ` Roger Pau Monne
2012-08-31 9:27 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Roger Pau Monne @ 2012-08-23 17:16 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-31 18:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-14 18:15 [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts Roger Pau Monne
2012-08-15 8:54 ` Ian Campbell
2012-08-23 17:16 ` Roger Pau Monne
2012-08-31 9:27 ` Ian Campbell
2012-08-31 18:22 ` Roger Pau Monne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).