[libvirt] [PATCH v2] lxcDomainShutdownFlags and lxcDomainReboot: Cleanup @flags usage

[Eric Blake]

On 01/09/2014 05:12 PM, Eric Blake wrote:
> On 01/07/2014 08:20 AM, Michal Privoznik wrote:
>> Currently, the @flags usage is a bit unclear at first sight to say the
>> least. There's no need for such unclear code especially when we can
>> borrow the working code from qemuDomainShutdownFlags().

Working, but awkward to read.  I think the qemu code also could benefit
from the rewrite I propose below.

>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---

>> +    if (signalRequested || rc == 0) {
> 
> On the left side of ||, the condition is true for A, C, and D - which is
> wrong if initctl succeeded [ouch].  On the right side of the ||, the
> condition is always true for C, as well as sometimes true for A and D
> (depending on whether initctl succeeded).
> 
> Simplifying this to 'if (rc == 0)' would fix the broken logic.

But would still be hard to read.

> 
> Still broken; needs a v3

May I suggest this layout:

    initctlRequested = !flags || flags & VIR_DOMAIN_SHUTDOWN_INITCTL;
    signalRequested = !flags || flags & VIR_DOMAIN_SHUTDOWN_SIGNAL;

    if (initctlRequested) {
        rc = attempt;
        if (rc < 0)
            goto error;
        if (rc > 0)
            goto success;
        // didn't work, fall through to next attempt
    }
    if (signalRequested) {
        rc = attempt;
        if (rc < 0)
            goto error;
        if (rc > 0)
            goto success;
        // didn't work, fall through to next attempt
    }
    // since we got here, nothing requested worked, so
    goto error;

and quit mentioning signalRequested inside the initctlRequested block,
as well as quit trying to use || on the conditions of whether to attempt
a signal.  Using a 'goto success' at the point where we know it worked,
instead of trying to write magic conditions to avoid duplicate work, is
easier to read.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140109/73f295af/attachment-0001.sig>