[libvirt PATCH 07/14] src/util/virfirewalld: convert to use GLib DBus

[Pavel Hrdina]

On Mon, Sep 21, 2020 at 09:57:33AM +0200, Ján Tomko wrote:
> On a Thursday in 2020, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > 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

So the issue here is that the g_variant_get should have been:

    g_variant_get(reply, "(@as)", &array);

I'll post a patch shortly.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200921/e09d804c/attachment-0001.sig>