On Sun, Dec 02, 2018 at 23:09:57 -0600, Chris Venteicher wrote: After s/Qmp/QMP/g Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt;

On Sun, Dec 02, 2018 at 23:09:58 -0600, Chris Venteicher wrote: ... ... ... You forgot to update the indentation of the line above. ... After fixing that and doing s/Qmp/QMP/g Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt;

On Sun, Dec 02, 2018 at 23:10:00 -0600, Chris Venteicher wrote: This would still crash libvirt if qemuProcessQmpNew() failed. Here you'd need to add if (!proc) return; ... And obviously s/Qmp/QMP/g. Jirka

On Sun, Dec 02, 2018 at 23:10:01 -0600, Chris Venteicher wrote: ... I don't think there's any need for this empty line. ... ... Drop this extra empty line. With the empty lines removed and s/Qmp/QMP/g Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt;

On Sun, Dec 02, 2018 at 23:10:02 -0600, Chris Venteicher wrote: Looks like the patch is unnecessarily doing more things at once... ... I agree calling virQEMUCapsLogProbeFailure from inside virQEMUCapsInitQMP makes a bit more sense then doing it in the caller, but it should be done separately. The change has nothing to do with the purpose of this patch. This could be moved to the "qemu_process: Persist stderr in qemuProcessQmp struct" patch. There should be no reason for splitting a single logical change in two patches. [*] This would return -1 without setting any error. The virReportError [*] from the previous hunk would need to be repeated here. And doing so would indicate this is a wrong place for reporting the error. The qemuProcessQmpRun function returned -1 and thus it should have called virReportError itself. And as I suggested in my review for the previous version, there's no need to have two almost identical copies of the same code in virQEMUCapsInitQMP. Just separate the code into a new function which will then be called twice in a new patch put before this one. Then you don't have to change two places and even moving the virQEMUCapsLogProbeFailure call into virQEMUCapsInitQMP will be simpler. ... ... Just report the error [*] here instead of logging a debug message and there will be no reason to propagate the status up in the call stack or put it into qemuProcessQmpPtr. Reporting the error here would also avoid the issue in one of the following patches: qemuConnectBaselineHypervisorCPU may return -1 without reporting any error if QEMU failed to start. And obviously the error message will need to be a bit generalized (later once the function starts to be usable for more then probing capabilities). This would return -1 without reporting any error because virPidFileReadPath returns -errno and does not set any error. Thus VIR_DEBUG needs to be replaced with virReportSystemError(-rc, _("Failed to read pidfile %s"), proc->pidfile); where rc is the return value of virPidFileReadPath; ... Jirka

On Sun, Dec 02, 2018 at 23:10:03 -0600, Chris Venteicher wrote: This patch will obviously change a bit according to the comments to previous patches. Jirka

