From nobody Mon Feb 9 13:39:11 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org ARC-Seal: i=1; a=rsa-sha256; t=1560508761; cv=none; d=zoho.com; s=zohoarc; b=CJtkyOSd3R/qrVIFXdKGRpuHLvoJ1q5tsi3mNidBfcG29mKEhyPh+pcfEGgwgKrax0abG/GcepgbRBDFlBQ04UrcLqm24OPDmQMyWuhdGZaxO4eY46UxCBWKVniRAZg8o0bk8qU8f+AZEm++Gww/N1LNFfU9XvZQTWhenHRnn+s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1560508761; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=imPCwo0NbDpOyfk+TE1E7eia5hdmwTw2ejQkgMMRtn8=; b=L+WP7R8HqnI3QvS7Ev4PdPZQ72X0xvsiWSwB6R/ykiW2lkFByLbdb48CfqKHmeovaJFEKYFcJ9BF6YqDSnjrzrPa5b9mL+JYkh1S71z1G/m1OvJQRbXkPDXNjKZ0NPMChw5QS+ZgWelFb5nvGD78HFF9FS1ezsmQV0O6p5Q7J6I= ARC-Authentication-Results: i=1; mx.zoho.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1560508761966756.1170370487914; Fri, 14 Jun 2019 03:39:21 -0700 (PDT) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hbjb4-0002ly-BE; Fri, 14 Jun 2019 10:38:14 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hbjb2-0002kw-R9 for xen-devel@lists.xenproject.org; Fri, 14 Jun 2019 10:38:12 +0000 Received: from esa5.hc3370-68.iphmx.com (unknown [216.71.155.168]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTPS id 816b857d-8e90-11e9-8980-bc764e045a96; Fri, 14 Jun 2019 10:38:10 +0000 (UTC) X-Inumbo-ID: 816b857d-8e90-11e9-8980-bc764e045a96 Authentication-Results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=anthony.perard@citrix.com; spf=Pass smtp.mailfrom=anthony.perard@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Received-SPF: None (esa5.hc3370-68.iphmx.com: no sender authenticity information available from domain of anthony.perard@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa5.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa5.hc3370-68.iphmx.com: domain of anthony.perard@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa5.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ~all" Received-SPF: None (esa5.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa5.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: getYCXxD9POOaAOyC7G8S26zWJPs1CS8PJv96E8vEJWX7G38aNH7BdxzavYrRHX2cNcZ6mWyO8 j8Bi5I+tyLdukLL4/iURnhfbJAhOux2/WaQOimmEFZpF4ELNk0fgdowRjK7D1KmEMJ4BC9SXBF 7s+zOyWcflfg7683A7pLRI+KIC+hWW7vkeDYAbq1hmSe4fQE0m9UcZ9unm38aFCOdz8RpMdrO6 5KHdOVnxcdIXfhm90oCql0hKgNWakCMzmUs5zJlT4CXiKyhtbBGnRvALwZsKXM4Ay13n3Ny19F vuo= X-SBRS: 2.7 X-MesageID: 1722752 X-Ironport-Server: esa5.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.63,373,1557201600"; d="scan'208";a="1722752" From: Anthony PERARD To: Date: Fri, 14 Jun 2019 11:37:55 +0100 Message-ID: <20190614103801.22619-4-anthony.perard@citrix.com> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190614103801.22619-1-anthony.perard@citrix.com> References: <20190614103801.22619-1-anthony.perard@citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v2 3/9] libxl_internal: Introduce libxl__ev_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 , Ian Jackson , Wei Liu Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" 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. This lock is outside CTX_LOCK in the lock hierarchy. libxl__ev_lock_get will have CTX_UNLOCK before trying to grab the ev_lock. The callback is used to notify when the ev_lock have been acquired. Signed-off-by: Anthony PERARD --- Notes: Should libxl__ev_unlock() kill the child? =20 That would mean that we would need to handle the case where the process trying to grab the lock got impatient and decided that it was taking too long. =20 v2: - new patch, to replace 2 patch implement a different lock tools/libxl/libxl_internal.c | 174 +++++++++++++++++++++++++++++++++++ tools/libxl/libxl_internal.h | 76 ++++++++++++++- 2 files changed, 246 insertions(+), 4 deletions(-) diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index f492dae5ff..3906a0512d 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -575,6 +575,180 @@ void libxl__update_domain_configuration(libxl__gc *gc, dst->b_info.video_memkb =3D src->b_info.video_memkb; } =20 +void libxl__ev_lock_init(libxl__ev_lock *lock) +{ + libxl__ev_child_init(&lock->child); + lock->path =3D NULL; + lock->fd =3D -1; + lock->held =3D false; +} + +static void ev_lock_prepare_fork(libxl__egc *egc, libxl__ev_lock *lock); +static void ev_lock_child_callback(libxl__egc *egc, libxl__ev_child *child, + pid_t pid, int status); + +void libxl__ev_lock_get(libxl__egc *egc, libxl__ev_lock *lock) +{ + STATE_AO_GC(lock->ao); + const char *lockfile; + + lockfile =3D libxl__userdata_path(gc, lock->domid, + "libxl-device-changes-lock", "l"); + if (!lockfile) goto out; + lock->path =3D libxl__strdup(NOGC, lockfile); + + ev_lock_prepare_fork(egc, lock); + return; +out: + lock->callback(egc, lock, ERROR_LOCK_FAIL); +} + +static void ev_lock_prepare_fork(libxl__egc *egc, libxl__ev_lock *lock) +{ + STATE_AO_GC(lock->ao); + pid_t pid; + int fd; + + /* Convenience aliases */ + libxl_domid domid =3D lock->domid; + const char *lockfile =3D lock->path; + + lock->fd =3D open(lockfile, O_RDWR|O_CREAT, 0666); + if (lock->fd < 0) { + LOGED(ERROR, domid, "cannot open lockfile %s", lockfile); + goto out; + } + fd =3D lock->fd; + + pid =3D libxl__ev_child_fork(gc, &lock->child, ev_lock_child_callback); + if (pid < 0) + goto out; + if (!pid) { + /* child */ + int exit_val =3D 0; + + /* Lock the file in exclusive mode, wait indefinitely to + * acquire the lock */ + while (flock(fd, LOCK_EX)) { + switch (errno) { + case EINTR: + /* Signal received, retry */ + continue; + default: + /* All other errno: EBADF, EINVAL, ENOLCK, EWOULDBLOCK */ + LOGED(ERROR, domid, + "unexpected error while trying to lock %s, fd=3D%d, = errno=3D%d", + lockfile, fd, errno); + exit_val =3D 1; + break; + } + } + _exit(exit_val); + } + + /* Now that the child has the fd, set cloexec in the parent to prevent + * more leakage than necessary */ + libxl_fd_set_cloexec(CTX, fd, 1); + return; +out: + libxl__ev_unlock(gc, lock); + lock->callback(egc, lock, ERROR_LOCK_FAIL); +} + +static void ev_lock_child_callback(libxl__egc *egc, libxl__ev_child *child, + pid_t pid, int status) +{ + EGC_GC; + libxl__ev_lock *lock =3D CONTAINER_OF(child, *lock, child); + struct stat stab, fstab; + int rc =3D ERROR_LOCK_FAIL; + + /* Convenience aliases */ + int fd =3D lock->fd; + const char *lockfile =3D lock->path; + libxl_domid domid =3D lock->domid; + + if (status) { + libxl_report_child_exitstatus(CTX, XTL_ERROR, "flock child", + pid, status); + goto out; + } + + if (fstat(fd, &fstab)) { + LOGED(ERROR, domid, "cannot fstat %s, fd=3D%d", lockfile, fd); + goto out; + } + if (stat(lockfile, &stab)) { + if (errno !=3D ENOENT) { + LOGED(ERROR, domid, "cannot stat %s", lockfile); + goto out; + } + } else { + if (stab.st_dev =3D=3D fstab.st_dev && stab.st_ino =3D=3D fstab.st= _ino) { + /* We held the lock */ + lock->held =3D true; + rc =3D 0; + goto out; + } + } + + /* We didn't grab the lock, let's try again */ + flock(lock->fd, LOCK_UN); + close(lock->fd); + lock->fd =3D -1; + ev_lock_prepare_fork(egc, lock); + return; + +out: + if (lock->held) { + /* Check the domain is still there, if not we should release the + * lock and clean up. */ + if (libxl_domain_info(CTX, NULL, domid)) + rc =3D ERROR_LOCK_FAIL; + } + if (rc) { + LOGD(ERROR, domid, "Failed to grab qmp-lock"); + libxl__ev_unlock(gc, lock); + } + lock->callback(egc, lock, rc); +} + +void libxl__ev_unlock(libxl__gc *gc, libxl__ev_lock *lock) +{ + int r; + + assert(!libxl__ev_child_inuse(&lock->child)); + + /* It's important to unlink the file before releasing the lock to avoid + * the following race (if unlock/close before unlink): + * + * P1 LOCK P2 UNLOCK + * fd1 =3D open(lockfile) + * unlock(fd2) + * flock(fd1) + * fstat and stat check success + * unlink(lockfile) + * return lock + * + * In above case P1 thinks it has got hold of the lock but + * actually lock is released by P2 (lockfile unlinked). + */ + if (lock->path && lock->held) + unlink(lock->path); + + if (lock->fd >=3D 0) { + /* We need to call unlock as the fd may have leaked into other + * processes */ + r =3D flock(lock->fd, LOCK_UN); + if (r) + LOGED(ERROR, lock->domid, "failed to unlock fd=3D%d, path=3D%s= ", + lock->fd, lock->path); + close(lock->fd); + } + free(lock->path); + libxl__ev_lock_init(lock); +} + /* * Local variables: * mode: C diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index ca7206aaac..2c2476b55b 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -194,6 +194,7 @@ typedef struct libxl__osevent_hook_nexus libxl__osevent= _hook_nexus; typedef struct libxl__osevent_hook_nexi libxl__osevent_hook_nexi; typedef struct libxl__json_object libxl__json_object; typedef struct libxl__carefd libxl__carefd; +typedef struct libxl__ev_lock libxl__ev_lock; =20 typedef struct libxl__domain_create_state libxl__domain_create_state; typedef void libxl__domain_create_cb(struct libxl__egc *egc, @@ -2723,11 +2724,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) @@ -2742,6 +2743,24 @@ struct libxl__multidev { * end for loop * unlock json config * + * Or in case QEMU is the primary config, this pattern can be use: + * qmp_lock (libxl__ev_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_lock + * (CTX_LOCK can be acquired and released several time while holding the + * qmp_lock) + * * Device removal routines are not touched. * * Here is the proof that we always maintain that invariant and we @@ -4600,6 +4619,55 @@ static inline const char *libxl__qemu_qmp_path(libxl= __gc *gc, int domid) { return GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid); } + +/* + * Lock for device hotplug, qmp_lock. + * + * libxl__ev_lock implement a lock that is outside of CTX_LOCK in the + * lock hierarchy. It can be used when one want to make QMP calls to QEMU, + * which may take a significant amount time. + * It is to be acquired by an ao event callback. + * + * It is to be acquired when adding/removing devices or making changes + * to them when this is a slow operation and json_lock isn't appropriate. + * + * Possible states of libxl__ev_lock: + * Undefined + * Might contain anything. + * Idle + * Struct contents are defined enough to pass to any + * libxl__ev_lock_* function. + * The struct does not contain references to any allocated private + * resources so can be thrown away. + * Active + * Waiting to get a lock. + * Needs to wait until the callback is called. + * LockAcquired + * libxl__ev_unlock will need to be called to release the lock and the + * resources of libxl__ev_lock. + * + * libxl__ev_lock_init: Undefined/Idle -> Idle + * libxl__ev_lock_get: Idle -> Active + * May call callback synchronously. + * libxl__ev_unlock: LockAcquired/Idle -> Idle + * callback: When called: Active -> LockAcquired (on error: Idle) + * The callback is only called once. + */ +struct libxl__ev_lock { + /* filled by user */ + libxl__ao *ao; + libxl_domid domid; + void (*callback)(libxl__egc *, libxl__ev_lock *, int rc); + /* private to libxl__ev_lock* */ + libxl__ev_child child; + char *path; /* path of the lock file itself */ + int fd; + bool held; +}; +_hidden void libxl__ev_lock_init(libxl__ev_lock *); +_hidden void libxl__ev_lock_get(libxl__egc *, libxl__ev_lock *); +_hidden void libxl__ev_unlock(libxl__gc *, libxl__ev_lock *); + #endif =20 /* --=20 Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel