[libvirt] [PATCH v2 3/7] util: new virFirewallD APIs + docs

[Daniel P. Berrangé]

On Fri, Feb 01, 2019 at 10:06:44AM -0500, Laine Stump wrote:
> On 2/1/19 8:17 AM, Daniel P. Berrangé wrote:
> > On Thu, Jan 31, 2019 at 08:24:54PM -0500, Laine Stump wrote:
> > > +int
> > > +virFirewallDGetBackend(void)
> > > +{
> > > +    DBusConnection *sysbus = virDBusGetSystemBus();
> > > +    DBusMessage *reply = NULL;
> > > +    virError error;
> > > +    VIR_AUTOFREE(char *) backendStr = NULL;
> > > +    int backend = -1;
> > > +
> > > +    if (!sysbus)
> > > +        return -1;
> > > +
> > > +    memset(&error, 0, sizeof(error));
> > > +
> > > +    if (virDBusCallMethod(sysbus,
> > > +                          &reply,
> > > +                          &error,
> > > +                          VIR_FIREWALL_FIREWALLD_SERVICE,
> > > +                          "/org/fedoraproject/FirewallD1/config",
> > > +                          "org.freedesktop.DBus.Properties",
> > > +                          "Get",
> > > +                          "ss",
> > > +                          "org.fedoraproject.FirewallD1.config",
> > > +                          "FirewallBackend") < 0)
> > > +        goto cleanup;
> > > +
> > > +    if (error.level == VIR_ERR_ERROR) {
> > > +        /* we don't want to log any error in the case that
> > > +         * FirewallBackend isn't implemented in this firewalld, since
> > > +         * that just means that it is an old version, and only has an
> > > +         * iptables backend.
> > > +         */
> > > +        VIR_DEBUG("Failed to get FirewallBackend setting, assuming 'iptables'");
> > > +        backend = VIR_FIREWALLD_BACKEND_IPTABLES;
> > > +        goto cleanup;
> > > +    }
> > Surely we need an '} else {' case here to propagate 'error'
> > as fatal.
> 
> 
> The point is to ignore all errors. If error.level != VIR_ERR_ERROR, then
> there is no error to propagate (and if it is then we already ignored it). Am
> I missing something? (Very likely, since I can count on one hand the number
> of times I've had to directly interact with an error object.)

No, I'm just being dumb  misreading this.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|