Re: [libvirt] [PATCH v2] lxcDomainShutdownFlags and lxcDomainReboot: Cleanup @flags usage
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)