[libvirt] [PATCH v2 02/12] qemu: introduce qemu domain job status

[Jiri Denemark]

On Wed, Dec 28, 2016 at 17:39:11 +0300, Nikolay Shirokovskiy wrote:
> This patch simply switches code from using VIR_DOMAIN_JOB_* to
> introduced QEMU_DOMAIN_JOB_STATUS_*. Later this gives us freedom
> to introduce states for postcopy and mirroring phases.
> ---
>  src/qemu/qemu_domain.c    | 24 ++++++++++++++++++++--
>  src/qemu/qemu_domain.h    | 11 +++++++++-
>  src/qemu/qemu_driver.c    | 11 +++++-----
>  src/qemu/qemu_migration.c | 52 +++++++++++++++++++++++------------------------
>  src/qemu/qemu_process.c   |  2 +-
>  5 files changed, 63 insertions(+), 37 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index acc27d0..3582151 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -386,11 +386,31 @@ qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo)
>      return 0;
>  }
>  
> +static virDomainJobType
> +qemuDomainJobStatusToType(qemuDomainJobStatus status)
> +{
> +    switch (status) {
> +    case QEMU_DOMAIN_JOB_STATUS_NONE:
> +        return VIR_DOMAIN_JOB_NONE;
> +    case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
> +        return VIR_DOMAIN_JOB_UNBOUNDED;
> +    case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
> +        return VIR_DOMAIN_JOB_COMPLETED;
> +    case QEMU_DOMAIN_JOB_STATUS_FAILED:
> +        return VIR_DOMAIN_JOB_FAILED;
> +    case QEMU_DOMAIN_JOB_STATUS_CANCELED:
> +        return VIR_DOMAIN_JOB_CANCELLED;
> +    }

Please, put separate cases by an empty line. It's pretty hard to read
such a big block of uppercase latters.

> +
> +    /* should not reach here */
> +    return VIR_DOMAIN_JOB_NONE;

I'm afraid this could be reported as unreachable code by some picky
static analyzers, so how about:

    switch (status) {
    case QEMU_DOMAIN_JOB_STATUS_NONE:
        break;

    ...
    }

    return VIR_DOMAIN_JOB_NONE;

> +}
> +
...
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 0f4a6cf..c5184b2 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
...
> @@ -2555,19 +2555,19 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)

I think it would make sense to rename this function as
qemuMigrationUpdateJobStatus in a follow-up patch.

ACK with the cosmetic changes.

Jirka