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

[Jiri Denemark]

On Fri, Jan 29, 2016 at 11:49:45 +0300, Nikolay Shirokovskiy wrote:
> 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;

I was pretty confused when I saw this for the first time... it's too
easy to miss the comparison at the end. I suggest changing the code to
something similar to the following:

    bool restored = false;
    ...
    if (qemuProcessStart(conn, driver, vm, asyncJob,
                         "stdio", *fd, path, NULL,
                         VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
                         VIR_QEMU_PROCESS_START_PAUSED) == 0)
        restored = true;

Jirka