On 07/28/2017 10:57 AM, Scott Garfinkle wrote:
From: Scott Garfinkle <seg@us.ibm.com>
Need a commit message. Typically this is something like "Set up wire and protocol for xxx" (see commit id 20ffaf59d for the SetMaxDowntime example)
--- src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 4 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 +++++++++++++- src/remote_protocol-structs | 8 +++++++ 6 files changed, 82 insertions(+), 1 deletion(-)
This patch failed 'make check syntax-check' (see below)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7b4cc3..8b8ae57 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13147,6 +13147,56 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, return ret; }
+ +static int +qemuDomainMigrateGetMaxDowntime(virDomainPtr dom, + unsigned long long *downtime, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv; + qemuMonitorMigrationParams migparams;
s/migparams;/migparams = { 0 }; just to be clear - so that downtimeLimit_set isn't 1 due to random stack stuff
+ int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup;
Could return -1 here
+ + if (virDomainMigrateGetMaxDowntimeEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + priv = vm->privateData; + qemuDomainObjEnterMonitor(driver, vm); + + if (!(ret = qemuMonitorGetMigrationParams(priv->mon, &migparams))) {
I go back and forth on this "if (!(ret = ())) vs. "if ((ret = ()) == 0)" - I like the latter mainly because when I see (!( I'm thinking pointers.
+ if (migparams.downtimeLimit_set) + *downtime = migparams.downtimeLimit; + else + ret = -1;
I think this needs some sort of error message; otherwise, if the "downtime-limit" doesn't exist from a "query-migrate-parameters" then you'd get the fallback libvirt failed for some unknown reason error message. Other places that return -1 here would all elicit some message... Thus: if (qemuMonitorGetMigrationParams(priv->mon, &migparams) == 0) { if (migparams.downtimeLimit_set) { *downtime = migparams.downtimeLimit; ret = 0; } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Getting maximum downtime limit is not supported")); } } (or some error message such as that - I'm not sure if there is a way to determine like the cache code does that the parameter exists.
+ } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} +
Extra space after too...
static int qemuDomainMigrateGetCompressionCache(virDomainPtr dom, unsigned long long *cacheSize, @@ -20826,6 +20876,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetJobInfo = qemuDomainGetJobInfo, /* 0.7.7 */ .domainGetJobStats = qemuDomainGetJobStats, /* 1.0.3 */ .domainAbortJob = qemuDomainAbortJob, /* 0.7.7 */ + .domainMigrateGetMaxDowntime = qemuDomainMigrateGetMaxDowntime, /* 3.6.0 */
3.7.0
.domainMigrateSetMaxDowntime = qemuDomainMigrateSetMaxDowntime, /* 0.8.0 */ .domainMigrateGetCompressionCache = qemuDomainMigrateGetCompressionCache, /* 1.0.3 */ .domainMigrateSetCompressionCache = qemuDomainMigrateSetCompressionCache, /* 1.0.3 */ diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 31f7e97..9805a33 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -627,6 +627,9 @@ struct _qemuMonitorMigrationParams { * whereas, some string value indicates we can support setting/clearing */ char *migrateTLSAlias; char *migrateTLSHostname; + + bool downtimeLimit_set; + unsigned long long downtimeLimit; };
int qemuMonitorGetMigrationParams(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b8a6815..b7b809d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2703,6 +2703,10 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, PARSE(cpuThrottleInitial, "cpu-throttle-initial"); PARSE(cpuThrottleIncrement, "cpu-throttle-increment");
+ if (virJSONValueObjectGetNumberUlong(result, "downtime-limit", + ¶ms->downtimeLimit) == 0) + params->downtimeLimit_set = true; + #undef PARSE
Put the new code after the #undef since it's not part of the PARSE
if ((tlsStr = virJSONValueObjectGetString(result, "tls-creds"))) { diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a57d25f..aa8d8a1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8400,6 +8400,7 @@ static virHypervisorDriver hypervisor_driver = { .domainGetJobInfo = remoteDomainGetJobInfo, /* 0.7.7 */ .domainGetJobStats = remoteDomainGetJobStats, /* 1.0.3 */ .domainAbortJob = remoteDomainAbortJob, /* 0.7.7 */ + .domainMigrateGetMaxDowntime = remoteDomainMigrateGetMaxDowntime, /* 3.6.0 */
3.7.0
.domainMigrateSetMaxDowntime = remoteDomainMigrateSetMaxDowntime, /* 0.8.0 */ .domainMigrateGetCompressionCache = remoteDomainMigrateGetCompressionCache, /* 1.0.3 */ .domainMigrateSetCompressionCache = remoteDomainMigrateSetCompressionCache, /* 1.0.3 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index aa0aa38..e1f4e27 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2326,6 +2326,15 @@ struct remote_domain_abort_job_args { };
+struct remote_domain_migrate_get_max_downtime_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_migrate_get_max_downtime_ret { + unsigned hyper downtime; /* insert@1 */ +}; + struct remote_domain_migrate_set_max_downtime_args { remote_nonnull_domain dom; unsigned hyper downtime; @@ -6064,7 +6073,12 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386 + REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386,
+ /** + * @generate: both + * @acl: domain:migrate + */ + REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_DOWNTIME = 387
}; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index a46fe37..5804e60 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1778,6 +1778,13 @@ struct remote_domain_migrate_set_max_downtime_args { uint64_t downtime; u_int flags; }; +struct remote_domain_migrate_get_max_downtime_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_migrate_get_max_downtime_ret { + uint64_t downtime; +};
These two should be above the "set_max_downtime_args" (check/syntax-check failure)
struct remote_domain_migrate_get_compression_cache_args { remote_nonnull_domain dom; u_int flags; @@ -3233,4 +3240,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_VCPU = 384, REMOTE_PROC_DOMAIN_EVENT_BLOCK_THRESHOLD = 385, REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386, + REMOTE_PROC_DOMAIN_GET_MAX_DOWNTIME = 387
This needs to be: REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_DOWNTIME = 387, from check/syntax-check failure John
};