On a Thursday in 2020, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
---
src/util/virfirewalld.c | 197 ++++++++++++++++----------------
tests/meson.build | 4 +-
tests/networkxml2firewalltest.c | 39 ++++---
tests/virfirewalltest.c | 154 ++++++++++---------------
4 files changed, 180 insertions(+), 214 deletions(-)
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index c14a6b6e65..69c8b73da0 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -196,9 +195,9 @@ virFirewallDGetBackend(void)
int
virFirewallDGetZones(char ***zones, size_t *nzones)
{
- DBusConnection *sysbus = virDBusGetSystemBus();
- DBusMessage *reply = NULL;
- int ret = -1;
+ GDBusConnection *sysbus = virGDBusGetSystemBus();
+ g_autoptr(GVariant) reply = NULL;
+ g_autoptr(GVariant) array = NULL;
*nzones = 0;
*zones = NULL;
@@ -206,23 +205,20 @@ virFirewallDGetZones(char ***zones, size_t *nzones)
if (!sysbus)
return -1;
- if (virDBusCallMethod(sysbus,
- &reply,
- NULL,
- VIR_FIREWALL_FIREWALLD_SERVICE,
- "/org/fedoraproject/FirewallD1",
- "org.fedoraproject.FirewallD1.zone",
- "getZones",
- NULL) < 0)
- goto cleanup;
+ if (virGDBusCallMethod(sysbus,
+ &reply,
+ NULL,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1",
+ "org.fedoraproject.FirewallD1.zone",
+ "getZones",
+ NULL) < 0)
+ return -1;
- if (virDBusMessageDecode(reply, "a&s", nzones, zones) < 0)
- goto cleanup;
+ g_variant_get(reply, "(@as)", array);
Throughout the series you're not checking return values of g_variant_get. I'm getting assertion errors with firewalld disabled: (process:156524): GLib-CRITICAL **: 09:56:49.398: g_variant_get_type: assertion 'value != NULL' failed (process:156524): GLib-CRITICAL **: 09:56:49.399: g_variant_type_is_subtype_of: assertion 'g_variant_type_check (type)' failed (process:156524): GLib-CRITICAL **: 09:56:49.399: g_variant_dup_strv: assertion 'g_variant_is_of_type (value, G_VARIANT_TYPE_STRING_ARRAY)' failed Jano
+ *zones = g_variant_dup_strv(array, nzones);
- ret = 0;
- cleanup:
- virDBusMessageUnref(reply);
- return ret;
+ return 0;
}
@@ -273,10 +269,10 @@ virFirewallDApplyRule(virFirewallLayer layer,
char **output)
{
const char *ipv = virFirewallLayerFirewallDTypeToString(layer);
- DBusConnection *sysbus = virDBusGetSystemBus();
- DBusMessage *reply = NULL;
- virError error;
- int ret = -1;
+ GDBusConnection *sysbus = virGDBusGetSystemBus();
+ g_autoptr(GVariant) message = NULL;
+ g_autoptr(GVariant) reply = NULL;
+ g_autoptr(virError) error = NULL;
if (!sysbus)
return -1;
@@ -287,23 +283,27 @@ virFirewallDApplyRule(virFirewallLayer layer,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown firewall layer %d"),
layer);
- goto cleanup;
+ return -1;
}
- if (virDBusCallMethod(sysbus,
- &reply,
- &error,
- VIR_FIREWALL_FIREWALLD_SERVICE,
- "/org/fedoraproject/FirewallD1",
- "org.fedoraproject.FirewallD1.direct",
- "passthrough",
- "sa&s",
- ipv,
- (int)argsLen,
- args) < 0)
- goto cleanup;
+ if (VIR_ALLOC(error) < 0)
+ return -1;
- if (error.level == VIR_ERR_ERROR) {
+ message = g_variant_new("(s@as)",
+ ipv,
+ g_variant_new_strv((const char * const*)args, argsLen));
+
+ if (virGDBusCallMethod(sysbus,
+ &reply,
+ error,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1",
+ "org.fedoraproject.FirewallD1.direct",
+ "passthrough",
+ message) < 0)
+ return -1;
+
+ if (error->level == VIR_ERR_ERROR) {
/*
* As of firewalld-0.3.9.3-1.fc20.noarch the name and
* message fields in the error look like
@@ -331,22 +331,16 @@ virFirewallDApplyRule(virFirewallLayer layer,
*/
if (ignoreErrors) {
VIR_DEBUG("Ignoring error '%s': '%s'",
- error.str1, error.message);
+ error->str1, error->message);
} else {
- virReportErrorObject(&error);
- goto cleanup;
+ virReportErrorObject(error);
+ return -1;
}
} else {
- if (virDBusMessageDecode(reply, "s", output) < 0)
- goto cleanup;
+ g_variant_get(reply, "(s)", output);
}
- ret = 0;
-
- cleanup:
- virResetError(&error);
- virDBusMessageUnref(reply);
- return ret;
+ return 0;
}
@@ -354,19 +348,20 @@ int
virFirewallDInterfaceSetZone(const char *iface,
const char *zone)
{
- DBusConnection *sysbus = virDBusGetSystemBus();
+ GDBusConnection *sysbus = virGDBusGetSystemBus();
+ g_autoptr(GVariant) message = NULL;
if (!sysbus)
return -1;
- return virDBusCallMethod(sysbus,
+ message = g_variant_new("(ss)", zone, iface);
+
+ return virGDBusCallMethod(sysbus,
NULL,
NULL,
VIR_FIREWALL_FIREWALLD_SERVICE,
"/org/fedoraproject/FirewallD1",
"org.fedoraproject.FirewallD1.zone",
"changeZoneOfInterface",
- "ss",
- zone,
- iface);
+ message);
}
diff --git a/tests/meson.build b/tests/meson.build
index 75bfb3effe..2c4f044d30 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -314,7 +314,7 @@ tests += [
{ 'name': 'virerrortest' },
{ 'name': 'virfilecachetest' },
{ 'name': 'virfiletest' },
- { 'name': 'virfirewalltest', 'deps': [ dbus_dep ] },
+ { 'name': 'virfirewalltest' },
{ 'name': 'virhashtest' },
{ 'name': 'virhostcputest', 'link_whole': [ test_file_wrapper_lib ] },
{ 'name': 'virhostdevtest' },
@@ -400,7 +400,7 @@ endif
if conf.has('WITH_NETWORK')
tests += [
{ 'name': 'networkxml2conftest', 'link_with': [ network_driver_impl ] },
- { 'name': 'networkxml2firewalltest', 'link_with': [ network_driver_impl ], 'deps': [ dbus_dep ] },
+ { 'name': 'networkxml2firewalltest', 'link_with': [ network_driver_impl ] },
{ 'name': 'networkxml2xmltest', 'link_with': [ network_driver_impl ] },
]
endif
diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
index 80e2d6a035..e0244f508e 100644
--- a/tests/networkxml2firewalltest.c
+++ b/tests/networkxml2firewalltest.c
@@ -26,9 +26,7 @@
#if defined (__linux__)
-# if WITH_DBUS
-# include <dbus/dbus.h>
-# endif
+# include <gio/gio.h>
# include "network/bridge_driver_platform.h"
# include "virbuffer.h"
@@ -48,21 +46,30 @@
# error "test case not ported to this platform"
# endif
-# if WITH_DBUS
-VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
- DBusMessage *,
- DBusConnection *, connection,
- DBusMessage *, message,
- int, timeout_milliseconds,
- DBusError *, error)
+VIR_MOCK_WRAP_RET_ARGS(g_dbus_connection_call_sync,
+ GVariant *,
+ GDBusConnection *, connection,
+ const gchar *, bus_name,
+ const gchar *, object_path,
+ const gchar *, interface_name,
+ const gchar *, method_name,
+ GVariant *, parameters,
+ const GVariantType *, reply_type,
+ GDBusCallFlags, flags,
+ gint, timeout_msec,
+ GCancellable *, cancellable,
+ GError **, error)
{
- VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
+ if (parameters)
+ g_variant_unref(parameters);
- dbus_set_error_const(error, "org.freedesktop.error", "dbus is disabled");
+ VIR_MOCK_REAL_INIT(g_dbus_connection_call_sync);
+
+ *error = g_dbus_error_new_for_dbus_error("org.freedesktop.error",
+ "dbus is disabled");
return NULL;
}
-# endif
static void
testCommandDryRun(const char *const*args G_GNUC_UNUSED,
@@ -197,11 +204,7 @@ mymain(void)
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
-# if WITH_DBUS
-VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdbus"))
-# else
-VIR_TEST_MAIN(mymain)
-# endif
+VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virgdbus"))
#else /* ! defined (__linux__) */
diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index ce252bd0e0..607638e9d0 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -22,6 +22,8 @@
#if defined(__linux__)
+# include <gio/gio.h>
+
# include "virbuffer.h"
# define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW
# include "vircommandpriv.h"
@@ -30,15 +32,9 @@
# define LIBVIRT_VIRFIREWALLDPRIV_H_ALLOW
# include "virfirewalldpriv.h"
# include "virmock.h"
-# define LIBVIRT_VIRDBUSPRIV_H_ALLOW
-# include "virdbuspriv.h"
# define VIR_FROM_THIS VIR_FROM_FIREWALL
-# if WITH_DBUS
-# include <dbus/dbus.h>
-# endif
-
static bool fwDisabled = true;
static virBufferPtr fwBuf;
static bool fwError;
@@ -66,64 +62,64 @@ static bool fwError;
"Chain POSTROUTING (policy ACCEPT)\n" \
"target prot opt source destination\n"
-# if WITH_DBUS
-VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
- DBusMessage *,
- DBusConnection *, connection,
- DBusMessage *, message,
- int, timeout_milliseconds,
- DBusError *, error)
+VIR_MOCK_WRAP_RET_ARGS(g_dbus_connection_call_sync,
+ GVariant *,
+ GDBusConnection *, connection,
+ const gchar *, bus_name,
+ const gchar *, object_path,
+ const gchar *, interface_name,
+ const gchar *, method_name,
+ GVariant *, parameters,
+ const GVariantType *, reply_type,
+ GDBusCallFlags, flags,
+ gint, timeout_msec,
+ GCancellable *, cancellable,
+ GError **, error)
{
- DBusMessage *reply = NULL;
- const char *service = dbus_message_get_destination(message);
- const char *member = dbus_message_get_member(message);
- size_t i;
- size_t nargs = 0;
- char **args = NULL;
- char *type = NULL;
+ GVariant *reply = NULL;
+ g_autoptr(GVariant) params = parameters;
- VIR_MOCK_REAL_INIT(dbus_connection_send_with_reply_and_block);
+ VIR_MOCK_REAL_INIT(g_dbus_connection_call_sync);
- if (STREQ(service, "org.freedesktop.DBus") &&
- STREQ(member, "ListNames")) {
- const char *svc1 = "org.foo.bar.wizz";
- const char *svc2 = VIR_FIREWALL_FIREWALLD_SERVICE;
- DBusMessageIter iter;
- DBusMessageIter sub;
- reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
- dbus_message_iter_init_append(reply, &iter);
- dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
- "s", &sub);
+ if (STREQ(bus_name, "org.freedesktop.DBus") &&
+ STREQ(method_name, "ListNames")) {
+ GVariantBuilder builder;
- if (!dbus_message_iter_append_basic(&sub,
- DBUS_TYPE_STRING,
- &svc1))
- goto error;
- if (!fwDisabled &&
- !dbus_message_iter_append_basic(&sub,
- DBUS_TYPE_STRING,
- &svc2))
- goto error;
- dbus_message_iter_close_container(&iter, &sub);
- } else if (STREQ(service, VIR_FIREWALL_FIREWALLD_SERVICE) &&
- STREQ(member, "passthrough")) {
+ g_variant_builder_init(&builder, G_VARIANT_TYPE("(as)"));
+ g_variant_builder_open(&builder, G_VARIANT_TYPE("as"));
+
+ g_variant_builder_add(&builder, "s", "org.foo.bar.wizz");
+
+ if (!fwDisabled)
+ g_variant_builder_add(&builder, "s", VIR_FIREWALL_FIREWALLD_SERVICE);
+
+ g_variant_builder_close(&builder);
+
+ reply = g_variant_builder_end(&builder);
+ } else if (STREQ(bus_name, VIR_FIREWALL_FIREWALLD_SERVICE) &&
+ STREQ(method_name, "passthrough")) {
+ g_autoptr(GVariantIter) iter = NULL;
+ VIR_AUTOSTRINGLIST args = NULL;
+ size_t nargs = 0;
+ char *type = NULL;
+ char *item = NULL;
bool isAdd = false;
bool doError = false;
+ size_t i;
- if (virDBusMessageDecode(message,
- "sa&s",
- &type,
- &nargs,
- &args) < 0)
- goto error;
+ g_variant_get(params, "(&sas)", &type, &iter);
- for (i = 0; i < nargs; i++) {
+ nargs = g_variant_iter_n_children(iter);
+
+ while (g_variant_iter_loop(iter, "s", &item)) {
/* Fake failure on the command with this IP addr */
- if (STREQ(args[i], "-A")) {
+ if (STREQ(item, "-A")) {
isAdd = true;
- } else if (isAdd && STREQ(args[i], "192.168.122.255")) {
+ } else if (isAdd && STREQ(item, "192.168.122.255")) {
doError = true;
}
+
+ virStringListAdd(&args, g_strdup(item));
}
if (fwBuf) {
@@ -134,61 +130,42 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
else
virBufferAddLit(fwBuf, EBTABLES_PATH);
}
+
for (i = 0; i < nargs; i++) {
if (fwBuf) {
virBufferAddLit(fwBuf, " ");
virBufferEscapeShell(fwBuf, args[i]);
}
}
+
if (fwBuf)
virBufferAddLit(fwBuf, "\n");
+
if (doError) {
- dbus_set_error_const(error,
- "org.firewalld.error",
- "something bad happened");
+ if (error)
+ *error = g_dbus_error_new_for_dbus_error("org.firewalld.error",
+ "something bad happened");
} else {
if (nargs == 1 &&
STREQ(type, "ipv4") &&
STREQ(args[0], "-L")) {
- if (virDBusCreateReply(&reply,
- "s", TEST_FILTER_TABLE_LIST) < 0)
- goto error;
+ reply = g_variant_new("(s)", TEST_FILTER_TABLE_LIST);
} else if (nargs == 3 &&
STREQ(type, "ipv4") &&
STREQ(args[0], "-t") &&
STREQ(args[1], "nat") &&
STREQ(args[2], "-L")) {
- if (virDBusCreateReply(&reply,
- "s", TEST_NAT_TABLE_LIST) < 0)
- goto error;
+ reply = g_variant_new("(s)", TEST_NAT_TABLE_LIST);
} else {
- if (virDBusCreateReply(&reply,
- "s", "success") < 0)
- goto error;
+ reply = g_variant_new("(s)", "success");
}
}
} else {
- reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
+ reply = g_variant_new("()");
}
- cleanup:
- VIR_FREE(type);
- for (i = 0; i < nargs; i++)
- VIR_FREE(args[i]);
- VIR_FREE(args);
return reply;
-
- error:
- virDBusMessageUnref(reply);
- reply = NULL;
- if (error && !dbus_error_is_set(error))
- dbus_set_error_const(error,
- "org.firewalld.error",
- "something unexpected happened");
-
- goto cleanup;
}
-# endif
struct testFirewallData {
virFirewallBackend tryBackend;
@@ -1073,8 +1050,7 @@ mymain(void)
ret = -1; \
} while (0)
-# if WITH_DBUS
-# define RUN_TEST_FIREWALLD(name, method) \
+# define RUN_TEST_FIREWALLD(name, method) \
do { \
struct testFirewallData data; \
data.tryBackend = VIR_FIREWALL_BACKEND_AUTOMATIC; \
@@ -1089,13 +1065,9 @@ mymain(void)
ret = -1; \
} while (0)
-# define RUN_TEST(name, method) \
+# define RUN_TEST(name, method) \
RUN_TEST_DIRECT(name, method); \
RUN_TEST_FIREWALLD(name, method)
-# else /* ! WITH_DBUS */
-# define RUN_TEST(name, method) \
- RUN_TEST_DIRECT(name, method)
-# endif /* ! WITH_DBUS */
virFirewallSetLockOverride(true);
@@ -1113,11 +1085,7 @@ mymain(void)
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
-# if WITH_DBUS
-VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdbus"))
-# else
-VIR_TEST_MAIN(mymain)
-# endif
+VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virgdbus"))
#else /* ! defined (__linux__) */
--
2.26.2
Attachment:
signature.asc
Description: PGP signature
