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