Re: [Xen-devel] [PATCH] libxl: fix usage of backend parameter and run_hotplug_scripts

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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)



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