[Xen-devel] [PATCH RFC] libxl: set disk defaults in remove/destroy functions

[Jim Fehlig <jfehlig@xxxxxxxx>]

The attached patch is a hack I cooked up to fix one of the libvirt-TCK
Xen failures.  The test (200-disk-hotplug.t) attempts to hot add and
remove a disk from a running domain.  The add works fine, but remove
fails with

libxl: debug: libxl.c:3858:libxl_device_disk_remove: ao 0x7f9b9c0015a0:
create: how=(nil) callback=(nil) poller=
0x7f9ba0004590
libxl: error: libxl.c:2399:libxl__device_from_disk: unrecognized disk
backend type: 0

The test does not define a backend type, in which case the libvirt libxl
driver allows libxl to choose an appropriate backend via
libxl__device_disk_set_backend().  The backend type is never set on a
remove operation, hence it fails with the above error.

I spent some time trying to figure out the best place to set backend
type on remove, but in the end could only come up with this hack.  It
wouldn't feel so gross if I could have simply added
'libxl__device_##type##_setdefault(gc, type);' to the existing
DEFINE_DEVICE_REMOVE macro, but alas libxl__device_nic_setdefault() has
a different prototype than the other devices.

Better suggestions welcomed!  One I considered was fixing this in
libvirt.  But the Xen community suggested allowing libxl to choose a
suitable backend when not specified, so I think this recommendation
should be symmetrical in the add and remove operations.

Regards,
Jim

From 48f5dc7b80384538ff343af46379efefe51986be Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@xxxxxxxx>
Date: Mon, 26 Jan 2015 15:30:20 -0700
Subject: [PATCH] libxl: set disk defaults in remove/destroy functions

It is possible to hot add a disk without specifying a backend,
allowing libxl to determine a suitable one.  E.g. a disk can be
hot added with the following libvirt domXML snippet

<disk type='file' device='disk'>
  <source file='/some/path'/>
  <target dev='xvdb' bus='xen'/>
</disk>

But trying to remove the disk with the same XML snippet results in

libxl: error: libxl.c:2399:libxl__device_from_disk: unrecognized disk
       backend type: 0

Unlike disk_add, disk_remove does not call libxl__device_disk_setdefault().
This patch ensures libxl__device_disk_setdefault() is called on the
remove operation as well.

Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
 tools/libxl/libxl.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 82227e8..e08638f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4108,6 +4108,36 @@ out:
  * libxl_device_vfb_remove
  * libxl_device_vfb_destroy
  */
+#define DEFINE_DISK_DEVICE_REMOVE(removedestroy, f)                     \
+    int libxl_device_disk_##removedestroy(libxl_ctx *ctx,               \
+        uint32_t domid, libxl_device_disk *disk,                        \
+        const libxl_asyncop_how *ao_how)                                \
+    {                                                                   \
+        AO_CREATE(ctx, domid, ao_how);                                  \
+        libxl__device *device;                                          \
+        libxl__ao_device *aodev;                                        \
+        int rc;                                                         \
+                                                                        \
+        rc = libxl__device_disk_setdefault(gc, disk);                   \
+        if (rc) goto out;                                               \
+                                                                        \
+        GCNEW(device);                                                  \
+        rc = libxl__device_from_disk(gc, domid, disk, device);          \
+        if (rc != 0) goto out;                                          \
+                                                                        \
+        GCNEW(aodev);                                                   \
+        libxl__prepare_ao_device(ao, aodev);                            \
+        aodev->action = LIBXL__DEVICE_ACTION_REMOVE;                    \
+        aodev->dev = device;                                            \
+        aodev->callback = device_addrm_aocomplete;                      \
+        aodev->force = f;                                               \
+        libxl__initiate_device_remove(egc, aodev);                      \
+                                                                        \
+    out:                                                                \
+        if (rc) return AO_ABORT(rc);                                    \
+        return AO_INPROGRESS;                                           \
+    }
+
 #define DEFINE_DEVICE_REMOVE(type, removedestroy, f)                    \
     int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,           \
         uint32_t domid, libxl_device_##type *type,                      \
@@ -4138,8 +4168,8 @@ out:
 /* Define all remove/destroy functions and undef the macro */
 
 /* disk */
-DEFINE_DEVICE_REMOVE(disk, remove, 0)
-DEFINE_DEVICE_REMOVE(disk, destroy, 1)
+DEFINE_DISK_DEVICE_REMOVE(remove, 0)
+DEFINE_DISK_DEVICE_REMOVE(destroy, 1)
 
 /* nic */
 DEFINE_DEVICE_REMOVE(nic, remove, 0)
@@ -4162,6 +4192,7 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
  * 1. add support for secondary consoles to xenconsoled
  * 2. dynamically add/remove qemu chardevs via qmp messages. */
 
+#undef DEFINE_DISK_DEVICE_REMOVE
 #undef DEFINE_DEVICE_REMOVE
 
 
/******************************************************************************/
-- 
1.8.0.1

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