From nobody Fri Apr 26 16:28:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1555090045; cv=none; d=zoho.com; s=zohoarc; b=jQpVPuHMRKy/D6DBTDD8onVAdPoyPSUhE7UQEOTPUpNOVn9g9a6UtR0EC3LP00iMLCc0cvvsL+Apg5dwu1Eo1W8qt/IY+VKaSN9JomEA4nyHoTfQf/cA7mJydUuw2S/HmwTDX08CLxjjTWlZVUhpTMjhJvAAZvQ/Ueh/9o8JL64= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1555090045; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=J32m46eQUGfs2AbCqzhwKLRxHCr77/9A+1T4EUQQuCc=; b=P/GuuugxtowiT5Vz0k6S+9+DNn3voh8ZjQ7tehxTaTVCUujb02gPuGorKIZiuBrmeuogm36E//A41jnYse2rdgfBdlApwjgW6XQL7+FSG/JhD2VkVlbXtecr3zCVZ6fY7cr5xjmC08+dkF1tGkuMVHirf/bxULgq05/juPpu4p0= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1555090045931931.8608710597346; Fri, 12 Apr 2019 10:27:25 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8123B612A2; Fri, 12 Apr 2019 17:27:23 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 52D711001E85; Fri, 12 Apr 2019 17:27:23 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 0F435181AC46; Fri, 12 Apr 2019 17:27:23 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x3CHRGXg020343 for ; Fri, 12 Apr 2019 13:27:16 -0400 Received: by smtp.corp.redhat.com (Postfix) id D297217F9D; Fri, 12 Apr 2019 17:27:16 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-38.phx2.redhat.com [10.3.117.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5D1EC17794; Fri, 12 Apr 2019 17:27:13 +0000 (UTC) From: Laine Stump To: libvir-list@redhat.com Date: Fri, 12 Apr 2019 13:26:57 -0400 Message-Id: <20190412172658.3132-2-laine@laine.org> In-Reply-To: <20190412172658.3132-1-laine@laine.org> References: <20190412172658.3132-1-laine@laine.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 1/2] util: eliminate duplicate function virDBusMessageRead X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 12 Apr 2019 17:27:24 +0000 (UTC) Content-Type: text/plain; charset="utf-8" When virDBusMessageRead() and virDBusMessageDecode were first added in commit 834c9c94, they were identical except that virDBusMessageRead() would unref the message after decoding it. This difference was eliminated later in commit dc7f3ffc after it became apparent that unref-ing the message so soon was never the right thing to do. The two identical functions remained though, with the tests and virDBus library itself calling the Decode variant, and all other users calling the Read variant. This patch eliminates the duplication, switching all users to virDBusMessageDecode (and moving the nice API documentation comment from the Read function up to the Decode function). Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrang=C3=A9 --- src/libvirt_private.syms | 1 - src/util/virdbus.c | 53 +++++++++++++--------------------------- src/util/virdbus.h | 4 +-- src/util/virdbuspriv.h | 4 --- src/util/virfirewalld.c | 8 +++--- src/util/virpolkit.c | 12 ++++----- src/util/virsystemd.c | 6 ++--- tests/virpolkittest.c | 24 +++++++++--------- 8 files changed, 44 insertions(+), 68 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6e64e77839..f7e99d3bf1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1750,7 +1750,6 @@ virDBusGetSystemBus; virDBusHasSystemBus; virDBusMessageDecode; virDBusMessageEncode; -virDBusMessageRead; virDBusMessageUnref; virDBusSetSharedBus; =20 diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 6725e0edb0..b0ac8d7055 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1228,9 +1228,23 @@ int virDBusMessageEncode(DBusMessage* msg, } =20 =20 -int virDBusMessageDecode(DBusMessage* msg, - const char *types, - ...) +/** + * virDBusMessageDecode: + * @msg: the reply to decode + * @types: type signature for following return values + * @...: pointers in which to store return values + * + * The @types type signature is the same format as + * that used for the virDBusCallMethod. The difference + * is that each variadic parameter must be a pointer to + * be filled with the values. eg instead of passing an + * 'int', pass an 'int *'. + * + */ +int +virDBusMessageDecode(DBusMessage* msg, + const char *types, + ...) { int ret; va_list args; @@ -1654,32 +1668,6 @@ int virDBusCallMethod(DBusConnection *conn, } =20 =20 -/** - * virDBusMessageRead: - * @msg: the reply to decode - * @types: type signature for following return values - * @...: pointers in which to store return values - * - * The @types type signature is the same format as - * that used for the virDBusCallMethod. The difference - * is that each variadic parameter must be a pointer to - * be filled with the values. eg instead of passing an - * 'int', pass an 'int *'. - * - */ -int virDBusMessageRead(DBusMessage *msg, - const char *types, ...) -{ - va_list args; - int ret; - - va_start(args, types); - ret =3D virDBusMessageDecodeArgs(msg, types, args); - va_end(args); - - return ret; -} - static int virDBusIsServiceInList(const char *listMethod, const char *name) { DBusConnection *conn; @@ -1854,13 +1842,6 @@ int virDBusCallMethod(DBusConnection *conn ATTRIBUTE= _UNUSED, return -1; } =20 -int virDBusMessageRead(DBusMessage *msg ATTRIBUTE_UNUSED, - const char *types ATTRIBUTE_UNUSED, ...) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("DBus support not compiled into this binary")); - return -1; -} =20 int virDBusMessageEncode(DBusMessage* msg ATTRIBUTE_UNUSED, const char *types ATTRIBUTE_UNUSED, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 268a42afd4..c1e5de31ab 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -67,8 +67,8 @@ int virDBusCallMethod(DBusConnection *conn, const char *iface, const char *member, const char *types, ...); -int virDBusMessageRead(DBusMessage *msg, - const char *types, ...); +int virDBusMessageDecode(DBusMessage *msg, + const char *types, ...); void virDBusMessageUnref(DBusMessage *msg); =20 int virDBusIsServiceEnabled(const char *name); diff --git a/src/util/virdbuspriv.h b/src/util/virdbuspriv.h index 4f0140c520..5c0c7f8b6e 100644 --- a/src/util/virdbuspriv.h +++ b/src/util/virdbuspriv.h @@ -56,8 +56,4 @@ int virDBusMessageEncode(DBusMessage* msg, const char *types, ...); =20 -int virDBusMessageDecode(DBusMessage* msg, - const char *types, - ...); - #endif /* LIBVIRT_VIRDBUSPRIV_H */ diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c index 5d83c3ea2b..60b75e7e25 100644 --- a/src/util/virfirewalld.c +++ b/src/util/virfirewalld.c @@ -102,7 +102,7 @@ virFirewallDGetVersion(unsigned long *version) "version") < 0) goto cleanup; =20 - if (virDBusMessageRead(reply, "v", "s", &versionStr) < 0) + if (virDBusMessageDecode(reply, "v", "s", &versionStr) < 0) goto cleanup; =20 if (virParseVersionString(versionStr, version, false) < 0) { @@ -163,7 +163,7 @@ virFirewallDGetBackend(void) goto cleanup; } =20 - if (virDBusMessageRead(reply, "v", "s", &backendStr) < 0) + if (virDBusMessageDecode(reply, "v", "s", &backendStr) < 0) goto cleanup; =20 VIR_DEBUG("FirewallD backend: %s", backendStr); @@ -216,7 +216,7 @@ virFirewallDGetZones(char ***zones, size_t *nzones) NULL) < 0) goto cleanup; =20 - if (virDBusMessageRead(reply, "a&s", nzones, zones) < 0) + if (virDBusMessageDecode(reply, "a&s", nzones, zones) < 0) goto cleanup; =20 ret =3D 0; @@ -337,7 +337,7 @@ virFirewallDApplyRule(virFirewallLayer layer, goto cleanup; } } else { - if (virDBusMessageRead(reply, "s", output) < 0) + if (virDBusMessageDecode(reply, "s", output) < 0) goto cleanup; } =20 diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index 198439cea2..25eaad2c63 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -97,12 +97,12 @@ int virPolkitCheckAuth(const char *actionid, "" /* cancellation ID */) < 0) goto cleanup; =20 - if (virDBusMessageRead(reply, - "(bba&{ss})", - &is_authorized, - &is_challenge, - &nretdetails, - &retdetails) < 0) + if (virDBusMessageDecode(reply, + "(bba&{ss})", + &is_authorized, + &is_challenge, + &nretdetails, + &retdetails) < 0) goto cleanup; =20 for (i =3D 0; i < (nretdetails / 2); i++) { diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index b868f88a59..3f03e3bd63 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -182,7 +182,7 @@ virSystemdGetMachineNameByPID(pid_t pid) "u", pid) < 0) goto cleanup; =20 - if (virDBusMessageRead(reply, "o", &object) < 0) + if (virDBusMessageDecode(reply, "o", &object) < 0) goto cleanup; =20 virDBusMessageUnref(reply); @@ -201,7 +201,7 @@ virSystemdGetMachineNameByPID(pid_t pid) "Name") < 0) goto cleanup; =20 - if (virDBusMessageRead(reply, "v", "s", &name) < 0) + if (virDBusMessageDecode(reply, "v", "s", &name) < 0) goto cleanup; =20 VIR_DEBUG("Domain with pid %lld has machine name '%s'", @@ -533,7 +533,7 @@ virSystemdPMSupportTarget(const char *methodName, bool = *result) NULL) < 0) return ret; =20 - if ((ret =3D virDBusMessageRead(message, "s", &response)) < 0) + if ((ret =3D virDBusMessageDecode(message, "s", &response)) < 0) goto cleanup; =20 *result =3D STREQ("yes", response) || STREQ("challenge", response); diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c index 25e759d703..94a6daae0c 100644 --- a/tests/virpolkittest.c +++ b/tests/virpolkittest.c @@ -72,18 +72,18 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_= and_block, int is_authorized =3D 1; int is_challenge =3D 0; =20 - if (virDBusMessageRead(message, - "(sa{sv})sa&{ss}us", - &type, - 3, - &pidkey, "u", &pidval, - &timekey, "t", &timeval, - &uidkey, "i", &uidval, - &actionid, - &detailslen, - &details, - &allowInteraction, - &cancellationId) < 0) + if (virDBusMessageDecode(message, + "(sa{sv})sa&{ss}us", + &type, + 3, + &pidkey, "u", &pidval, + &timekey, "t", &timeval, + &uidkey, "i", &uidval, + &actionid, + &detailslen, + &details, + &allowInteraction, + &cancellationId) < 0) goto error; =20 if (STREQ(actionid, "org.libvirt.test.success")) { --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 16:28:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1555090049; cv=none; d=zoho.com; s=zohoarc; b=IvFzsUr5Ahm4qbecTt2obmaTnFU6DdAcTkQc7C5xOFE9Sg4L2QdxCSqWbrg3rv4MS4mn/g405D0LjyHJBtKbsBt7xUP0/FdBeEwcEMR1cC4KbBTylS+vUd/ns4K7B25guErkFh1ZCFFvG2V7us+25WLWqD+g5uKs74LyxNuPQNU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1555090049; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=j7tkA/XQbdo+SniLwcnZ3b3Wmzyfd+ETQ1ZIySuPQoM=; b=jxvwrFxI9+7+HOuQ/LHtUbcnLkDJMg1TyfvNNIvJ6IB0kMi3gi5QMRXWORe71tUDxfBJF5a4Z3Xye82UbbGubwSSGavoEZlbaQ+zothiuUGSzfzgkT0pVB9NdVgmGAFuAuAAi/+BSPEAiG7rIfLyPCi7KAwrtcsZBrcEI0NnoT4= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1555090049465617.0009648630752; Fri, 12 Apr 2019 10:27:29 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 62BE181F22; Fri, 12 Apr 2019 17:27:27 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 409335C21E; Fri, 12 Apr 2019 17:27:27 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 078F2181AC8E; Fri, 12 Apr 2019 17:27:27 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x3CHRHww020348 for ; Fri, 12 Apr 2019 13:27:17 -0400 Received: by smtp.corp.redhat.com (Postfix) id 698E918505; Fri, 12 Apr 2019 17:27:17 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-38.phx2.redhat.com [10.3.117.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0C86F2633D; Fri, 12 Apr 2019 17:27:16 +0000 (UTC) From: Laine Stump To: libvir-list@redhat.com Date: Fri, 12 Apr 2019 13:26:58 -0400 Message-Id: <20190412172658.3132-3-laine@laine.org> In-Reply-To: <20190412172658.3132-1-laine@laine.org> References: <20190412172658.3132-1-laine@laine.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 2/2] network: only reload firewall after firewalld is finished restarting X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 12 Apr 2019 17:27:27 +0000 (UTC) Content-Type: text/plain; charset="utf-8" The network driver used to reload the firewall rules whenever a dbus NameOwnerChanged message for org.fedoraproject.FirewallD1 was received. Presumably at some point in the past this was successful at reloading our rules after a firewalld restart. Recently though I noticed that once firewalld was restarted, libvirt's logs would get this message: The name org.fedoraproject.FirewallD1 was not provided by any .service fi= les After this point, no networks could be started until libvirtd itself was restarted. The problem is that the NameOwnerChanged message is sent twice during a firewalld restart - once when the old firewalld is stopped, and again when the new firewalld is started. If we try to reload at the point the old firewalld is stopped, none of the firewalld dbus calls will succeed. The solution is to check the new_owner field of the message - we should reload our firewall rules only if new_owner is non-empty (it is set to "" when firewalld is stopped, and some sort of epoch number when it is again started). Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrang=C3=A9 --- src/network/bridge_driver.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4d4ab0f375..a091ed7181 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -543,12 +543,32 @@ firewalld_dbus_filter_bridge(DBusConnection *connecti= on ATTRIBUTE_UNUSED, void *user_data) { virNetworkDriverStatePtr driver =3D user_data; + bool reload =3D false; =20 - if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, - "NameOwnerChanged") || - dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", - "Reloaded")) - { + if (dbus_message_is_signal(message, + "org.fedoraproject.FirewallD1", "Reloaded")= ) { + reload =3D true; + + } else if (dbus_message_is_signal(message, + DBUS_INTERFACE_DBUS, "NameOwnerChang= ed")) { + + VIR_AUTOFREE(char *) name =3D NULL; + VIR_AUTOFREE(char *) old_owner =3D NULL; + VIR_AUTOFREE(char *) new_owner =3D NULL; + + if (virDBusMessageDecode(message, "sss", &name, &old_owner, &new_o= wner) < 0) { + VIR_WARN("Failed to decode DBus NameOwnerChanged message"); + return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; + } + /* + * if new_owner is empty, firewalld is shutting down. If it is + * non-empty, then it is starting + */ + if (new_owner && *new_owner) + reload =3D true; + } + + if (reload) { VIR_DEBUG("Reload in bridge_driver because of firewalld."); networkReloadFirewallRules(driver, false); } --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list