Re: [Xen-devel] [PATCH 0 of 2] xl: fix localhost migration after 25733:353bc0801b11

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-08-07 at 11:33 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH 0 of 2] xl: fix localhost migration after 
> 25733:353bc0801b11"):
> > On Tue, 2012-08-07 at 09:25 +0100, Ian Campbell wrote:
> > > I'm going to investigate if I can make xl sequence the device
> > > teardowns in the migration so as to make it work. 
> > 
> > Here is a short series which does this. 
> > 
> > It's a bit ad-hoc, we probably want to revist this for 4.3 (if not
> > now).
> 
> I think the right answer at this stage of the release is to back out
> the part of the recent change which is causing the problem.
> 
> In this case that means abolishing the execution of
> /etc/xen/scripts/block, which "libxl: support custom block hotplug
> scripts" had as an intended side-effect.

Here it is. This approach has the side effect that we now require block
hotplug scripts to properly nest/reference count for localhost migration
to work. e.g. the effect of add->add->remove calls to the script should
be that the device remains connected until a second remove. This may be
more or less trivial depending on the device

I suppose we can live with that limitation since localhost migration is
mostly for test purposes.

I'm slightly concerned that this may also effect non-localhost
migration. e.g. an iSCSI target which only permits a single login.

8<------------------------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1344337292 -3600
# Node ID c5f673d8b330d6195e2aa3bbf63bb594b4bc99ee
# Parent  a7ad22e5525831dd491d7ee1fe538b7543404ac7
libxl: write physical-device node if user did not supply a block script

This reverts one of the intentional changes from 25733:353bc0801b11.
That change exposed an issue with the xl migration protocol, which
although safe triggers the hotplug scripts device sharing logic.

For 4.2 we disable this logic by writing the physical-device xenstore
node ourselves if a user did not supply a script. If the user did
supply a script then we continue to rely on it to write the
physical-device node (not least because the script may create the
device and therefore it is not available before we run the script).

This means that to support localhost migration a block hotplug script
needs to be robust against adding a device twice and should not
deactivate the device until it has been removed twice.

This should be revisted for 4.3.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r a7ad22e55258 -r c5f673d8b330 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Mon Aug 06 12:31:02 2012 +0100
+++ b/tools/libxl/libxl.c       Tue Aug 07 12:01:32 2012 +0100
@@ -1845,18 +1845,31 @@ static void device_disk_add(libxl__egc *
             case LIBXL_DISK_BACKEND_PHY:
                 dev = disk->pdev_path;
 
-                script = libxl__abs_path(gc, disk->script ?: "block",
-                                         libxl__xen_script_dir_path());
-
         do_backend_phy:
                 flexarray_append(back, "params");
                 flexarray_append(back, dev);
 
-                assert(script);
+                script = libxl__abs_path(gc, disk->script?: "block",
+                                         libxl__xen_script_dir_path());
                 flexarray_append_pair(back, "script", script);
 
+                /* If the user did not supply a block script then we
+                 * write the physical-device node ourselves.
+                 *
+                 * If the user did supply a script then that script is
+                 * responsible for this since the block device may not
+                 * exist yet.
+                 */
+                if (!disk->script) {
+                    int major, minor;
+                    libxl__device_physdisk_major_minor(dev, &major, &minor);
+                    flexarray_append_pair(back, "physical-device",
+                            libxl__sprintf(gc, "%x:%x", major, minor));
+                }
+
                 assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
                 break;
+
             case LIBXL_DISK_BACKEND_TAP:
                 dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
                 if (!dev) {
@@ -1870,12 +1883,9 @@ static void device_disk_add(libxl__egc *
                     libxl__device_disk_string_of_format(disk->format),
                     disk->pdev_path));
 
-                /*
-                 * tap devices do not support custom block scripts and
-                 * always use the plain block script.
-                 */
-                script = libxl__abs_path(gc, "block",
-                                         libxl__xen_script_dir_path());
+                /* tap backends with scripts are rejected by
+                 * libxl__device_disk_set_backend */
+                assert(!disk->script);
 
                 /* now create a phy device to export the device to the guest */
                 goto do_backend_phy;



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