From patchwork Thu Mar 3 18:12:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Jackson X-Patchwork-Id: 8495241 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AB3CB9F2F0 for ; Thu, 3 Mar 2016 18:15:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 88B0E20386 for ; Thu, 3 Mar 2016 18:15:57 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6742A2037E for ; Thu, 3 Mar 2016 18:15:56 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1abXkG-0007Rp-D1; Thu, 03 Mar 2016 18:13:04 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1abXkE-0007Rj-Lx for xen-devel@lists.xensource.com; Thu, 03 Mar 2016 18:13:02 +0000 Received: from [85.158.139.211] by server-3.bemta-5.messagelabs.com id 11/B5-02408-DAE78D65; Thu, 03 Mar 2016 18:13:01 +0000 X-Env-Sender: prvs=863aa12bc=Ian.Jackson@citrix.com X-Msg-Ref: server-5.tower-206.messagelabs.com!1457028779!26640107!1 X-Originating-IP: [66.165.176.63] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni42MyA9PiAzMDYwNDg=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 8.11; banners=-,-,- X-VirusChecked: Checked Received: (qmail 63591 invoked from network); 3 Mar 2016 18:13:01 -0000 Received: from smtp02.citrix.com (HELO SMTP02.CITRIX.COM) (66.165.176.63) by server-5.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 3 Mar 2016 18:13:01 -0000 X-IronPort-AV: E=Sophos;i="5.22,532,1449532800"; d="scan'208";a="342995051" From: Ian Jackson To: Date: Thu, 3 Mar 2016 18:12:50 +0000 Message-ID: <1457028770-31925-1-git-send-email-ian.jackson@eu.citrix.com> X-Mailer: git-send-email 1.7.10.4 MIME-Version: 1.0 X-DLP: MIA2 Cc: Wei Liu , Ian Jackson , George Dunlap , Roger Pau Monne Subject: [Xen-devel] [PATCH] RFC: libxl: Change hotplug script interface to use physical-device-path X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP * Change block-common.sh on Linux to write physical-device-path with the path of the device node, rather than physical-device with its major:minor numbers. * Have the libxl Linux hotplug script scheduler convert this, by reading physical-device-path and writing physical-device. This happens just when we have decided that there is no more script to run. * When libxl itself writes the device to xenstore, have it write physical-device-path. On Linux this will be converted to physical-device by the new code discussed above. * Add a note about libxl__device_physdisk_major_minor's poor error handling. * Add an xxx about the fact that the sharing check in tools/hotplug/Linux/block needs adjusting. Note that WITHOUT THIS OTHER FIX THE CHANGE TO BLOCK-COMMON.SH IS BROKEN. This is one reason this patch is RFC. * Untested. That is the other reason this patch is RFC. (I have checked that it does compile.) I wasn't able to find a document describing the hotplug script interface (!) so this patch does not contain docs hunks. CC: George Dunlap CC: Roger Pau Monne CC: Wei Liu Signed-off-by: Ian Jackson --- tools/hotplug/Linux/block-common.sh | 19 +++------- tools/libxl/libxl.c | 5 +-- tools/libxl/libxl_internal.h | 1 + tools/libxl/libxl_linux.c | 70 ++++++++++++++++++++++++++++++++++- 4 files changed, 75 insertions(+), 20 deletions(-) diff --git a/tools/hotplug/Linux/block-common.sh b/tools/hotplug/Linux/block-common.sh index ee95009..560f189 100644 --- a/tools/hotplug/Linux/block-common.sh +++ b/tools/hotplug/Linux/block-common.sh @@ -50,23 +50,14 @@ device_major_minor() ## -# Write physical-device = MM,mm to the store, where MM and mm are the major -# and minor numbers of device respectively. +# Write physical-device-path = "$1" # -# @param device The device from which major and minor numbers are read, which -# will be written into the store. +# @param device The device which will be written into the store. # write_dev() { - local mm - - mm=$(device_major_minor "$1") - - if [ -z $mm ] - then - fatal "Backend device does not exist" - fi - - xenstore_write "$XENBUS_PATH/physical-device" "$mm" + xxx check_sharing in block needs fixing too + + xenstore_write "$XENBUS_PATH/physical-device-path" "$1" success } diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 4cdc169..0bcfada 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2450,10 +2450,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, */ if (!disk->script && disk->backend_domid == LIBXL_TOOLSTACK_DOMID) { - int major, minor; - if (!libxl__device_physdisk_major_minor(dev, &major, &minor)) - flexarray_append_pair(back, "physical-device", - GCSPRINTF("%x:%x", major, minor)); + flexarray_append_pair(back, "physical-device-path", dev); } assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 005fe53..ada5f40 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1154,6 +1154,7 @@ _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format); _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*); _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor); + /* xxx libxl__device_physdisk_major_minor has irregular error handling */ _hidden int libxl__device_disk_dev_number(const char *virtpath, int *pdisk, int *ppartition); diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c index be4afc6..aa7b42d 100644 --- a/tools/libxl/libxl_linux.c +++ b/tools/libxl/libxl_linux.c @@ -233,6 +233,65 @@ error: return rc; } +static int disk_copy_block_device(libxl__gc *gc, libxl__device *dev, + libxl__device_action action) +{ + int rc; + xs_transaction_t t = 0; + const char *be_path = libxl__device_frontend_path(gc, dev); + const char *majmin_path = GCSPRINTF("%s/physical-device", be_path); + const char *path_path = GCSPRINTF("%s/physical-device-path", be_path); + int major, minor; + + if (action != LIBXL__DEVICE_ACTION_ADD) + return 0; + + for (;;) { + rc = libxl__xs_transaction_start(gc, &t); + if (rc) goto out; + + const char *majmin; + rc = libxl__xs_read_checked(gc,t, majmin_path, &majmin); + if (rc) goto out; + if (majmin) { + /* Old-style Linux-only hotplug script wrote physical-device */ + rc = 0; + goto out; + } + + const char *bdev; + rc = libxl__xs_read_checked(gc,t, path_path, &bdev); + if (rc) goto out; + if (!bdev) { + LOGE(ERROR, "nothing wrote physical device to %s", path_path); + rc = ERROR_FAIL; + goto out; + } + + if (!libxl__device_physdisk_major_minor(bdev, &major, &minor)) { + LOG(ERROR, "libxl__device_physdisk_major_minor failed (on %s)", + bdev); + rc = ERROR_FAIL; + goto out; + } + + majmin = GCSPRINTF("%x:%x", major, minor); + rc = libxl__xs_write_checked(gc,t, majmin_path, majmin); + if (rc) goto out; + + rc = libxl__xs_transaction_commit(gc, &t); + if (!rc) break; + if (rc<0) goto out; + } + + return rc; + + out: + libxl__xs_transaction_abort(gc, &t); + return rc; +} + + int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, char ***args, char ***env, libxl__device_action action, @@ -245,9 +304,16 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev, if (num_exec != 0) { LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec); rc = 0; - goto out; + } else { + rc = libxl__hotplug_disk(gc, dev, args, env, action); + } + if (!rc) { + /* No more hotplug scripts to run. But, the hotplug + * scripts write physical-device-path but we blkback wants + * physical-device (major:minor). So now we adjust + * that. */ + rc = disk_copy_block_device(gc, dev, action); } - rc = libxl__hotplug_disk(gc, dev, args, env, action); break; case LIBXL__DEVICE_KIND_VIF: /*