Re: Fwd: Re: [PATCH v3 3/5] libxl: call hotplug scripts from libxl for vbd

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2012-04-26 at 12:48 +0100, Roger Pau Monne wrote:
> Ian Jackson escribió:
> > Thanks for this mammoth patch.  My comments are below.
> >
> > Roger Pau Monne writes ("[Xen-devel] [PATCH v3 3/5] libxl: call hotplug scripts from libxl for vbd"):
> >> This is a rather big change, that adds the necessary machinery to perform
> >> hotplug script calling from libxl for Linux. This patch launches the necessary
> >> scripts to attach and detach PHY and TAP backend types (Qemu doesn't
> >> use hotplug scripts).
> >>
> >> Here's a list of the major changes introduced:
> >
> > Can you please wrap your commit messages to no more than 75
> > characters ?  Some vcs's indent them.  And of course they get indented
> > when we reply leading to wrap damage.
> >
> >>   * libxl__devices_destroy no longer calls libxl__device_destroy, and instead
> >>     calls libxl__initiate_device_remove, so we can disconnect the device and
> >>     execute the necessary hotplug scripts instead of just deleting the backend
> >>     entries. So libxl__devices_destroy now uses the event library, and so does
> >>     libxl_domain_destroy.
> >
> > So we no longer have any function which will synchronously and
> > immediately destroy a domain and throw away everything to do with it.
> > Is it actually the case that you think such a function cannot be
> > provided ?
> 
> It can be provided, but we should create another command or add an
> option to "destroy" (like -f), that doesn't wait for backend
> disconnection and just nukes both frontend/backend xenstore entries.
> This will also prevent us from executing hotplug scripts, and could lead
> to unexpected results.

We have a plan of action to fix this properly in 4.3 (via the Driver Dom
protocol, which separates the backend from the hotplug info).

It would IMHO be acceptable for 4.2 to keep on simply nuking the
backend, and accepting the downsides which this entails. AFAIK this is
not a regression vs Xend -- xend also just nukes the backend (please
correct me if I am wrong about this).

> >>   * Added a check in xen-hotplug-common.sh, so scripts are only executed from
> >>     udev when using xend. xl adds a disable_udev=y to xenstore private directory
> >>     and with the modification of the udev rules every call from udev gets
> >>     HOTPLUG_GATE passed, that points to disable_udev. If HOTPLUG_GATE is passed
> >>     and points to a value, the hotplug script is not executed.
> >
> > WDYM "points to a value"?  Did you just mean "is set to a nonempty
> > value" ?  I'm not convinced by the name of this variable.  Shouldn't
> > it have Xen in the name, and specify its own sense ?  Eg,
> > XEN_HOTPLUG_DISABLE or something ?
> 
> This was decided with Ian Campbell,

Please do feel free to take my half-baked suggestions and make them
sensible ;-)

Ian.



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