[libvirt] [PATCH 0/5] qemu: Improve the way we handle migration capabilities
Jiri Denemark (5): qemu: Create a wrapper around qemuMonitorSetCapabilities qemu: Store supported migration capabilities in a bitmap qemu: Use bitmap with migration capabilities qemu: Drop qemuMonitorGetMigrationCapability qemu: Enhance debug message in qemuMonitorSetMigrationCapability src/qemu/qemu_domain.c | 75 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 9 ++++++ src/qemu/qemu_driver.c | 32 ++++++++----------- src/qemu/qemu_migration.c | 45 +++++++++++++++----------- src/qemu/qemu_migration.h | 4 +++ src/qemu/qemu_monitor.c | 22 ++----------- src/qemu/qemu_monitor.h | 2 -- src/qemu/qemu_monitor_json.c | 18 ----------- src/qemu/qemu_monitor_json.h | 2 -- src/qemu/qemu_process.c | 43 ++++++++++++++----------- tests/qemumonitorjsontest.c | 16 ++++++---- 11 files changed, 163 insertions(+), 105 deletions(-) -- 2.14.2
The new function is called qemuProcessInitMonitor and it will enter/exit the monitor so that the caller doesn't have to deal with this. The goal of this patch is to simplify the code in qemuConnectMonitor which would otherwise be a bit hairy after the following patch. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e062194294..24675498a2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1730,12 +1730,31 @@ qemuProcessMonitorLogFree(void *opaque) virObjectUnref(logCtxt); } + +static int +qemuProcessInitMonitor(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorSetCapabilities(QEMU_DOMAIN_PRIVATE(vm)->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + return ret; +} + + static int qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, qemuDomainLogContextPtr logCtxt) { qemuDomainObjPrivatePtr priv = vm->privateData; - int ret = -1; qemuMonitorPtr mon = NULL; unsigned long long timeout = 0; @@ -1794,13 +1813,12 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, return -1; } + if (qemuProcessInitMonitor(driver, vm, asyncJob) < 0) + return -1; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - if (qemuMonitorSetCapabilities(priv->mon) < 0) - goto cleanup; - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) && qemuMonitorSetMigrationCapability(priv->mon, QEMU_MONITOR_MIGRATION_CAPS_EVENTS, @@ -1809,12 +1827,10 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); } - ret = 0; - - cleanup: if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - return ret; + return -1; + + return 0; } -- 2.14.2
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
The new function is called qemuProcessInitMonitor and it will enter/exit the monitor so that the caller doesn't have to deal with this.
The goal of this patch is to simplify the code in qemuConnectMonitor which would otherwise be a bit hairy after the following patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
Heads up - you'll have some merge work to do... I was able to cobble together by going back quite a bit and merging/applying... Anyway, Reviewed-by: John Ferlan <jferlan@redhat.com> John
Each time we need to check whether a given migration capability is supported by QEMU, we call query-migrate-capabilities QMP command and lookup the capability in the returned list. Asking for the list of supported capabilities once when we connect to QEMU and storing the result in a bitmap is much better and we don't need to enter a monitor just to check whether a migration capability is supported. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 9 +++++++ src/qemu/qemu_process.c | 13 +--------- 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 05e8b96aa4..a8cabc5727 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1767,6 +1767,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) priv->namespaces = NULL; priv->reconnectBlockjobs = VIR_TRISTATE_BOOL_ABSENT; + + virBitmapFree(priv->migrationCaps); + priv->migrationCaps = NULL; } @@ -10122,3 +10125,68 @@ qemuDomainGetMachineName(virDomainObjPtr vm) return ret; } + + +int +qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char **caps = NULL; + char **capStr; + int ret = -1; + int rc; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuMonitorGetMigrationCapabilities(priv->mon, &caps); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + if (!caps) { + ret = 0; + goto cleanup; + } + + priv->migrationCaps = virBitmapNew(QEMU_MONITOR_MIGRATION_CAPS_LAST); + if (!priv->migrationCaps) + goto cleanup; + + for (capStr = caps; *capStr; capStr++) { + int cap = qemuMonitorMigrationCapsTypeFromString(*capStr); + + if (cap < 0) { + VIR_DEBUG("Unknown migration capability: '%s'", *capStr); + } else { + ignore_value(virBitmapSetBit(priv->migrationCaps, cap)); + VIR_DEBUG("Found migration capability: '%s'", *capStr); + } + } + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; + + rc = qemuMonitorSetMigrationCapability(priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_EVENTS, + true); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + if (rc < 0) { + virResetLastError(); + VIR_DEBUG("Cannot enable migration events; clearing capability"); + virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); + } + } + + ret = 0; + + cleanup: + virStringListFree(caps); + return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0ac..fb20d8ea63 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate { /* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */ virTristateBool reconnectBlockjobs; + + /* Migration capabilities. Rechecked on reconnect, not to be saved in + * private XML. */ + virBitmapPtr migrationCaps; }; # define QEMU_DOMAIN_PRIVATE(vm) \ @@ -978,4 +982,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm); +int +qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 24675498a2..cea2f90ce1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1816,18 +1816,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, if (qemuProcessInitMonitor(driver, vm, asyncJob) < 0) return -1; - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - return -1; - - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) && - qemuMonitorSetMigrationCapability(priv->mon, - QEMU_MONITOR_MIGRATION_CAPS_EVENTS, - true) < 0) { - VIR_DEBUG("Cannot enable migration events; clearing capability"); - virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); - } - - if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainCheckMigrationCapabilities(driver, vm, asyncJob) < 0) return -1; return 0; -- 2.14.2
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
Each time we need to check whether a given migration capability is supported by QEMU, we call query-migrate-capabilities QMP command and lookup the capability in the returned list. Asking for the list of supported capabilities once when we connect to QEMU and storing the result in a bitmap is much better and we don't need to enter a monitor just to check whether a migration capability is supported.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 9 +++++++ src/qemu/qemu_process.c | 13 +--------- 3 files changed, 78 insertions(+), 12 deletions(-)
There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and qemuDomainObjPrivateXMLParse in order to handle the restart scenario. The rest of this looks OK, but do you need the Format/Parse logic for the bitmap? It was late in my day and I was too lazy to go chase seeing as I already did the merging ;-) John
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 05e8b96aa4..a8cabc5727 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1767,6 +1767,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) priv->namespaces = NULL;
priv->reconnectBlockjobs = VIR_TRISTATE_BOOL_ABSENT; + + virBitmapFree(priv->migrationCaps); + priv->migrationCaps = NULL; }
@@ -10122,3 +10125,68 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
return ret; } + + +int +qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char **caps = NULL; + char **capStr; + int ret = -1; + int rc; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuMonitorGetMigrationCapabilities(priv->mon, &caps); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + if (!caps) { + ret = 0; + goto cleanup; + } + + priv->migrationCaps = virBitmapNew(QEMU_MONITOR_MIGRATION_CAPS_LAST); + if (!priv->migrationCaps) + goto cleanup; + + for (capStr = caps; *capStr; capStr++) { + int cap = qemuMonitorMigrationCapsTypeFromString(*capStr); + + if (cap < 0) { + VIR_DEBUG("Unknown migration capability: '%s'", *capStr); + } else { + ignore_value(virBitmapSetBit(priv->migrationCaps, cap)); + VIR_DEBUG("Found migration capability: '%s'", *capStr); + } + } + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; + + rc = qemuMonitorSetMigrationCapability(priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_EVENTS, + true); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + if (rc < 0) { + virResetLastError(); + VIR_DEBUG("Cannot enable migration events; clearing capability"); + virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); + } + } + + ret = 0; + + cleanup: + virStringListFree(caps); + return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0ac..fb20d8ea63 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
/* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */ virTristateBool reconnectBlockjobs; + + /* Migration capabilities. Rechecked on reconnect, not to be saved in + * private XML. */ + virBitmapPtr migrationCaps; };
# define QEMU_DOMAIN_PRIVATE(vm) \ @@ -978,4 +982,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm, char * qemuDomainGetMachineName(virDomainObjPtr vm);
+int +qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 24675498a2..cea2f90ce1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1816,18 +1816,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, if (qemuProcessInitMonitor(driver, vm, asyncJob) < 0) return -1;
- if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - return -1; - - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) && - qemuMonitorSetMigrationCapability(priv->mon, - QEMU_MONITOR_MIGRATION_CAPS_EVENTS, - true) < 0) { - VIR_DEBUG("Cannot enable migration events; clearing capability"); - virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); - } - - if (qemuDomainObjExitMonitor(driver, vm) < 0) + if (qemuDomainCheckMigrationCapabilities(driver, vm, asyncJob) < 0) return -1;
return 0;
On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
Each time we need to check whether a given migration capability is supported by QEMU, we call query-migrate-capabilities QMP command and lookup the capability in the returned list. Asking for the list of supported capabilities once when we connect to QEMU and storing the result in a bitmap is much better and we don't need to enter a monitor just to check whether a migration capability is supported.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 9 +++++++ src/qemu/qemu_process.c | 13 +--------- 3 files changed, 78 insertions(+), 12 deletions(-)
There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
The rest of this looks OK, but do you need the Format/Parse logic for the bitmap?
No. The migration capabilities are rechecked every time libvirt connects to QEMU as said in the commit message and in qemu_domain.h:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0ac..fb20d8ea63 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
/* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */ virTristateBool reconnectBlockjobs; + + /* Migration capabilities. Rechecked on reconnect, not to be saved in + * private XML. */ + virBitmapPtr migrationCaps; };
Jirka
On 10/20/2017 03:03 AM, Jiri Denemark wrote:
On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
Each time we need to check whether a given migration capability is supported by QEMU, we call query-migrate-capabilities QMP command and lookup the capability in the returned list. Asking for the list of supported capabilities once when we connect to QEMU and storing the result in a bitmap is much better and we don't need to enter a monitor just to check whether a migration capability is supported.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 9 +++++++ src/qemu/qemu_process.c | 13 +--------- 3 files changed, 78 insertions(+), 12 deletions(-)
There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
The rest of this looks OK, but do you need the Format/Parse logic for the bitmap?
No. The migration capabilities are rechecked every time libvirt connects to QEMU as said in the commit message and in qemu_domain.h:
OK, so to be official... Reviewed-by: John Ferlan <jferlan@redhat.com> John
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5201c6a0ac..fb20d8ea63 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
/* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */ virTristateBool reconnectBlockjobs; + + /* Migration capabilities. Rechecked on reconnect, not to be saved in + * private XML. */ + virBitmapPtr migrationCaps; };
Jirka
On Fri, Oct 20, 2017 at 07:08:37 -0400, John Ferlan wrote:
On 10/20/2017 03:03 AM, Jiri Denemark wrote:
On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
Each time we need to check whether a given migration capability is supported by QEMU, we call query-migrate-capabilities QMP command and lookup the capability in the returned list. Asking for the list of supported capabilities once when we connect to QEMU and storing the result in a bitmap is much better and we don't need to enter a monitor just to check whether a migration capability is supported.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 9 +++++++ src/qemu/qemu_process.c | 13 +--------- 3 files changed, 78 insertions(+), 12 deletions(-)
There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
The rest of this looks OK, but do you need the Format/Parse logic for the bitmap?
No. The migration capabilities are rechecked every time libvirt connects to QEMU as said in the commit message and in qemu_domain.h:
OK, so to be official...
Reviewed-by: John Ferlan <jferlan@redhat.com>
I pushed this series. Thanks for the review. Jirka
All calls to qemuMonitorGetMigrationCapability in QEMU driver are replaced with qemuMigrationCapsGet. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ src/qemu/qemu_driver.c | 32 +++++++++++++------------------- src/qemu/qemu_migration.c | 45 ++++++++++++++++++++++++++------------------- src/qemu/qemu_migration.h | 4 ++++ 4 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a8cabc5727..1dcf263ce5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10184,6 +10184,13 @@ qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, } } + /* Migration events capability must always be enabled, clearing it from + * migration capabilities bitmap makes sure it won't be touched anywhere + * else. + */ + ignore_value(virBitmapClearBit(priv->migrationCaps, + QEMU_MONITOR_MIGRATION_CAPS_EVENTS)); + ret = 0; cleanup: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fb4d722368..7f77dcb994 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13413,20 +13413,17 @@ qemuDomainMigrateGetCompressionCache(virDomainPtr dom, priv = vm->privateData; - qemuDomainObjEnterMonitor(driver, vm); - - ret = qemuMonitorGetMigrationCapability( - priv->mon, - QEMU_MONITOR_MIGRATION_CAPS_XBZRLE); - if (ret == 0) { + if (!qemuMigrationCapsGet(vm, QEMU_MONITOR_MIGRATION_CAPS_XBZRLE)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Compressed migration is not supported by " "QEMU binary")); - ret = -1; - } else if (ret > 0) { - ret = qemuMonitorGetMigrationCacheSize(priv->mon, cacheSize); + goto endjob; } + qemuDomainObjEnterMonitor(driver, vm); + + ret = qemuMonitorGetMigrationCacheSize(priv->mon, cacheSize); + if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; @@ -13467,21 +13464,18 @@ qemuDomainMigrateSetCompressionCache(virDomainPtr dom, priv = vm->privateData; - qemuDomainObjEnterMonitor(driver, vm); - - ret = qemuMonitorGetMigrationCapability( - priv->mon, - QEMU_MONITOR_MIGRATION_CAPS_XBZRLE); - if (ret == 0) { + if (!qemuMigrationCapsGet(vm, QEMU_MONITOR_MIGRATION_CAPS_XBZRLE)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Compressed migration is not supported by " "QEMU binary")); - ret = -1; - } else if (ret > 0) { - VIR_DEBUG("Setting compression cache to %llu B", cacheSize); - ret = qemuMonitorSetMigrationCacheSize(priv->mon, cacheSize); + goto endjob; } + qemuDomainObjEnterMonitor(driver, vm); + + VIR_DEBUG("Setting compression cache to %llu B", cacheSize); + ret = qemuMonitorSetMigrationCacheSize(priv->mon, cacheSize); + if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b286d68061..72edbb667c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1274,17 +1274,12 @@ qemuMigrationSetOption(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; int ret; - if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0) - return -1; + if (!qemuMigrationCapsGet(vm, capability)) { + if (!state) { + /* Unsupported but we want it off anyway */ + return 0; + } - ret = qemuMonitorGetMigrationCapability(priv->mon, capability); - - if (ret < 0) { - goto cleanup; - } else if (ret == 0 && !state) { - /* Unsupported but we want it off anyway */ - goto cleanup; - } else if (ret == 0) { if (job == QEMU_ASYNC_JOB_MIGRATION_IN) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _("Migration option '%s' is not supported by " @@ -1296,15 +1291,17 @@ qemuMigrationSetOption(virQEMUDriverPtr driver, "source QEMU binary"), qemuMonitorMigrationCapsTypeToString(capability)); } - ret = -1; - goto cleanup; + return -1; } + if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0) + return -1; + ret = qemuMonitorSetMigrationCapability(priv->mon, capability, state); - cleanup: if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; + return ret; } @@ -5923,12 +5920,8 @@ qemuMigrationReset(virQEMUDriverPtr driver, goto cleanup; for (cap = 0; cap < QEMU_MONITOR_MIGRATION_CAPS_LAST; cap++) { - /* "events" capability is set (when supported) in qemuConnectMonitor - * and should never be cleared */ - if (cap == QEMU_MONITOR_MIGRATION_CAPS_EVENTS) - continue; - - if (qemuMigrationSetOption(driver, vm, cap, false, job) < 0) + if (qemuMigrationCapsGet(vm, cap) && + qemuMigrationSetOption(driver, vm, cap, false, job) < 0) goto cleanup; } @@ -5989,3 +5982,17 @@ qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver, virHashFree(blockinfo); return 0; } + + +bool +qemuMigrationCapsGet(virDomainObjPtr vm, + qemuMonitorMigrationCaps cap) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + bool enabled = false; + + if (priv->migrationCaps) + ignore_value(virBitmapGetBit(priv->migrationCaps, cap, &enabled)); + + return enabled; +} diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 63a4325624..f634138f4d 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -326,4 +326,8 @@ qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob, qemuDomainJobInfoPtr jobInfo); +bool +qemuMigrationCapsGet(virDomainObjPtr vm, + qemuMonitorMigrationCaps cap); + #endif /* __QEMU_MIGRATION_H__ */ -- 2.14.2
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
All calls to qemuMonitorGetMigrationCapability in QEMU driver are replaced with qemuMigrationCapsGet.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ src/qemu/qemu_driver.c | 32 +++++++++++++------------------- src/qemu/qemu_migration.c | 45 ++++++++++++++++++++++++++------------------- src/qemu/qemu_migration.h | 4 ++++ 4 files changed, 50 insertions(+), 38 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
The only remaining user of qemuMonitorGetMigrationCapability is our test suite. Let's replace qemuMonitorGetMigrationCapability with qemuMonitorGetMigrationCapabilities there and drop the unused function. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 19 ------------------- src/qemu/qemu_monitor.h | 2 -- src/qemu/qemu_monitor_json.c | 18 ------------------ src/qemu/qemu_monitor_json.h | 2 -- tests/qemumonitorjsontest.c | 16 ++++++++++------ 5 files changed, 10 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8ffce5a35d..55b123e5f5 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3938,25 +3938,6 @@ qemuMonitorGetMigrationCapabilities(qemuMonitorPtr mon, } -/** - * Returns 1 if @capability is supported, 0 if it's not, or -1 on error. - */ -int -qemuMonitorGetMigrationCapability(qemuMonitorPtr mon, - qemuMonitorMigrationCaps capability) -{ - VIR_DEBUG("capability=%d", capability); - - QEMU_CHECK_MONITOR(mon); - - /* No capability is supported without JSON monitor */ - if (!mon->json) - return 0; - - return qemuMonitorJSONGetMigrationCapability(mon, capability); -} - - int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 57893c61c6..0365b0f397 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -714,8 +714,6 @@ VIR_ENUM_DECL(qemuMonitorMigrationCaps); int qemuMonitorGetMigrationCapabilities(qemuMonitorPtr mon, char ***capabilities); -int qemuMonitorGetMigrationCapability(qemuMonitorPtr mon, - qemuMonitorMigrationCaps capability); int qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, bool state); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 663fce3c3c..f7567eb771 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6068,24 +6068,6 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, } -int -qemuMonitorJSONGetMigrationCapability(qemuMonitorPtr mon, - qemuMonitorMigrationCaps capability) -{ - int ret; - char **capsList = NULL; - const char *cap = qemuMonitorMigrationCapsTypeToString(capability); - - if (qemuMonitorJSONGetMigrationCapabilities(mon, &capsList) < 0) - return -1; - - ret = virStringListHasString((const char **) capsList, cap); - - virStringListFree(capsList); - return ret; -} - - int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 7c45be6725..b17348a119 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -146,8 +146,6 @@ int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon, int qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, char ***capabilities); -int qemuMonitorJSONGetMigrationCapability(qemuMonitorPtr mon, - qemuMonitorMigrationCaps capability); int qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, bool state); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 475fd270e1..4d3b738e52 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2214,7 +2214,8 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapability(const void *data) virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); int ret = -1; - int cap; + const char *cap; + char **caps = NULL; const char *reply = "{" " \"return\": [" @@ -2234,12 +2235,14 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapability(const void *data) "{\"return\":{}}") < 0) goto cleanup; - cap = qemuMonitorJSONGetMigrationCapability(qemuMonitorTestGetMonitor(test), - QEMU_MONITOR_MIGRATION_CAPS_XBZRLE); - if (cap != 1) { + if (qemuMonitorGetMigrationCapabilities(qemuMonitorTestGetMonitor(test), + &caps) < 0) + goto cleanup; + + cap = qemuMonitorMigrationCapsTypeToString(QEMU_MONITOR_MIGRATION_CAPS_XBZRLE); + if (!virStringListHasString((const char **) caps, cap)) { virReportError(VIR_ERR_INTERNAL_ERROR, - "Unexpected capability: %d, expecting 1", - cap); + "Expected capability %s is missing", cap); goto cleanup; } @@ -2251,6 +2254,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapability(const void *data) ret = 0; cleanup: qemuMonitorTestFree(test); + virStringListFree(caps); return ret; } -- 2.14.2
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
The only remaining user of qemuMonitorGetMigrationCapability is our test suite. Let's replace qemuMonitorGetMigrationCapability with qemuMonitorGetMigrationCapabilities there and drop the unused function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 19 ------------------- src/qemu/qemu_monitor.h | 2 -- src/qemu/qemu_monitor_json.c | 18 ------------------ src/qemu/qemu_monitor_json.h | 2 -- tests/qemumonitorjsontest.c | 16 ++++++++++------ 5 files changed, 10 insertions(+), 47 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55b123e5f5..64efb89e83 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3943,7 +3943,8 @@ qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, bool state) { - VIR_DEBUG("capability=%d", capability); + VIR_DEBUG("capability=%s, state=%d", + qemuMonitorMigrationCapsTypeToString(capability), state); QEMU_CHECK_MONITOR_JSON(mon); -- 2.14.2
On 10/18/2017 07:29 AM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55b123e5f5..64efb89e83 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3943,7 +3943,8 @@ qemuMonitorSetMigrationCapability(qemuMonitorPtr mon, qemuMonitorMigrationCaps capability, bool state) { - VIR_DEBUG("capability=%d", capability); + VIR_DEBUG("capability=%s, state=%d", + qemuMonitorMigrationCapsTypeToString(capability), state);
QEMU_CHECK_MONITOR_JSON(mon);
Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
Jiri Denemark -
John Ferlan