[libvirt] [PATCH v3 19/48] remote: in per-driver daemons ensure that state initialize succeeds

[Andrea Bolognani]

On Tue, 2019-07-30 at 12:59 +0200, Christophe de Dinechin wrote:
> Daniel P. Berrangé writes:
[... 163 lines removed ...]
> > @@ -648,15 +650,22 @@ virStateInitialize(bool privileged,
> > 
> >      for (i = 0; i < virStateDriverTabCount; i++) {
> >          if (virStateDriverTab[i]->stateInitialize) {
> > +            virDrvStateInitResult ret;
> >              VIR_DEBUG("Running global init for %s state driver",
> >                        virStateDriverTab[i]->name);
> > -            if (virStateDriverTab[i]->stateInitialize(privileged,
> > -                                                      callback,
> > -                                                      opaque) < 0) {
> > +            ret = virStateDriverTab[i]->stateInitialize(privileged,
> > +                                                        callback,
> > +                                                        opaque);
> > +            VIR_DEBUG("State init result %d (mandatory=%d)", ret, mandatory);
> > +            if (ret == VIR_DRV_STATE_INIT_ERROR) {
> 
> I'm a bit conflicted here. I like the explicit "error" in the name, but
> all the code checks for errors with < 0, and that would work here too.
> But then, you also just replied to me that libvirt only uses -1 as an
> error value, so the < 0 really means == -1... Not sure what to prefer
> here ;-)

Most functions in libvirt either succeed (0) or fail (-1), but in
some cases we need to be able to tell apart different reasons for the
failure and so, accordingly, we return different negative numbers:
that doesn't mean that every single caller of those functions will
care about the specific failure cause, so the <0 check might still be
perfectly fine even in those cases.

Here we have a small number of named return codes, so I agree with
Dan's approach: comparing by name instead of just checking whether
the return value is negative looks a bit cleaner.

[... 469 lines removed ...]
> 
> Reviewed-by: Christophe de Dinechin <dinechin at redhat.com>

Meta: can you please trim the parts of the original message that
you're not specifically replying to? In this particularly egregious
case, less than 10% of the message was actual content rather than
noise.

Please be considerate to list subscribers and make sure they don't
have to waste time and bandwidth fetching additional, unrelated text
just so they can then waste even more time scrolling past them before
getting to your actual reply.

-- 
Andrea Bolognani / Red Hat / Virtualization