Re: [Xen-devel] [PATCH v2 4/5] libxl: call hotplug scripts from libxl for vif

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On 18 Apr 2012, at 10:54, Ian Campbell wrote:

> On Tue, 2012-04-17 at 16:51 +0100, Roger Pau Monne wrote:
> > As the previous change already introduces most of needed machinery to 
> > call
> > hotplug scripts from libxl, this only adds the necessary bits to call 
> > this
> > scripts for vif interfaces also.
> >
> > libxl_device_nic_add has been changed to make use of the new event
> > functionality, and the necessary vif hotplug code has been added. No 
> > changes
> > where needed in the teardown part, since it uses exactly the same 
> > code
> > introduced for vbd.
> >
> > An exception has been introduced for tap devices, since hotplug 
> > scripts
> > should be called after the device model has been created, so the 
> > hotplug
> > call for those is done in do_domain_create after confirmation of the 
> > device
> > model startup. On the other hand, tap devices don't use teardown 
> > scripts,
> > so add another exception for that.
> >
> > libxl__device_from_nic was moved to libxl_device.c, so it can be used
> > in libxl_create.c, and libxl__device_from_disk was also moved to 
> > mantain
> > the simmetry.
> "maintain" and "symmetry"
>
> These are pure code motion or did the code actually change too?
>
> > libxl__initiate_device_remove has been changed a little, to nuke the 
> > frontend
> > xenstore path before trying to perform device teardown, this allows 
> > for
> > unitialized devices to be closed gracefully, specially vif interfaces 
> > added to
> uninitialized (or -ised)
>
> > HVM machines and not used.
> >
> > PV nic devices are set to LIBXL_NIC_TYPE_VIF, since the default value 
> > is
> > LIBXL_NIC_TYPE_IOEMU regardless of the guest type.
> >
> > A new gobal option, called disable_vif_scripts has been added to 
> > allow the user
>     global
>
> > decide if he wants to execute the hotplug scripts from xl or from 
> > udev. This has
> > been documented in the xl.conf man page.
> Did you do this for block too?
>
> > Changes since v1:
> >
> > * Propagated changes from previous patch (disable_udev and 
> > libxl_device_nic_add
> > switch).
> >
> > * Modified udev rules to add the new env variable.
> >
> > * Added support for named interfaces.
> >
> > Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > ---
> > docs/man/xl.conf.pod.5                |    7 ++
> > tools/hotplug/Linux/xen-backend.rules |    6 +-
> > tools/libxl/libxl.c                   |   90 +++++++-------------
> > tools/libxl/libxl.h                   |    3 +-
> > tools/libxl/libxl_create.c            |   22 +++++-
> > tools/libxl/libxl_device.c            |   77 +++++++++++++++--
> > tools/libxl/libxl_dm.c                |    2 +-
> > tools/libxl/libxl_internal.h          |    6 ++
> > tools/libxl/libxl_linux.c             |  150 
> > ++++++++++++++++++++++++++++++++-
> > tools/libxl/libxl_types.idl           |    1 +
> > tools/libxl/xl.c                      |    4 +
> > tools/libxl/xl.h                      |    1 +
> > tools/libxl/xl_cmdimpl.c              |   15 +++-
> > 13 files changed, 308 insertions(+), 76 deletions(-)
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 01ff363..41230a6 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -846,6 +846,19 @@ static void parse_config_data(const char 
> > *configfile_filename_report,
> >              nic->script = strdup(default_vifscript);
> >          }
> >
> > +            /* Set default nic type for PV guests correctly */
> > +            if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
> > +                nic->nictype = LIBXL_NIC_TYPE_VIF;
> > +            }
> Hrm, really the lib ought to be taking care of that for us...
>
> libxl__device_nic_setdefault has a domid so it should be able to query
> the domain type with libxl__domain_type.
int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic) 
is the prototype, and libxl_device_nic doesn't have a domid, so I'm not 
sure where should I get it from.
> > +            /*
> > +             * Disable nic network script calling, to allow the user
> > +             * to attach the nic backend from a different domain.
> > +             */
> > +            if (disable_vif_scripts) {
> > +                nic->disable_vif_script = 1;
> > +            }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel