From patchwork Fri Oct 25 17:05:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 11212729 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 62F4F112B for ; Fri, 25 Oct 2019 17:06:34 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 32B8820867 for ; Fri, 25 Oct 2019 17:06:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="QzKjjxUx" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 32B8820867 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iO31X-0000FG-Tc; Fri, 25 Oct 2019 17:05:15 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iO31W-0000En-K3 for xen-devel@lists.xenproject.org; Fri, 25 Oct 2019 17:05:14 +0000 X-Inumbo-ID: 99df8d54-f749-11e9-8aca-bc764e2007e4 Received: from esa6.hc3370-68.iphmx.com (unknown [216.71.155.175]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 99df8d54-f749-11e9-8aca-bc764e2007e4; Fri, 25 Oct 2019 17:05:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1572023109; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=IF2jPDNtvh5oIUB6R7x7BJzJR24lsWGIp7A/ZN2ODJ8=; b=QzKjjxUx9pqLpR7R/SH715u58k+aMdj+CyB7S8uk2oJGxPJAv39zxhDq K42OhNetu2syMAV6kK62AyY/C8oTRAI3Ft3hkEgTts63fWymAAxeB3pSH z2vtZu86yDd5QjSkiwKgDci58qfudlwddJHEZdPBrfOIUo5qR9nwZHwM1 0=; Authentication-Results: esa6.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 (esa6.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=esa6.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa6.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=esa6.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 ip4:168.245.78.127 ~all" Received-SPF: None (esa6.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=esa6.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: 4b0kYyL+obajI34/AlfhAcEcam6FsYEfV6Ch+olJIm+0HHfpS0BcZL5xfXec/tIG2F/PUR8ZPK 0Xwquwk9b0LO4xMPYTu1l85LH7LuqvnUhaHRJ8foR0FNyTvxjjOIOkO1xIq1EbeOIYtDUgzruR d9pyhaBmQGe29bXLzfn6mlg+atEyIKPm2WNu4TntCIUFeVDp5Z+xbAw5vTlP5vKhPbMnFQAhuP hGqyzksVFUgFZQo9Lwmqkw9YpwRxjixJieQ5ouCmcVhW0h2QSr8VxejQnaXTUGWwPZhF80yRdU av4= X-SBRS: 2.7 X-MesageID: 7801961 X-Ironport-Server: esa6.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.68,229,1569297600"; d="scan'208";a="7801961" From: Anthony PERARD To: Date: Fri, 25 Oct 2019 18:05:05 +0100 Message-ID: <20191025170505.2834957-5-anthony.perard@citrix.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191025170505.2834957-1-anthony.perard@citrix.com> References: <20191025170505.2834957-1-anthony.perard@citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [RFC XEN PATCH for-4.13 4/4] libxl_qmp: Have a lock for QMP socket access 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 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" FIXME: The case where something failed when trying to acquired the lock isn't handled yet. This patch workaround the fact that it's not possible to connect multiple time to a single QMP socket. QEMU listen on the socket with a backlog value of 1, which mean that on Linux when concurrent thread call connect() on the socket, they get EAGAIN. To work around this, we use a new lock. Signed-off-by: Anthony PERARD --- tools/libxl/libxl_internal.h | 29 ++++++++++------ tools/libxl/libxl_qmp.c | 65 +++++++++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 23 deletions(-) diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index ef6655587b79..d650188586e9 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -364,6 +364,18 @@ struct libxl__ev_child { LIBXL_LIST_ENTRY(struct libxl__ev_child) entry; }; +struct libxl__ev_devlock { + /* filled by user */ + libxl__ao *ao; + libxl_domid domid; + void (*callback)(libxl__egc *, libxl__ev_devlock *, int rc); + /* private to libxl__ev_devlock* */ + libxl__ev_child child; + char *path; /* path of the lock file itself */ + int fd; + bool held; +}; + /* * QMP asynchronous calls * @@ -428,6 +440,8 @@ _hidden void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev); typedef enum { /* initial state */ qmp_state_disconnected = 1, + /* waiting for lock */ + qmp_state_waiting_lock, /* connected to QMP socket, waiting for greeting message */ qmp_state_connecting, /* qmp_capabilities command sent, waiting for reply */ @@ -461,6 +475,7 @@ struct libxl__ev_qmp { libxl__carefd *cfd; libxl__ev_fd efd; libxl__qmp_state state; + libxl__ev_qmplock lock; int id; int next_id; /* next id to use */ /* receive buffer */ @@ -4686,6 +4701,9 @@ static inline const char *libxl__qemu_qmp_path(libxl__gc *gc, int domid) * which may take a significant amount time. * It is to be acquired by an ao event callback. * + * If libxl__ev_devlock is needed, it should be acquired while every + * libxl__ev_qmp are Idle for the current domain. + * * 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. * @@ -4711,17 +4729,6 @@ static inline const char *libxl__qemu_qmp_path(libxl__gc *gc, int domid) * callback: When called: Active -> LockAcquired (on error: Idle) * The callback is only called once. */ -struct libxl__ev_devlock { - /* filled by user */ - libxl__ao *ao; - libxl_domid domid; - void (*callback)(libxl__egc *, libxl__ev_devlock *, int rc); - /* private to libxl__ev_devlock* */ - libxl__ev_child child; - char *path; /* path of the lock file itself */ - int fd; - bool held; -}; _hidden void libxl__ev_devlock_init(libxl__ev_devlock *); _hidden void libxl__ev_devlock_lock(libxl__egc *, libxl__ev_devlock *); _hidden void libxl__ev_devlock_unlock(libxl__gc *, libxl__ev_devlock *); diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c index f0e0b50bd1c5..1ac50a95a42d 100644 --- a/tools/libxl/libxl_qmp.c +++ b/tools/libxl/libxl_qmp.c @@ -1084,6 +1084,7 @@ static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev, * * qmp_state External cfd efd id rx_buf* tx_buf* msg* * disconnected Idle NULL Idle reset free free free + * waiting_lock Active open Idle reset used free set * connecting Active open IN reset used free set * cap.neg Active open IN|OUT sent used cap_neg set * cap.neg Active open IN sent used free set @@ -1153,6 +1154,10 @@ static void qmp_ev_ensure_reading_writing(libxl__gc *gc, libxl__ev_qmp *ev) { short events = POLLIN; + if (ev->state == qmp_state_waiting_lock) + /* We can't modifie the efd yet, as it isn't registered. */ + return; + if (ev->tx_buf) events |= POLLOUT; else if ((ev->state == qmp_state_waiting_reply) && ev->msg) @@ -1168,9 +1173,13 @@ static void qmp_ev_set_state(libxl__gc *gc, libxl__ev_qmp *ev, switch (new_state) { case qmp_state_disconnected: break; - case qmp_state_connecting: + case qmp_state_waiting_lock: assert(ev->state == qmp_state_disconnected); break; + case qmp_state_connecting: + assert(ev->state == qmp_state_disconnected || + ev->state == qmp_state_waiting_lock); + break; case qmp_state_capability_negotiation: assert(ev->state == qmp_state_connecting); break; @@ -1231,20 +1240,20 @@ static int qmp_error_class_to_libxl_error_code(libxl__gc *gc, /* Setup connection */ -static int qmp_ev_connect(libxl__gc *gc, libxl__ev_qmp *ev) +static void qmp_ev_lock_aquired(libxl__egc *, libxl__ev_qmplock *, int rc); + +static int qmp_ev_connect(libxl__egc *egc, libxl__ev_qmp *ev) /* disconnected -> connecting but with `msg` free * on error: broken */ { + EGC_GC; int fd; - int rc, r; - struct sockaddr_un un; - const char *qmp_socket_path; - - assert(ev->state == qmp_state_disconnected); + int rc; - qmp_socket_path = libxl__qemu_qmp_path(gc, ev->domid); + /* Convenience aliases */ + libxl__ev_qmplock *lock = &ev->lock; - LOGD(DEBUG, ev->domid, "Connecting to %s", qmp_socket_path); + assert(ev->state == qmp_state_disconnected); libxl__carefd_begin(); fd = socket(AF_UNIX, SOCK_STREAM, 0); @@ -1258,6 +1267,35 @@ static int qmp_ev_connect(libxl__gc *gc, libxl__ev_qmp *ev) if (rc) goto out; + qmp_ev_set_state(gc, ev, qmp_state_waiting_lock); + + lock->ao = ev->ao; + lock->domid = ev->domid; + lock->callback = qmp_ev_lock_aquired; + libxl__ev_qmplock_lock(egc, &ev->lock); + + return 0; + +out: + return rc; +} + +static void qmp_ev_lock_aquired(libxl__egc *egc, libxl__ev_qmplock *lock, + int rc) +{ + libxl__ev_qmp *ev = CONTAINER_OF(lock, *ev, lock); + EGC_GC; + const char *qmp_socket_path; + struct sockaddr_un un; + int r; + + if (rc) + goto out; + + qmp_socket_path = libxl__qemu_qmp_path(gc, ev->domid); + + LOGD(DEBUG, ev->domid, "Connecting to %s", qmp_socket_path); + rc = libxl__prepare_sockaddr_un(gc, &un, qmp_socket_path, "QMP socket"); if (rc) @@ -1279,10 +1317,10 @@ static int qmp_ev_connect(libxl__gc *gc, libxl__ev_qmp *ev) qmp_ev_set_state(gc, ev, qmp_state_connecting); - return 0; + return; out: - return rc; + LOGD(ERROR, ev->domid, "connect failed"); } /* QMP FD callbacks */ @@ -1779,6 +1817,8 @@ void libxl__ev_qmp_init(libxl__ev_qmp *ev) ev->qemu_version.major = -1; ev->qemu_version.minor = -1; ev->qemu_version.micro = -1; + + libxl__ev_qmplock_init(&ev->lock); } int libxl__ev_qmp_send(libxl__egc *egc, libxl__ev_qmp *ev, @@ -1798,7 +1838,7 @@ int libxl__ev_qmp_send(libxl__egc *egc, libxl__ev_qmp *ev, /* Connect to QEMU if not already connected */ if (ev->state == qmp_state_disconnected) { - rc = qmp_ev_connect(gc, ev); + rc = qmp_ev_connect(egc, ev); if (rc) goto error; } @@ -1830,6 +1870,7 @@ void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev) libxl__ev_fd_deregister(gc, &ev->efd); libxl__carefd_close(ev->cfd); + libxl__ev_qmplock_dispose(gc, &ev->lock); libxl__ev_qmp_init(ev); }