[libvirt] [PATCH v2] qemu: return -1 on error paths in qemuDomainSaveImageStartVM

[Maxim Nestratov]

29.01.2016 11:49, Nikolay Shirokovskiy пишет:
> Error paths after sending the event that domain is started written as if ret = -1
> which is set at the beginning of the function. It's common idioma to keep 'ret'
> equal to -1 until the end of function where it is set to 0. But here we use ret
> to keep result of restore operation too and thus breaks the idioma and its users :)
>
> Let's use different variable to hold restore result.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>   src/qemu/qemu_driver.c | 21 ++++++++++-----------
>   1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 351e529..ced105b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6732,6 +6732,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
>                              qemuDomainAsyncJob asyncJob)
>   {
>       int ret = -1;
> +    bool restored;
>       virObjectEventPtr event;
>       int intermediatefd = -1;
>       virCommandPtr cmd = NULL;
> @@ -6758,13 +6759,13 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
>       }
>   
>       /* Set the migration source and start it up. */
> -    ret = qemuProcessStart(conn, driver, vm, asyncJob,
> -                           "stdio", *fd, path, NULL,
> -                           VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
> -                           VIR_QEMU_PROCESS_START_PAUSED);
> +    restored = qemuProcessStart(conn, driver, vm, asyncJob,
> +                                "stdio", *fd, path, NULL,
> +                                VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
> +                                VIR_QEMU_PROCESS_START_PAUSED) >= 0;
>   
>       if (intermediatefd != -1) {
> -        if (ret < 0) {
> +        if (!restored) {
>               /* if there was an error setting up qemu, the intermediate
>                * process will wait forever to write to stdout, so we
>                * must manually kill it.
> @@ -6775,7 +6776,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
>   
>           if (virCommandWait(cmd, NULL) < 0) {
>               qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
> -            ret = -1;
> +            restored = false;
>           }
>           VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf));
>       }
> @@ -6783,18 +6784,16 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
>   
>       if (VIR_CLOSE(*fd) < 0) {
>           virReportSystemError(errno, _("cannot close file: %s"), path);
> -        ret = -1;
> +        restored = false;
>       }
>   
> -    if (ret < 0) {
> -        virDomainAuditStart(vm, "restored", false);
> +    virDomainAuditStart(vm, "restored", restored);
> +    if (!restored)
>           goto cleanup;
> -    }
>   
>       event = virDomainEventLifecycleNewFromObj(vm,
>                                        VIR_DOMAIN_EVENT_STARTED,
>                                        VIR_DOMAIN_EVENT_STARTED_RESTORED);
> -    virDomainAuditStart(vm, "restored", true);
>       qemuDomainEventQueue(driver, event);
>   
>   
ACK.