On Sun, Dec 02, 2018 at 23:10:04 -0600, Chris Venteicher wrote: Since the changes are all hidden inside qemuProcessQmpStart (formerly called qemuProcessQmpRun) and nothing is calling the new internal functions, all this refactoring could have been separated from this series. But since it's already wired in the series, there's no reason to separate it. Unless serious issues are found in which case it could be easier to separate the refactoring (and deal with the result) than fixing the issues. I don't expect such issues, though. Too much copy&paste, this function is not about VM startup process. We usually log function parameters separately in the first debug message coming from a function. Why do you expect proc->binary to be NULL here? In VM startup code monitor connection is initiated inside the *Launch function. So I'd expect this to be similar if you're trying to mimic that code. But we'll see, it looks like there could be some benefit in doing it this way... IIUC you require the caller to always do qemuProcessQmpStop so why are you calling it too? And since qemuProcessQmpStop is always (if I'm not mistaken) used with qemuProcessQmpFree, why don't we just merge the two functions? The separated functions made sense when the same qemuProcessQmp structure was reused for several processes, but that changed several patches ago. s/qmperr/stderr/ ... Jirka

qemuMonitor code lives in qemuProcessQmpConnectMonitor rather than in qemuProcessQmpNew and qemuProcessQmpLaunch. This is consistent with existing structure in qemu_process.c where qemuConnectMonitor function contains monitor code for domain process activation. Simple code moves in this patch. Improvements in later patch. Only intended functional change in this patch is we don't move (include) code to initiate process stop on failure to create monitor. As comments in qemuProcessQmpStart say... Client must always call qemuProcessStop and qemuProcessQmpFree, even in error cases. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 50 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 938d328235..a688be7f2c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8163,10 +8163,6 @@ qemuProcessQmpNew(const char *binary, virPidFileForceCleanupPath(proc->pidfile); - proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX; - proc->config.data.nix.path = proc->monpath; - proc->config.data.nix.listen = false; - return proc; error: @@ -8198,7 +8194,6 @@ qemuProcessQmpInit(qemuProcessQmpPtr proc) static int qemuProcessQmpLaunch(qemuProcessQmpPtr proc) { - virDomainXMLOptionPtr xmlopt = NULL; const char *machine; int ret = -1; @@ -8250,6 +8245,28 @@ qemuProcessQmpLaunch(qemuProcessQmpPtr proc) goto cleanup; } + ret = 0; + + cleanup: + return ret; +} + + +/* Connect Monitor to QEMU Process + */ +static int +qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) +{ + int ret = -1; + virDomainXMLOptionPtr xmlopt = NULL; + + VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld", + proc, NULLSTR(proc->binary), (long long)proc->pid); + + proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX; + proc->config.data.nix.path = proc->monpath; + proc->config.data.nix.listen = false; + if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) || !(proc->vm = virDomainObjNew(xmlopt))) goto cleanup; @@ -8257,7 +8274,7 @@ qemuProcessQmpLaunch(qemuProcessQmpPtr proc) proc->vm->pid = proc->pid; if (!(proc->mon = qemuMonitorOpen(proc->vm, &proc->config, true, true, - 0, &callbacks, NULL))) + 0, &callbacks, NULL))) goto cleanup; virObjectLock(proc->mon); @@ -8265,27 +8282,8 @@ qemuProcessQmpLaunch(qemuProcessQmpPtr proc) ret = 0; cleanup: - if (!proc->mon) - qemuProcessQmpStop(proc); + VIR_DEBUG("ret=%i", ret); virObjectUnref(xmlopt); - - return ret; -} - - -/* Connect Monitor to QEMU Process - */ -static int -qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) -{ - int ret = -1; - - VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld", - proc, NULLSTR(proc->binary), (long long)proc->pid); - - ret = 0; - - VIR_DEBUG("ret=%i", ret); return ret; } -- 2.17.1

Store libDir path in the qemuProcessQmp struct in anticipation of moving path construction code into qemuProcessQmpInit function. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 8 +++++--- src/qemu/qemu_process.h | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a688be7f2c..d4025ac1bc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8115,6 +8115,7 @@ qemuProcessQmpFree(qemuProcessQmpPtr proc) qemuProcessQmpStop(proc); VIR_FREE(proc->binary); + VIR_FREE(proc->libDir); VIR_FREE(proc->monpath); VIR_FREE(proc->monarg); VIR_FREE(proc->pidfile); @@ -8135,7 +8136,8 @@ qemuProcessQmpNew(const char *binary, if (VIR_ALLOC(proc) < 0) goto error; - if (VIR_STRDUP(proc->binary, binary) < 0) + if (VIR_STRDUP(proc->binary, binary) < 0 || + VIR_STRDUP(proc->libDir, libDir) < 0) goto error; @@ -8146,7 +8148,7 @@ qemuProcessQmpNew(const char *binary, /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" */ - if (virAsprintf(&proc->monpath, "%s/%s", libDir, + if (virAsprintf(&proc->monpath, "%s/%s", proc->libDir, "capabilities.monitor.sock") < 0) goto error; if (virAsprintf(&proc->monarg, "unix:%s,server,nowait", proc->monpath) < 0) @@ -8158,7 +8160,7 @@ qemuProcessQmpNew(const char *binary, * -daemonize we need QEMU to be allowed to create them, rather * than libvirtd. So we're using libDir which QEMU can write to */ - if (virAsprintf(&proc->pidfile, "%s/%s", libDir, "capabilities.pidfile") < 0) + if (virAsprintf(&proc->pidfile, "%s/%s", proc->libDir, "capabilities.pidfile") < 0) goto error; virPidFileForceCleanupPath(proc->pidfile); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 1b64aeef4e..9967766120 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -218,6 +218,7 @@ typedef struct _qemuProcessQmp qemuProcessQmp; typedef qemuProcessQmp *qemuProcessQmpPtr; struct _qemuProcessQmp { char *binary; + char *libDir; uid_t runUid; gid_t runGid; int status; -- 2.17.1

Move code for setting paths and prepping file system from qemuProcessQmpNew to qemuProcessQmpInit. This keeps qemuProcessQmpNew limited to data structures and path initialization is done in qemuProcessQmpInit. The patch is a non-functional, cut / paste change, however goto is now "cleanup" rather than "error". Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 46 +++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4025ac1bc..fc15cb1a3c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8145,14 +8145,34 @@ qemuProcessQmpNew(const char *binary, proc->runGid = runGid; proc->forceTCG = forceTCG; + return proc; + + error: + qemuProcessQmpFree(proc); + return NULL; +} + + +/* Initialize configuration and paths prior to starting QEMU + */ +static int +qemuProcessQmpInit(qemuProcessQmpPtr proc) +{ + int ret = -1; + + VIR_DEBUG("Beginning VM startup process" + " proc=%p, emulator=%s", + proc, proc->binary); + /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" */ if (virAsprintf(&proc->monpath, "%s/%s", proc->libDir, "capabilities.monitor.sock") < 0) - goto error; + goto cleanup; + if (virAsprintf(&proc->monarg, "unix:%s,server,nowait", proc->monpath) < 0) - goto error; + goto cleanup; /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash * with a qemu domain called "capabilities" @@ -8161,31 +8181,13 @@ qemuProcessQmpNew(const char *binary, * than libvirtd. So we're using libDir which QEMU can write to */ if (virAsprintf(&proc->pidfile, "%s/%s", proc->libDir, "capabilities.pidfile") < 0) - goto error; + goto cleanup; virPidFileForceCleanupPath(proc->pidfile); - return proc; - - error: - qemuProcessQmpFree(proc); - return NULL; -} - - -/* Initialize configuration and paths prior to starting QEMU - */ -static int -qemuProcessQmpInit(qemuProcessQmpPtr proc) -{ - int ret = -1; - - VIR_DEBUG("Beginning VM startup process" - " proc=%p, emulator=%s", - proc, proc->binary); - ret = 0; + cleanup: VIR_DEBUG("ret=%i", ret); return ret; } -- 2.17.1

The monitor config data is removed from the qemuProcessQmp struct. The monitor config data can be initialized immediately before call to qemuMonitorOpen and does not need to be maintained after the call because qemuMonitorOpen copies any strings it needs. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 9 +++++---- src/qemu/qemu_process.h | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fc15cb1a3c..8906a22e3c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8263,13 +8263,14 @@ qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) { int ret = -1; virDomainXMLOptionPtr xmlopt = NULL; + virDomainChrSourceDef monConfig; VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld", proc, NULLSTR(proc->binary), (long long)proc->pid); - proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX; - proc->config.data.nix.path = proc->monpath; - proc->config.data.nix.listen = false; + monConfig.type = VIR_DOMAIN_CHR_TYPE_UNIX; + monConfig.data.nix.path = proc->monpath; + monConfig.data.nix.listen = false; if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) || !(proc->vm = virDomainObjNew(xmlopt))) @@ -8277,7 +8278,7 @@ qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) proc->vm->pid = proc->pid; - if (!(proc->mon = qemuMonitorOpen(proc->vm, &proc->config, true, true, + if (!(proc->mon = qemuMonitorOpen(proc->vm, &monConfig, true, true, 0, &callbacks, NULL))) goto cleanup; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 9967766120..0a79d15290 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -228,7 +228,6 @@ struct _qemuProcessQmp { char *pidfile; virCommandPtr cmd; qemuMonitorPtr mon; - virDomainChrSourceDef config; pid_t pid; virDomainObjPtr vm; bool forceTCG; -- 2.17.1

Gracefully handle case when proc activation failed prior to calling. Consistent with the existing code for qemuConnectMonitor (for domains) in qemu_process.c... - Handle qemMonitorOpen failure with INFO message and NULL ptr - Identify parameters passed to qemuMonitorOpen Monitor callbacks will be removed in future patch so we prep for passing NULL for the callback pointer. Set proc->mon to NULL then use VIR_STEAL_PTR if successful to be consistent with other functions. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8906a22e3c..31d41688fe 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8262,25 +8262,50 @@ static int qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) { int ret = -1; + qemuMonitorPtr mon = NULL; + unsigned long long timeout = 0; + bool retry = true; + bool enableJson = true; + virQEMUDriverPtr driver = NULL; + qemuMonitorCallbacksPtr monCallbacks = &callbacks; virDomainXMLOptionPtr xmlopt = NULL; virDomainChrSourceDef monConfig; VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld", proc, NULLSTR(proc->binary), (long long)proc->pid); + if (!proc || !proc->pid) { + ret = 0; + goto cleanup; + } + + proc->mon = NULL; + monConfig.type = VIR_DOMAIN_CHR_TYPE_UNIX; monConfig.data.nix.path = proc->monpath; monConfig.data.nix.listen = false; + /* Create a NULL Domain object for qemuMonitor */ if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) || !(proc->vm = virDomainObjNew(xmlopt))) goto cleanup; proc->vm->pid = proc->pid; - if (!(proc->mon = qemuMonitorOpen(proc->vm, &monConfig, true, true, - 0, &callbacks, NULL))) + mon = qemuMonitorOpen(proc->vm, + &monConfig, + enableJson, + retry, + timeout, + monCallbacks, + driver); + + if (!mon) { + VIR_INFO("Failed to connect monitor to emulator %s", proc->binary); goto cleanup; + } + + VIR_STEAL_PTR(proc->mon, mon); virObjectLock(proc->mon); -- 2.17.1

qemuProcessQmpNew is one of the 4 public functions used to create and manage a qemu process for QMP command exchanges outside of domain operations. Add descriptive comment block, Debug statement and make source consistent with the cleanup / VIR_STEAL_PTR format used elsewhere. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 31d41688fe..faf86dac5d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8124,6 +8124,18 @@ qemuProcessQmpFree(qemuProcessQmpPtr proc) } +/** + * qemuProcessQmpNew: + * @binary: Qemu binary + * @libDir: Directory for process and connection artifacts + * @runUid: UserId for Qemu Process + * @runGid: GroupId for Qemu Process + * @forceTCG: Force TCG mode if true + * + * Allocate and initialize domain structure encapsulating + * QEMU Process state and monitor connection to QEMU + * for completing QMP Queries. + */ qemuProcessQmpPtr qemuProcessQmpNew(const char *binary, const char *libDir, @@ -8131,25 +8143,33 @@ qemuProcessQmpNew(const char *binary, gid_t runGid, bool forceTCG) { + qemuProcessQmpPtr ret = NULL; qemuProcessQmpPtr proc = NULL; + VIR_DEBUG("exec=%s, libDir=%s, runUid=%u, runGid=%u, forceTCG=%d", + NULLSTR(binary), NULLSTR(libDir), runUid, runGid, forceTCG); + if (VIR_ALLOC(proc) < 0) - goto error; + goto cleanup; if (VIR_STRDUP(proc->binary, binary) < 0 || VIR_STRDUP(proc->libDir, libDir) < 0) - goto error; + goto cleanup; proc->runUid = runUid; proc->runGid = runGid; proc->forceTCG = forceTCG; - return proc; + VIR_STEAL_PTR(ret, proc); - error: - qemuProcessQmpFree(proc); - return NULL; + cleanup: + if (proc) + qemuProcessQmpFree(proc); + + VIR_DEBUG("ret=%p", ret); + + return ret; } -- 2.17.1

qemuProcessQmpStop is one of the 4 public functions used to create and manage a Qemu process for QMP command exchanges. Add comment header and debug message. Other minor code formatting cleanup. No change in functionality is intended. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index faf86dac5d..e9b50745d3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8390,10 +8390,20 @@ qemuProcessQmpStart(qemuProcessQmpPtr proc) goto cleanup; } - -void -qemuProcessQmpStop(qemuProcessQmpPtr proc) +/** + * qemuProcessStop: + * @proc: Stores Process and Connection State + * + * Stop Monitor Connection and QEMU Process + */ +void qemuProcessQmpStop(qemuProcessQmpPtr proc) { + if (!proc) + return; + + VIR_DEBUG("Shutting down proc=%p emulator=%s mon=%p pid=%lld", + proc, proc->binary, proc->mon, (long long)proc->pid); + if (proc->mon) { virObjectUnlock(proc->mon); qemuMonitorClose(proc->mon); -- 2.17.1

Catch execution paths where qemuProcessQmpFree is called before qemuProcessQmpStop then report error and force stop before proceeding. Also added public function header and debug message. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_process.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e9b50745d3..1b6adf1b64 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8107,13 +8107,28 @@ static qemuMonitorCallbacks callbacks = { }; +/** + * qemuProcessQmpFree: + * @proc: Stores Process and Connection State + * + * Free process data structure. + */ void qemuProcessQmpFree(qemuProcessQmpPtr proc) { + VIR_DEBUG("proc=%p, proc->mon=%p", proc, (proc ? proc->mon : NULL)); + if (!proc) return; - qemuProcessQmpStop(proc); + /* This should never be non-NULL if we get here, but just in case... */ + if (proc->mon || proc->pid) { + VIR_ERROR(_("Unexpected QEMU still active during process free" + " emulator: %s, pid: %lld, mon: %p"), + NULLSTR(proc->binary), (long long)proc->pid, proc->mon); + qemuProcessQmpStop(proc); + } + VIR_FREE(proc->binary); VIR_FREE(proc->libDir); VIR_FREE(proc->monpath); -- 2.17.1

Qemu process code for capababilities doesn't use monitor callbacks and defines empty callback functions. Allow NULL to be passed to qemuMonitorOpen for callbacks and remove the empty functions from the QMP process code. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_monitor.c | 4 ++-- src/qemu/qemu_process.c | 14 +------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a65d638ab8..84065c59dc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -813,12 +813,12 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, { qemuMonitorPtr mon; - if (!cb->eofNotify) { + if (cb && !cb->eofNotify) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("EOF notify callback must be supplied")); return NULL; } - if (!cb->errorNotify) { + if (cb && !cb->errorNotify) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error notify callback must be supplied")); return NULL; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b6adf1b64..24945b1d17 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8095,18 +8095,6 @@ qemuProcessReconnectAll(virQEMUDriverPtr driver) } -static void virQEMUCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) -{ -} - -static qemuMonitorCallbacks callbacks = { - .eofNotify = virQEMUCapsMonitorNotify, - .errorNotify = virQEMUCapsMonitorNotify, -}; - - /** * qemuProcessQmpFree: * @proc: Stores Process and Connection State @@ -8302,7 +8290,7 @@ qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) bool retry = true; bool enableJson = true; virQEMUDriverPtr driver = NULL; - qemuMonitorCallbacksPtr monCallbacks = &callbacks; + qemuMonitorCallbacksPtr monCallbacks = NULL; virDomainXMLOptionPtr xmlopt = NULL; virDomainChrSourceDef monConfig; -- 2.17.1

qemuProcessQmpStart starts a QEMU process and monitor connection that can be used by multiple functions possibly for multiple QMP commands. The QMP exchange to exit capabilities negotiation mode and enter command mode can only be performed once after the monitor connection is established. Move responsibility for entering QMP command mode into the qemuProcessQmp code so multiple functions can issue QMP commands in arbitrary orders. This also simplifies the functions using the connection provided by qemuProcessQmpStart to issue QMP commands. Test code now needs to call qemuMonitorSetCapabilities to send the message to switch to command mode because the test code does not use the qemuProcessQmp command that internally calls qemuMonitorSetCapabilities. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 12 ------------ src/qemu/qemu_process.c | 8 ++++++++ tests/qemucapabilitiestest.c | 7 +++++++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce60648897..038e3ecf7a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4035,12 +4035,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, /* @mon is supposed to be locked by callee */ - if (qemuMonitorSetCapabilities(mon) < 0) { - VIR_DEBUG("Failed to set monitor capabilities %s", - virGetLastErrorMessage()); - goto cleanup; - } - if (qemuMonitorGetVersion(mon, &major, &minor, &micro, &package) < 0) { @@ -4213,12 +4207,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, { int ret = -1; - if (qemuMonitorSetCapabilities(mon) < 0) { - VIR_DEBUG("Failed to set monitor capabilities %s", - virGetLastErrorMessage()); - goto cleanup; - } - if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 24945b1d17..80b938cc0b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8332,6 +8332,14 @@ qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc) virObjectLock(proc->mon); + /* Exit capabilities negotiation mode and enter QEMU command mode + * by issuing qmp_capabilities command to QEMU */ + if (qemuMonitorSetCapabilities(proc->mon) < 0) { + VIR_DEBUG("Failed to set monitor capabilities %s", + virGetLastErrorMessage()); + goto cleanup; + } + ret = 0; cleanup: diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index a043b0fa64..5fffbc266a 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -58,6 +58,9 @@ testQemuCaps(const void *opaque) if (!(mon = qemuMonitorTestNewFromFileFull(repliesFile, &data->driver, NULL))) goto cleanup; + if (qemuMonitorSetCapabilities(qemuMonitorTestGetMonitor(mon)) < 0) + goto cleanup; + if (!(capsActual = virQEMUCapsNew()) || virQEMUCapsInitQMPMonitor(capsActual, qemuMonitorTestGetMonitor(mon)) < 0) @@ -65,6 +68,10 @@ testQemuCaps(const void *opaque) if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) { qemuMonitorResetCommandID(qemuMonitorTestGetMonitor(mon)); + + if (qemuMonitorSetCapabilities(qemuMonitorTestGetMonitor(mon)) < 0) + goto cleanup; + if (virQEMUCapsInitQMPMonitorTCG(capsActual, qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup; -- 2.17.1

Quoting Jiri Denemark (2019-01-03 08:54:21) In the hypervisor baseline code (following patches) a series of different QMP transactions are completed using the same QEMU process instance. There are a series of QMP baseline messages followed by a QMP cpu expansion message over the same connection to the same QEMU process. Since qemuMonitorSetCapabilities must be called immediately and can only be called once to put the connection monitor in the right state to handle QMP messages we don't want to do qemuMonitorSetCapabilities in individual functions to execute QMP baseline, expansion, etc. because these functions can't know if qemuMonitorSetCapabilities has already been called and it can't be called twice. This seems to push in a direction where qemuMonitorSetCapabilities would always be required right after qemuProcessQMPStart since the process is not usable without and you can't allow qemuMonitorSetCapabilities to be called more than once in downstream execution paths in different decoupled functions downstream from qemuProcessQMPStart. Since it seems mandatory that qemuMonitorSetCapabilities be called once and only once after any call to qemuProcessQMPStart it seemed to make sense to pull qemuMonitorSetCapabilities into qemuProcessStart rather than always require the same series of two coupled function calls anytime a qemu process (for QMP) is started. That the idea behind why qemuMonitorSetCapabilities was pulled into qemuProcessQMPStart but can go with whatever you think makes more sense.

On Sun, Dec 02, 2018 at 23:10:15 -0600, Chris Venteicher wrote: Directories for domains are called "domain-%s" so maybe we could call directories for QMP processes as "qmp-%s" rather than "qemu.%s". mkdtemp returns NULL on error and you need to handle it here. ... Jirka

On Sun, Dec 02, 2018 at 23:10:16 -0600, Chris Venteicher wrote: The expanded use case has nothing to do with locking. You just call QMP commands and the use case only changes what commands are called. qemuConnectMonitor does not lock the monitor code because we have EnterMonitor and ExitMonitor calls which lock/unlock the monitor around every QMP command we call. All monitor code expects the monitor to be locked. Not to mention that there is a corresponding unlock in qemuProcessQmpStop(). In other words, NACK to this patch. Did you hit any issues which inspired you to make this patch? Or was it just a way to make the code seemingly consistent with qemuConnectMonitor? Jirka

On Sun, Dec 02, 2018 at 23:10:17 -0600, Chris Venteicher wrote: This makes no sense. You have qemuMonitorCPUModelInfoNew which takes a model name, call it with NULL and set hostCPU->name manually two lines later. VIR_ALLOC() clears the memory, which is why you won't see any code resetting anything to 0 (NULL, false) after allocation. Please, drop this part, it will likely become incomplete once a new field is added into the structure anyway. I think the !orig part is not really necessary here, unless you explicitly want to support qemuMonitorCPUModelInfoCopy(NULL). But if that's the case, the change should go into separate patch with a description why it is useful. This way it's hidden in an unrelated patch. Looking at all users of qemuMonitorCPUModelInfoPtr I can see the new function should also be used in qemuMonitorJSONGetCPUModelExpansion. Jirka

On Sun, Dec 02, 2018 at 23:10:18 -0600, Chris Venteicher wrote: I think this patch is doing too much at once and it's quite easy to get lost between the FromJSON and ToJSON converters. Introducing each converter in a separate patch would be better. It looks a bit strange to have this wrapper for a code that would only be in one place... Anyway, either put an empty line here or change this to qemuMonitorCPUProperty prop = { .type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN, .value.boolean = prop_value }; ... ... The patch looks like a refactoring, but it actually changes a bit more. Previously "props" was required, while now it is silently ignored if missing. Perhaps it's a valid change, but it should not be hidden inside a refactoring patch. ... Jirka

On Sun, Dec 02, 2018 at 23:10:19 -0600, Chris Venteicher wrote: This does not belong to a commit message. If you have any comments or questions, put them after the "---" line. The name is quite misleading. The function does not produce a diff, it just uses two different views on CPU props to set migratable flags for each prop. I think it would make more sense to just return the augmented ModelInfo. The function will never set @augmented = NULL on success and thus we can return NULL on error. That way you could get rid of the strange tmp variable, which is quite a bit more than just a simple temporary variable. "nmProp" and "mProp" look very similar, it took me some time to realize they are different. This is pretty suspicious. Either cpuData->info was already NULL and there's no need to reset it again or we just leaked the memory cpuData->info was pointing to. ... Jirka

On Sun, Dec 02, 2018 at 23:10:20 -0600, Chris Venteicher wrote: Did you forget to remove this sentence after splitting the patch in two parts? ... There's only "static" or "full" expansion. The STATIC_FULL enum item is just requesting a "full" expansion on the result of a previous "static" expansion. And CPU model expansion does not provide delta from model in any case. It always reports all features. The "full" expansion as opposed to a "static" one also reports aliases (it reports even more, but aliases is what we care about). So for example if "full" expansion reports "sse4-1", "sse4_1", and "sse4.1", "static" expansion will only report one of them (I think it's "sse4.1", but I'm not sure which one is the preferred name in QEMU). I don't see a reason for shuffling the parameters. Changing names or types or both is OK, but changing the order of parameters at the same time makes this patch harder to follow. ... So instead of *expansion = qemuMonitor...(); you do expanded_model = qemuMonitor...(); *expansion = expanded_model; expanded_model = NULL; Why? What reused contents are you talking about here? Why do you call qemuMonitorCPUModelInfoFree on something which was just freed and is thus guaranteed to be NULL? Not to mention that expanded_model is guaranteed to be always NULL once we enter the cleanup part, which means even the VIR_FREE does nothing. Jirka

On Sun, Dec 02, 2018 at 23:10:21 -0600, Chris Venteicher wrote: ... Our common function documentation format would be better. Each parameter on a separate line please. And I'd put @migratable after @model since @model is the primary parameter in the conversion and @migratable just changes the way some features are converted. I don't think this function would actually work as expected if either model or model->name were NULL. At best we'd return NULL, i.e., error without reporting any error message. VIR_ALLOC() should go after the initial VIR_DEBUG. Two empty lines between functions. ... Jirka

On Sun, Dec 02, 2018 at 23:10:22 -0600, Chris Venteicher wrote: Similar comment to the one in the previous patch. I don't think there's any need to check for feature->name == NULL. If there's a feature in cpuDef, its name is not NULL. This semantics should be documented and probably even configurable in some way. -1 is easy, it's only used in host CPUs and the feature is either there (with policy -1) or missing. But I can imagine that VIR_CPU_FEATURE_FORCE would need to be ignored in some cases. Alternatively we could define the function only for -1 and REQUIRE and let the caller adapt the input cpuDef if they need something else. This hunk does not belong to this patch. Jirka

Simple cut/paste operations within function qemuConnectBaselineHypervisorCPU to group together code specific to computing baseline using only libvirt utility functions. This is done in anticipation of creating new utility functions for 1) baseline using libvirt utilities (this code) 2) baseline using qemu qmp command (future) Future patches are easier to follow if the code for using libvirt is consolidated. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7d9e17e72c..007f64d6ad 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13657,7 +13657,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, virQEMUCapsPtr qemuCaps = NULL; virArch arch; virDomainVirtType virttype; - virDomainCapsCPUModelsPtr cpuModels; bool migratable; virCPUDefPtr cpu = NULL; char *cpustr = NULL; @@ -13669,8 +13668,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (virConnectBaselineHypervisorCPUEnsureACL(conn) < 0) goto cleanup; - migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); - if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_AUTO))) goto cleanup; @@ -13683,17 +13680,21 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (!qemuCaps) goto cleanup; - if (!(cpuModels = virQEMUCapsGetCPUDefinitions(qemuCaps, virttype)) || - cpuModels->nmodels == 0) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("QEMU '%s' does not support any CPU models for " - "virttype '%s'"), - virQEMUCapsGetBinary(qemuCaps), - virDomainVirtTypeToString(virttype)); - goto cleanup; - } - if (ARCH_IS_X86(arch)) { + migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); + + virDomainCapsCPUModelsPtr cpuModels; + + if (!(cpuModels = virQEMUCapsGetCPUDefinitions(qemuCaps, virttype)) || + cpuModels->nmodels == 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("QEMU '%s' does not support any CPU models for " + "virttype '%s'"), + virQEMUCapsGetBinary(qemuCaps), + virDomainVirtTypeToString(virttype)); + goto cleanup; + } + int rc = virQEMUCapsGetCPUFeatures(qemuCaps, virttype, migratable, &features); if (rc < 0) -- 2.17.1

