[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Ian Campbell escribiÃ:
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).

This is ok if we execute hotplug scripts with udev, because they are executed when the "physical" device is destroyed, but if we have to launch the scripts from the toolstack, the only way to know that the physical device is no longer in use is by watching xenstore backend entries, that's why we cannot nuke them unless it is our last option.

   * 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.