From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jim Fehlig Subject: [PATCH RFC] libxl: set disk defaults in remove/destroy functions Date: Mon, 26 Jan 2015 16:14:18 -0700 Message-ID: <54C6CA4A.4040505@suse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------020407060608020409030209" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------020407060608020409030209 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 --------------020407060608020409030209 Content-Type: text/x-diff; name="0001-libxl-set-disk-defaults-in-remove-destroy-functions.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-libxl-set-disk-defaults-in-remove-destroy-functions.pat"; filename*1="ch" >>From 48f5dc7b80384538ff343af46379efefe51986be Mon Sep 17 00:00:00 2001 From: Jim Fehlig 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 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 --- 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 --------------020407060608020409030209 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------020407060608020409030209--