Hypervisor baseline cpu can be computed locally using libvirt utility functions or remotely using QEMU QMP commands. Likewise, cpu feature expansion can be computed locally using libvirt utility functions or remotely using QEMU QMP commands. This patch identifies using libvirt as a distinct case in the hypervisor baseline logic and triggers that case when the X86 architecture is being baselined. There is one functionality change introduced by this patch. Local libvirt functions are only used for feature expansion when the local utility functions are used for CPU baseline and an error is generated when no method is available to expand cpu featues. The useQEMU option will be introduced in a future patch for using QEMU to compute baseline rather than using libvirt. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86fc5aa249..4e8a3902d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13709,6 +13709,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, bool migratable; virCPUDefPtr cpu = NULL; char *cpustr = NULL; + bool useLibvirt = false; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -13728,7 +13729,9 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (!qemuCaps) goto cleanup; - if (ARCH_IS_X86(arch)) { + useLibvirt = ARCH_IS_X86(arch); + + if (useLibvirt) { migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); if (qemuConnectBaselineHypervisorCPUViaLibvirt(qemuCaps, migratable, @@ -13744,9 +13747,17 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, cpu->fallback = VIR_CPU_FALLBACK_FORBID; - if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && - virCPUExpandFeatures(arch, cpu) < 0) - goto cleanup; + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { + if (useLibvirt && virCPUExpandFeatures(arch, cpu) < 0) { + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("expand features while " + "computing baseline hypervisor CPU is not supported " + "for arch %s"), virArchToString(arch)); + goto cleanup; + } + } cpustr = virCPUDefFormat(cpu, NULL); -- 2.17.1

Support using QEMU to do feature expansion when also using QEMU to compute hypervisor baseline. A QEMU process is already created to send the QMP messages to baseline using QEMU. The same QEMU process is used for the CPU feature expansion. QEMU only returns migratable features when expanding CPU model in architectures where QEMU is used for baseline so no attempt is made to ask for non-migratable features in expansions when using QEMU for baseline. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8c6838f584..4149e4794a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13843,6 +13843,27 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { if (useLibvirt && virCPUExpandFeatures(arch, cpu) < 0) { goto cleanup; + } else if (useQemu && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) { + + if (!(modelInfo = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) + goto cleanup; + + virCPUDefFree(cpu); + cpu = NULL; + + /* QEMU can only include migratable features + for all archs that use QEMU for baseline calculation */ + migratable = true; + + if (qemuMonitorGetCPUModelExpansion(proc->mon, + QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, + migratable, modelInfo, &expansion) < 0) + goto cleanup; + + if (!(cpu = virQEMUCapsCPUModelInfoToCPUDef(migratable, expansion))) + goto cleanup; + } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("expand features while " -- 2.17.1

QEMU only returns migratable props when expanding model unless explicitly told to also include non-migratable props. Props will be marked migratable when we are certain QEMU returned only migratable props resulting in consistent information and expansion output for s390 that is consistent with x86. After this change, immediately default prop->migratable = _YES for all props when we know QEMU only included migratable props in CPU Model. Set model->migratability = true when we have set prop->migratable. Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; --- src/qemu/qemu_monitor.c | 53 ++++++++++++++- src/qemu/qemu_monitor.h | 7 +- .../caps_2.10.0.s390x.xml | 60 ++++++++--------- .../caps_2.11.0.s390x.xml | 58 ++++++++--------- .../caps_2.12.0.s390x.xml | 56 ++++++++-------- .../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 32 +++++----- .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 34 +++++----- .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 64 +++++++++---------- 8 files changed, 210 insertions(+), 154 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 0c7d7b4b55..23a9c2ff2b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3682,12 +3682,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *expansion ) { + int ret = -1; + qemuMonitorCPUModelInfoPtr tmp; + VIR_DEBUG("type=%d model_name=%s migratable=%d", type, input->name, migratable); + *expansion = NULL; + QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable, input, expansion); + if ((ret = qemuMonitorJSONGetCPUModelExpansion(mon, type, migratable, input, &tmp)) < 0) + goto cleanup; + + if (migratable) { + /* Only migratable props were included in expanded CPU model */ + *expansion = qemuMonitorCPUModelInfoCopyDefaultMigratable(tmp); + } else { + VIR_STEAL_PTR(*expansion, tmp); + } + + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(tmp); + return ret; } @@ -3785,6 +3804,38 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig) } +qemuMonitorCPUModelInfoPtr +qemuMonitorCPUModelInfoCopyDefaultMigratable(const qemuMonitorCPUModelInfo *orig) +{ + qemuMonitorCPUModelInfoPtr ret = NULL; + qemuMonitorCPUModelInfoPtr tmp = NULL; + qemuMonitorCPUPropertyPtr prop = NULL; + size_t i; + + if (!(tmp = qemuMonitorCPUModelInfoCopy(orig))) + goto cleanup; + + for (i = 0; i < tmp->nprops; i++) { + prop = tmp->props + i; + + /* Default prop thats in cpu model (true) to migratable (_YES) + * unless prop already explicitly set not migratable (_NO) + */ + if (prop->type == QEMU_MONITOR_CPU_PROPERTY_BOOLEAN && + prop->value.boolean && + prop->migratable != VIR_TRISTATE_BOOL_NO) + prop->migratable = VIR_TRISTATE_BOOL_YES; + } + + tmp->migratability = true; /* prop->migratable = YES/NO for all CPU props */ + + VIR_STEAL_PTR(ret, tmp); + + cleanup: + return ret; +} + + /* Squash CPU Model Info property list * removing props of type boolean matching value */ void diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7aaecd97dd..77aa7cfdc6 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1025,7 +1025,7 @@ struct _qemuMonitorCPUModelInfo { char *name; size_t nprops; qemuMonitorCPUPropertyPtr props; - bool migratability; + bool migratability; /* true if prop->migratable is YES/NO for all CPU props */ }; typedef enum { @@ -1048,6 +1048,11 @@ qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoNew(const char *name); qemuMonitorCPUModelInfoPtr qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); +qemuMonitorCPUModelInfoPtr +qemuMonitorCPUModelInfoCopyDefaultMigratable(const qemuMonitorCPUModelInfo *orig) + ATTRIBUTE_NONNULL(1); + + int qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr model, const char *prop_name, bool prop_value) diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 180a688ba2..07f4da579b 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -120,36 +120,36 @@ <microcodeVersion>306247</microcodeVersion> <package></package> <arch>s390x</arch> - <hostCPU type='kvm' model='z14-base' migratability='no'> - <property name='aen' type='boolean' value='true'/> - <property name='cmmnt' type='boolean' value='true'/> - <property name='aefsi' type='boolean' value='true'/> - <property name='mepoch' type='boolean' value='true'/> - <property name='msa8' type='boolean' value='true'/> - <property name='msa7' type='boolean' value='true'/> - <property name='msa6' type='boolean' value='true'/> - <property name='msa5' type='boolean' value='true'/> - <property name='msa4' type='boolean' value='true'/> - <property name='msa3' type='boolean' value='true'/> - <property name='msa2' type='boolean' value='true'/> - <property name='msa1' type='boolean' value='true'/> - <property name='sthyi' type='boolean' value='true'/> - <property name='edat' type='boolean' value='true'/> - <property name='ri' type='boolean' value='true'/> - <property name='edat2' type='boolean' value='true'/> - <property name='vx' type='boolean' value='true'/> - <property name='ipter' type='boolean' value='true'/> - <property name='vxeh' type='boolean' value='true'/> - <property name='vxpd' type='boolean' value='true'/> - <property name='esop' type='boolean' value='true'/> - <property name='iep' type='boolean' value='true'/> - <property name='cte' type='boolean' value='true'/> - <property name='ais' type='boolean' value='true'/> - <property name='gs' type='boolean' value='true'/> - <property name='zpci' type='boolean' value='true'/> - <property name='sea_esop2' type='boolean' value='true'/> - <property name='te' type='boolean' value='true'/> - <property name='cmm' type='boolean' value='true'/> + <hostCPU type='kvm' model='z14-base' migratability='yes'> + <property name='aen' type='boolean' value='true' migratable='yes'/> + <property name='cmmnt' type='boolean' value='true' migratable='yes'/> + <property name='aefsi' type='boolean' value='true' migratable='yes'/> + <property name='mepoch' type='boolean' value='true' migratable='yes'/> + <property name='msa8' type='boolean' value='true' migratable='yes'/> + <property name='msa7' type='boolean' value='true' migratable='yes'/> + <property name='msa6' type='boolean' value='true' migratable='yes'/> + <property name='msa5' type='boolean' value='true' migratable='yes'/> + <property name='msa4' type='boolean' value='true' migratable='yes'/> + <property name='msa3' type='boolean' value='true' migratable='yes'/> + <property name='msa2' type='boolean' value='true' migratable='yes'/> + <property name='msa1' type='boolean' value='true' migratable='yes'/> + <property name='sthyi' type='boolean' value='true' migratable='yes'/> + <property name='edat' type='boolean' value='true' migratable='yes'/> + <property name='ri' type='boolean' value='true' migratable='yes'/> + <property name='edat2' type='boolean' value='true' migratable='yes'/> + <property name='vx' type='boolean' value='true' migratable='yes'/> + <property name='ipter' type='boolean' value='true' migratable='yes'/> + <property name='vxeh' type='boolean' value='true' migratable='yes'/> + <property name='vxpd' type='boolean' value='true' migratable='yes'/> + <property name='esop' type='boolean' value='true' migratable='yes'/> + <property name='iep' type='boolean' value='true' migratable='yes'/> + <property name='cte' type='boolean' value='true' migratable='yes'/> + <property name='ais' type='boolean' value='true' migratable='yes'/> + <property name='gs' type='boolean' value='true' migratable='yes'/> + <property name='zpci' type='boolean' value='true' migratable='yes'/> + <property name='sea_esop2' type='boolean' value='true' migratable='yes'/> + <property name='te' type='boolean' value='true' migratable='yes'/> + <property name='cmm' type='boolean' value='true' migratable='yes'/> </hostCPU> <cpu type='kvm' name='z10EC-base' usable='yes'/> <cpu type='kvm' name='z9EC-base' usable='yes'/> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 7dc643d9a3..c2f222bd41 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -127,35 +127,35 @@ <microcodeVersion>345099</microcodeVersion> <package></package> <arch>s390x</arch> - <hostCPU type='kvm' model='z14-base' migratability='no'> - <property name='aen' type='boolean' value='true'/> - <property name='cmmnt' type='boolean' value='true'/> - <property name='aefsi' type='boolean' value='true'/> - <property name='mepoch' type='boolean' value='true'/> - <property name='msa8' type='boolean' value='true'/> - <property name='msa7' type='boolean' value='true'/> - <property name='msa6' type='boolean' value='true'/> - <property name='msa5' type='boolean' value='true'/> - <property name='msa4' type='boolean' value='true'/> - <property name='msa3' type='boolean' value='true'/> - <property name='msa2' type='boolean' value='true'/> - <property name='msa1' type='boolean' value='true'/> - <property name='sthyi' type='boolean' value='true'/> - <property name='edat' type='boolean' value='true'/> - <property name='ri' type='boolean' value='true'/> - <property name='edat2' type='boolean' value='true'/> - <property name='vx' type='boolean' value='true'/> - <property name='ipter' type='boolean' value='true'/> - <property name='vxeh' type='boolean' value='true'/> - <property name='vxpd' type='boolean' value='true'/> - <property name='esop' type='boolean' value='true'/> - <property name='iep' type='boolean' value='true'/> - <property name='cte' type='boolean' value='true'/> - <property name='gs' type='boolean' value='true'/> - <property name='zpci' type='boolean' value='true'/> - <property name='sea_esop2' type='boolean' value='true'/> - <property name='te' type='boolean' value='true'/> - <property name='cmm' type='boolean' value='true'/> + <hostCPU type='kvm' model='z14-base' migratability='yes'> + <property name='aen' type='boolean' value='true' migratable='yes'/> + <property name='cmmnt' type='boolean' value='true' migratable='yes'/> + <property name='aefsi' type='boolean' value='true' migratable='yes'/> + <property name='mepoch' type='boolean' value='true' migratable='yes'/> + <property name='msa8' type='boolean' value='true' migratable='yes'/> + <property name='msa7' type='boolean' value='true' migratable='yes'/> + <property name='msa6' type='boolean' value='true' migratable='yes'/> + <property name='msa5' type='boolean' value='true' migratable='yes'/> + <property name='msa4' type='boolean' value='true' migratable='yes'/> + <property name='msa3' type='boolean' value='true' migratable='yes'/> + <property name='msa2' type='boolean' value='true' migratable='yes'/> + <property name='msa1' type='boolean' value='true' migratable='yes'/> + <property name='sthyi' type='boolean' value='true' migratable='yes'/> + <property name='edat' type='boolean' value='true' migratable='yes'/> + <property name='ri' type='boolean' value='true' migratable='yes'/> + <property name='edat2' type='boolean' value='true' migratable='yes'/> + <property name='vx' type='boolean' value='true' migratable='yes'/> + <property name='ipter' type='boolean' value='true' migratable='yes'/> + <property name='vxeh' type='boolean' value='true' migratable='yes'/> + <property name='vxpd' type='boolean' value='true' migratable='yes'/> + <property name='esop' type='boolean' value='true' migratable='yes'/> + <property name='iep' type='boolean' value='true' migratable='yes'/> + <property name='cte' type='boolean' value='true' migratable='yes'/> + <property name='gs' type='boolean' value='true' migratable='yes'/> + <property name='zpci' type='boolean' value='true' migratable='yes'/> + <property name='sea_esop2' type='boolean' value='true' migratable='yes'/> + <property name='te' type='boolean' value='true' migratable='yes'/> + <property name='cmm' type='boolean' value='true' migratable='yes'/> </hostCPU> <cpu type='kvm' name='z890.2' usable='yes'/> <cpu type='kvm' name='z990.4' usable='yes'/> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index b18bd74dee..f006634a9c 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -137,34 +137,34 @@ <microcodeVersion>375102</microcodeVersion> <package></package> <arch>s390x</arch> - <hostCPU type='kvm' model='z14-base' migratability='no'> - <property name='aen' type='boolean' value='true'/> - <property name='aefsi' type='boolean' value='true'/> - <property name='msa8' type='boolean' value='true'/> - <property name='msa7' type='boolean' value='true'/> - <property name='msa6' type='boolean' value='true'/> - <property name='msa5' type='boolean' value='true'/> - <property name='msa4' type='boolean' value='true'/> - <property name='msa3' type='boolean' value='true'/> - <property name='msa2' type='boolean' value='true'/> - <property name='msa1' type='boolean' value='true'/> - <property name='sthyi' type='boolean' value='true'/> - <property name='edat' type='boolean' value='true'/> - <property name='ri' type='boolean' value='true'/> - <property name='edat2' type='boolean' value='true'/> - <property name='vx' type='boolean' value='true'/> - <property name='ipter' type='boolean' value='true'/> - <property name='vxeh' type='boolean' value='true'/> - <property name='vxpd' type='boolean' value='true'/> - <property name='esop' type='boolean' value='true'/> - <property name='iep' type='boolean' value='true'/> - <property name='cte' type='boolean' value='true'/> - <property name='gs' type='boolean' value='true'/> - <property name='ppa15' type='boolean' value='true'/> - <property name='zpci' type='boolean' value='true'/> - <property name='sea_esop2' type='boolean' value='true'/> - <property name='te' type='boolean' value='true'/> - <property name='cmm' type='boolean' value='true'/> + <hostCPU type='kvm' model='z14-base' migratability='yes'> + <property name='aen' type='boolean' value='true' migratable='yes'/> + <property name='aefsi' type='boolean' value='true' migratable='yes'/> + <property name='msa8' type='boolean' value='true' migratable='yes'/> + <property name='msa7' type='boolean' value='true' migratable='yes'/> + <property name='msa6' type='boolean' value='true' migratable='yes'/> + <property name='msa5' type='boolean' value='true' migratable='yes'/> + <property name='msa4' type='boolean' value='true' migratable='yes'/> + <property name='msa3' type='boolean' value='true' migratable='yes'/> + <property name='msa2' type='boolean' value='true' migratable='yes'/> + <property name='msa1' type='boolean' value='true' migratable='yes'/> + <property name='sthyi' type='boolean' value='true' migratable='yes'/> + <property name='edat' type='boolean' value='true' migratable='yes'/> + <property name='ri' type='boolean' value='true' migratable='yes'/> + <property name='edat2' type='boolean' value='true' migratable='yes'/> + <property name='vx' type='boolean' value='true' migratable='yes'/> + <property name='ipter' type='boolean' value='true' migratable='yes'/> + <property name='vxeh' type='boolean' value='true' migratable='yes'/> + <property name='vxpd' type='boolean' value='true' migratable='yes'/> + <property name='esop' type='boolean' value='true' migratable='yes'/> + <property name='iep' type='boolean' value='true' migratable='yes'/> + <property name='cte' type='boolean' value='true' migratable='yes'/> + <property name='gs' type='boolean' value='true' migratable='yes'/> + <property name='ppa15' type='boolean' value='true' migratable='yes'/> + <property name='zpci' type='boolean' value='true' migratable='yes'/> + <property name='sea_esop2' type='boolean' value='true' migratable='yes'/> + <property name='te' type='boolean' value='true' migratable='yes'/> + <property name='cmm' type='boolean' value='true' migratable='yes'/> </hostCPU> <cpu type='kvm' name='z890.2' usable='yes'/> <cpu type='kvm' name='z990.4' usable='yes'/> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index f3a32ad376..8fa95c1f90 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -109,22 +109,22 @@ <microcodeVersion>244554</microcodeVersion> <package></package> <arch>s390x</arch> - <hostCPU type='kvm' model='zEC12.2-base' migratability='no'> - <property name='aefsi' type='boolean' value='true'/> - <property name='msa5' type='boolean' value='true'/> - <property name='msa4' type='boolean' value='true'/> - <property name='msa3' type='boolean' value='true'/> - <property name='msa2' type='boolean' value='true'/> - <property name='msa1' type='boolean' value='true'/> - <property name='sthyi' type='boolean' value='true'/> - <property name='edat' type='boolean' value='true'/> - <property name='ri' type='boolean' value='true'/> - <property name='edat2' type='boolean' value='true'/> - <property name='ipter' type='boolean' value='true'/> - <property name='esop' type='boolean' value='true'/> - <property name='cte' type='boolean' value='true'/> - <property name='te' type='boolean' value='true'/> - <property name='cmm' type='boolean' value='true'/> + <hostCPU type='kvm' model='zEC12.2-base' migratability='yes'> + <property name='aefsi' type='boolean' value='true' migratable='yes'/> + <property name='msa5' type='boolean' value='true' migratable='yes'/> + <property name='msa4' type='boolean' value='true' migratable='yes'/> + <property name='msa3' type='boolean' value='true' migratable='yes'/> + <property name='msa2' type='boolean' value='true' migratable='yes'/> + <property name='msa1' type='boolean' value='true' migratable='yes'/> + <property name='sthyi' type='boolean' value='true' migratable='yes'/> + <property name='edat' type='boolean' value='true' migratable='yes'/> + <property name='ri' type='boolean' value='true' migratable='yes'/> + <property name='edat2' type='boolean' value='true' migratable='yes'/> + <property name='ipter' type='boolean' value='true' migratable='yes'/> + <property name='esop' type='boolean' value='true' migratable='yes'/> + <property name='cte' type='boolean' value='true' migratable='yes'/> + <property name='te' type='boolean' value='true' migratable='yes'/> + <property name='cmm' type='boolean' value='true' migratable='yes'/> </hostCPU> <cpu type='kvm' name='z10EC-base'/> <cpu type='kvm' name='z9EC-base'/> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index 4bb61f09f0..93a7c72f3a 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -114,23 +114,23 @@ <microcodeVersion>267973</microcodeVersion> <package></package> <arch>s390x</arch> - <hostCPU type='kvm' model='z13.2-base' migratability='no'> - <property name='aefsi' type='boolean' value='true'/> - <property name='msa5' type='boolean' value='true'/> - <property name='msa4' type='boolean' value='true'/> - <property name='msa3' type='boolean' value='true'/> - <property name='msa2' type='boolean' value='true'/> - <property name='msa1' type='boolean' value='true'/> - <property name='sthyi' type='boolean' value='true'/> - <property name='edat' type='boolean' value='true'/> - <property name='ri' type='boolean' value='true'/> - <property name='edat2' type='boolean' value='true'/> - <property name='vx' type='boolean' value='true'/> - <property name='ipter' type='boolean' value='true'/> - <property name='esop' type='boolean' value='true'/> - <property name='cte' type='boolean' value='true'/> - <property name='te' type='boolean' value='true'/> - <property name='cmm' type='boolean' value='true'/> + <hostCPU type='kvm' model='z13.2-base' migratability='yes'> + <property name='aefsi' type='boolean' value='true' migratable='yes'/> + <property name='msa5' type='boolean' value='true' migratable='yes'/> + <property name='msa4' type='boolean' value='true' migratable='yes'/> + <property name='msa3' type='boolean' value='true' migratable='yes'/> + <property name='msa2' type='boolean' value='true' migratable='yes'/> + <property name='msa1' type='boolean' value='true' migratable='yes'/> + <property name='sthyi' type='boolean' value='true' migratable='yes'/> + <property name='edat' type='boolean' value='true' migratable='yes'/> + <property name='ri' type='boolean' value='true' migratable='yes'/> + <property name='edat2' type='boolean' value='true' migratable='yes'/> + <property name='vx' type='boolean' value='true' migratable='yes'/> + <property name='ipter' type='boolean' value='true' migratable='yes'/> + <property name='esop' type='boolean' value='true' migratable='yes'/> + <property name='cte' type='boolean' value='true' migratable='yes'/> + <property name='te' type='boolean' value='true' migratable='yes'/> + <property name='cmm' type='boolean' value='true' migratable='yes'/> </hostCPU> <cpu type='kvm' name='z10EC-base'/> <cpu type='kvm' name='z9EC-base'/> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml index 1c4177c9f4..472eaeeddd 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml @@ -139,38 +139,38 @@ <microcodeVersion>388416</microcodeVersion> <package></package> <arch>s390x</arch> - <hostCPU type='kvm' model='z14-base' migratability='no'> - <property name='aen' type='boolean' value='true'/> - <property name='cmmnt' type='boolean' value='true'/> - <property name='aefsi' type='boolean' value='true'/> - <property name='mepoch' type='boolean' value='true'/> - <property name='msa8' type='boolean' value='true'/> - <property name='msa7' type='boolean' value='true'/> - <property name='msa6' type='boolean' value='true'/> - <property name='msa5' type='boolean' value='true'/> - <property name='msa4' type='boolean' value='true'/> - <property name='msa3' type='boolean' value='true'/> - <property name='msa2' type='boolean' value='true'/> - <property name='msa1' type='boolean' value='true'/> - <property name='sthyi' type='boolean' value='true'/> - <property name='edat' type='boolean' value='true'/> - <property name='ri' type='boolean' value='true'/> - <property name='edat2' type='boolean' value='true'/> - <property name='vx' type='boolean' value='true'/> - <property name='ipter' type='boolean' value='true'/> - <property name='mepochptff' type='boolean' value='true'/> - <property name='vxeh' type='boolean' value='true'/> - <property name='vxpd' type='boolean' value='true'/> - <property name='esop' type='boolean' value='true'/> - <property name='iep' type='boolean' value='true'/> - <property name='cte' type='boolean' value='true'/> - <property name='bpb' type='boolean' value='true'/> - <property name='gs' type='boolean' value='true'/> - <property name='ppa15' type='boolean' value='true'/> - <property name='zpci' type='boolean' value='true'/> - <property name='sea_esop2' type='boolean' value='true'/> - <property name='te' type='boolean' value='true'/> - <property name='cmm' type='boolean' value='true'/> + <hostCPU type='kvm' model='z14-base' migratability='yes'> + <property name='aen' type='boolean' value='true' migratable='yes'/> + <property name='cmmnt' type='boolean' value='true' migratable='yes'/> + <property name='aefsi' type='boolean' value='true' migratable='yes'/> + <property name='mepoch' type='boolean' value='true' migratable='yes'/> + <property name='msa8' type='boolean' value='true' migratable='yes'/> + <property name='msa7' type='boolean' value='true' migratable='yes'/> + <property name='msa6' type='boolean' value='true' migratable='yes'/> + <property name='msa5' type='boolean' value='true' migratable='yes'/> + <property name='msa4' type='boolean' value='true' migratable='yes'/> + <property name='msa3' type='boolean' value='true' migratable='yes'/> + <property name='msa2' type='boolean' value='true' migratable='yes'/> + <property name='msa1' type='boolean' value='true' migratable='yes'/> + <property name='sthyi' type='boolean' value='true' migratable='yes'/> + <property name='edat' type='boolean' value='true' migratable='yes'/> + <property name='ri' type='boolean' value='true' migratable='yes'/> + <property name='edat2' type='boolean' value='true' migratable='yes'/> + <property name='vx' type='boolean' value='true' migratable='yes'/> + <property name='ipter' type='boolean' value='true' migratable='yes'/> + <property name='mepochptff' type='boolean' value='true' migratable='yes'/> + <property name='vxeh' type='boolean' value='true' migratable='yes'/> + <property name='vxpd' type='boolean' value='true' migratable='yes'/> + <property name='esop' type='boolean' value='true' migratable='yes'/> + <property name='iep' type='boolean' value='true' migratable='yes'/> + <property name='cte' type='boolean' value='true' migratable='yes'/> + <property name='bpb' type='boolean' value='true' migratable='yes'/> + <property name='gs' type='boolean' value='true' migratable='yes'/> + <property name='ppa15' type='boolean' value='true' migratable='yes'/> + <property name='zpci' type='boolean' value='true' migratable='yes'/> + <property name='sea_esop2' type='boolean' value='true' migratable='yes'/> + <property name='te' type='boolean' value='true' migratable='yes'/> + <property name='cmm' type='boolean' value='true' migratable='yes'/> </hostCPU> <cpu type='kvm' name='z890.2' usable='yes'/> <cpu type='kvm' name='z990.4' usable='yes'/> -- 2.17.1