From patchwork Tue Apr 9 16:45:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 10891729 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C321C17E0 for ; Tue, 9 Apr 2019 16:47:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AF4EA285A5 for ; Tue, 9 Apr 2019 16:47:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A3B7D2888E; Tue, 9 Apr 2019 16:47:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2410D285A5 for ; Tue, 9 Apr 2019 16:47:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hDtse-00089M-Hq; Tue, 09 Apr 2019 16:45:52 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hDtsd-00088Z-1N for xen-devel@lists.xenproject.org; Tue, 09 Apr 2019 16:45:51 +0000 X-Inumbo-ID: ecc43376-5ae6-11e9-9b6d-2f6f6651b6ba Received: from SMTP03.CITRIX.COM (unknown [162.221.156.55]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id ecc43376-5ae6-11e9-9b6d-2f6f6651b6ba; Tue, 09 Apr 2019 16:45:47 +0000 (UTC) X-IronPort-AV: E=Sophos;i="5.60,330,1549929600"; d="scan'208";a="83137611" From: Anthony PERARD To: Date: Tue, 9 Apr 2019 17:45:37 +0100 Message-ID: <20190409164542.30274-5-anthony.perard@citrix.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190409164542.30274-1-anthony.perard@citrix.com> References: <20190409164542.30274-1-anthony.perard@citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH 4/9] libxl_internal: Create new lock for devices hotplug via QMP X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Anthony PERARD , Wei Liu , Ian Jackson Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP The current lock `domain_userdata_lock' can't be used when modification to a guest is done by sending command to QEMU, this is a slow process and requires to call CTX_UNLOCK, which is not possible while holding the `domain_userdata_lock'. To resolve this issue, we create a new lock which can take over part of the job of the json_lock. Signed-off-by: Anthony PERARD --- Quote from Ian: > The invariant that we want to maintain is: > > * Nothing may exist in the primary config without > a corresponding entry in libxl-json. [...] > How about the following scheme. We split the libxl-json lock into > two. I'm going to call them the fast lock and the slow lock. > > * The fast lock is the existing libxl-json lock. > > * The slow lock is outside the libxl-json lock in the lock > hierarchy. It is also outside the libxl_ctx lock. It is > to be acquired by an ao event callback. > > * No-one may read or edit the libxl-json without holding the fast > lock across their read operation, or their read/modify/write > cycle. > > * However, there are special rules for thing removal/addition, for > things added/removed via qmp. Call these `qmp things'. It is > permissible to add or remove a qmp thing across two separate > acquisitions of the fast lock, one to read the old state of the > thing, and one to read/modify/write to update (only) the new state > of the thing. This is subject to the thing add/removal rule, from > before, which becomes: > > * You may not cause a thing to be added to the primary config > unless you have held the relevant thing lock continuously > since ensuring that the libxl-json config describes it. > > * Conversely you may not cause a thing to be removed from the > libxl-json unless you have held the relevant thing lock > continuously since ensuring the thing is absent from the primary > config. > > * The `relevant thing lock' is the slow lock for qmp things, and the > fast lock for other things. > > * Acquiring the fast lock fails for a destroyed domain, as at > present. --- tools/libxl/libxl_internal.c | 27 +++++++++++++++++++++++++ tools/libxl/libxl_internal.h | 39 ++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index fa0bbc3960..db281ac259 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -607,6 +607,33 @@ void libxl__update_domain_configuration(libxl__gc *gc, dst->b_info.video_memkb = src->b_info.video_memkb; } +struct libxl__domain_qmp_lock { + libxl__generic_lock lock; +}; + +libxl__domain_qmp_lock *libxl__lock_domain_qmp(libxl__gc *gc, + libxl_domid domid) +{ + libxl__domain_qmp_lock *lock = NULL; + int rc; + + lock = libxl__zalloc(NOGC, sizeof(*lock)); + rc = libxl__lock_generic(gc, domid, &lock->lock, + "libxl-device-changes-lock"); + if (rc) { + libxl__unlock_domain_qmp(lock); + return NULL; + } + + return lock; +} + +void libxl__unlock_domain_qmp(libxl__domain_qmp_lock *lock) +{ + libxl__unlock_generic(&lock->lock); + free(lock); +} + /* * Local variables: * mode: C diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index f1aefaf98a..43b44f2c30 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2561,6 +2561,21 @@ typedef struct libxl__ao_device libxl__ao_device; typedef struct libxl__multidev libxl__multidev; typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*); +/* + * Lock for device hotplug, qmp_lock. + * + * This lock is outside the json_lock lock in lock hierarchy. + * It is also outside the libxl_ctx lock. + * It is to be acquired by an ao event callback. + * + * It is to be acquired when adding/removing devices or making changes + * to them via QMP, when this is a slow operation. + */ +typedef struct libxl__domain_qmp_lock libxl__domain_qmp_lock; +_hidden libxl__domain_qmp_lock *libxl__lock_domain_qmp(libxl__gc *gc, + libxl_domid domid); +_hidden void libxl__unlock_domain_qmp(libxl__domain_qmp_lock *lock); + /* This functions sets the necessary libxl__ao_device struct values to use * safely inside functions. It marks the operation as "active" * since we need to be sure that all device status structs are set @@ -2749,11 +2764,11 @@ struct libxl__multidev { * device information, in JSON files, so that we can use this JSON * file as a template to reconstruct domain configuration. * - * In essense there are now two views of device state, one is xenstore, - * the other is JSON file. We use xenstore as primary reference. + * In essense there are now two views of device state, one is the + * primary config (xenstore or QEMU), the other is JSON file. * - * Here we maintain one invariant: every device in xenstore must have - * an entry in JSON file. + * Here we maintain one invariant: every device in the primary config + * must have an entry in JSON file. * * All device hotplug routines should comply to following pattern: * lock json config (json_lock) @@ -2768,6 +2783,22 @@ struct libxl__multidev { * end for loop * unlock json config * + * Or in case QEMU is the primary config, this pattern can be use: + * lock qmp (qmp_lock) + * lock json config (json_lock) + * read json config + * update in-memory json config with new entry, replacing + * any stale entry + * unlock json config + * apply new config to primary config + * lock json config (json_lock) + * read json config + * update in-memory json config with new entry, replacing + * any stale entry + * write in-memory json config to disk + * unlock json config + * unlock qmp + * * Device removal routines are not touched. * * Here is the proof that we always maintain that invariant and we