- Devel - libvirt List Archives

[libvirt] [PATCH 0/1] dirty-bitmaps: remove deprecated autoload parameter
by John Snow 29 Oct '19

29 Oct '19

I'm going to be honest, here. There's actually no real reason to remove this now, but we could, so I'm going to. Also, in terms of the API serving as documentation, it's nicer to not pretend this is an option that does anything, so out it goes. This will serve as a little smoke test to see what happens if we actually stop dropping features we claimed were deprecated. John Snow (1): dirty-bitmaps: remove deprecated autoload parameter qemu-deprecated.texi | 20 +++++++++++++++----- qapi/block-core.json | 6 +----- blockdev.c | 6 ------ 3 files changed, 16 insertions(+), 16 deletions(-) -- 2.21.0

4 6

[libvirt] [RFC] Add API to change qemu agent response timeout
by Jonathon Jongsma 28 Oct '19

28 Oct '19

Some layered products such as oVirt have requested a way to avoid being blocked by guest agent commands when querying a loaded vm. For example, many guest agent commands are polled periodically to monitor changes, and rather than blocking the calling process, they'd prefer to simply time out when an agent query is taking too long. This patch adds a way for the user to specify a custom agent timeout that is applied to all agent commands. One special case to note here is the 'guest-sync' command. 'guest-sync' is issued internally prior to calling any other command. (For example, when libvirt wants to call 'guest-get-fsinfo', we first call 'guest-sync' and then call 'guest-get-fsinfo'). Previously, the 'guest-sync' command used a 5-second timeout (VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT), whereas the actual command that followed always blocked indefinitely (VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK). As part of this patch, if a custom timeout is specified that is shorter than 5 seconds, this new timeout also used for 'guest-sync'. If there is no custom timeout or if the custom timeout is longer than 5 seconds, we will continue to use the 5-second timeout. See https://bugzilla.redhat.com/show_bug.cgi?id=1705426 for additional details. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- include/libvirt/libvirt-qemu.h | 2 + src/driver-hypervisor.h | 5 +++ src/libvirt-qemu.c | 40 ++++++++++++++++++++ src/libvirt_qemu.syms | 4 ++ src/qemu/qemu_agent.c | 69 +++++++++++++++++++--------------- src/qemu/qemu_agent.h | 3 ++ src/qemu/qemu_driver.c | 24 ++++++++++++ src/qemu_protocol-structs | 8 ++++ src/remote/qemu_protocol.x | 18 ++++++++- src/remote/remote_driver.c | 1 + 10 files changed, 143 insertions(+), 31 deletions(-) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index 891617443f..8d3cc776e9 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -53,6 +53,8 @@ typedef enum { char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd, int timeout, unsigned int flags); +int virDomainQemuAgentSetTimeout(virDomainPtr domain, int timeout); + /** * virConnectDomainQemuMonitorEventCallback: * @conn: the connection pointer diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 015b2cd01c..2f17bff844 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1372,6 +1372,10 @@ typedef int int *nparams, unsigned int flags); +typedef int +(*virDrvDomainQemuAgentSetTimeout)(virDomainPtr domain, + int timeout); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1632,4 +1636,5 @@ struct _virHypervisorDriver { virDrvDomainCheckpointGetParent domainCheckpointGetParent; virDrvDomainCheckpointDelete domainCheckpointDelete; virDrvDomainGetGuestInfo domainGetGuestInfo; + virDrvDomainQemuAgentSetTimeout domainQemuAgentSetTimeout; }; diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index 1afb5fe529..73f119cb23 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -216,6 +216,46 @@ virDomainQemuAgentCommand(virDomainPtr domain, return NULL; } +/** + * virDomainQemuAgentSetTimeout: + * @domain: a domain object + * @timeout: timeout in seconds + * + * Set how long to wait for a response from qemu agent commands. By default, + * agent commands block forever waiting for a response. + * + * @timeout must be -2, -1, 0 or positive. + * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK(-2): meaning to block forever waiting for + * a result. + * VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT(-1): use default timeout value. + * VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT(0): does not wait. + * positive value: wait for @timeout seconds + * + * Returns 0 on success, -1 on failure + */ +int +virDomainQemuAgentSetTimeout(virDomainPtr domain, + int timeout) +{ + virConnectPtr conn; + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + if (conn->driver->domainQemuAgentSetTimeout) { + if (conn->driver->domainQemuAgentSetTimeout(domain, timeout) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} /** * virConnectDomainQemuMonitorEventRegister: diff --git a/src/libvirt_qemu.syms b/src/libvirt_qemu.syms index 3a297e3a2b..348caea72e 100644 --- a/src/libvirt_qemu.syms +++ b/src/libvirt_qemu.syms @@ -30,3 +30,7 @@ LIBVIRT_QEMU_1.2.3 { virConnectDomainQemuMonitorEventDeregister; virConnectDomainQemuMonitorEventRegister; } LIBVIRT_QEMU_0.10.0; +LIBVIRT_QEMU_5.8.0 { + global: + virDomainQemuAgentSetTimeout; +} LIBVIRT_QEMU_1.2.3; diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 34e1a85d64..86352aaec5 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -128,6 +128,7 @@ struct _qemuAgent { * but fire up an event on qemu monitor instead. * Take that as indication of successful completion */ qemuAgentEvent await_event; + int timeout; }; static virClassPtr qemuAgentClass; @@ -696,6 +697,8 @@ qemuAgentOpen(virDomainObjPtr vm, if (!(mon = virObjectLockableNew(qemuAgentClass))) return NULL; + /* agent commands block by default, user can choose different behavior */ + mon->timeout = VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK; mon->fd = -1; if (virCondInit(&mon->notify) < 0) { virReportSystemError(errno, "%s", @@ -851,6 +854,11 @@ static int qemuAgentSend(qemuAgentPtr mon, return -1; if (seconds == VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT) seconds = QEMU_AGENT_WAIT_TIME; + + /* if user specified a custom agent timeout that is lower than the + * default timeout, use the shorter timeout instead */ + if ((mon->timeout > 0) && (mon->timeout < seconds)) + seconds = mon->timeout; then = now + seconds * 1000ull; } @@ -1305,8 +1313,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, if (!cmd) goto cleanup; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { @@ -1343,8 +1350,7 @@ int qemuAgentFSThaw(qemuAgentPtr mon) if (!cmd) return -1; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { @@ -1381,8 +1387,7 @@ qemuAgentSuspend(qemuAgentPtr mon, return -1; mon->await_event = QEMU_AGENT_EVENT_SUSPEND; - ret = qemuAgentCommand(mon, cmd, &reply, false, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK); + ret = qemuAgentCommand(mon, cmd, &reply, false, mon->timeout); virJSONValueFree(cmd); virJSONValueFree(reply); @@ -1438,8 +1443,7 @@ qemuAgentFSTrim(qemuAgentPtr mon, if (!cmd) return ret; - ret = qemuAgentCommand(mon, cmd, &reply, false, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK); + ret = qemuAgentCommand(mon, cmd, &reply, false, mon->timeout); virJSONValueFree(cmd); virJSONValueFree(reply); @@ -1460,8 +1464,7 @@ qemuAgentGetVCPUs(qemuAgentPtr mon, if (!(cmd = qemuAgentMakeCommand("guest-get-vcpus", NULL))) return -1; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { @@ -1576,8 +1579,7 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr mon, NULL))) goto cleanup; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; /* All negative values are invalid. Return of 0 is bogus since we wouldn't @@ -1732,8 +1734,7 @@ qemuAgentGetHostname(qemuAgentPtr mon, if (!cmd) return ret; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) { + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) { if (qemuAgentErrorCommandUnsupported(reply)) ret = -2; goto cleanup; @@ -1778,8 +1779,7 @@ qemuAgentGetTime(qemuAgentPtr mon, if (!cmd) return ret; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { @@ -1844,8 +1844,7 @@ qemuAgentSetTime(qemuAgentPtr mon, if (!cmd) return ret; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; ret = 0; @@ -2054,8 +2053,7 @@ qemuAgentGetFSInfoInternal(qemuAgentPtr mon, if (!cmd) return ret; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) { + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) { if (qemuAgentErrorCommandUnsupported(reply)) ret = -2; goto cleanup; @@ -2347,8 +2345,7 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) goto cleanup; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { @@ -2529,8 +2526,7 @@ qemuAgentSetUserPassword(qemuAgentPtr mon, NULL))) goto cleanup; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) goto cleanup; ret = 0; @@ -2561,8 +2557,7 @@ qemuAgentGetUsers(qemuAgentPtr mon, if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL))) return -1; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) { + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) { if (qemuAgentErrorCommandUnsupported(reply)) return -2; return -1; @@ -2651,8 +2646,7 @@ qemuAgentGetOSInfo(qemuAgentPtr mon, if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL))) return -1; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) { + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) { if (qemuAgentErrorCommandUnsupported(reply)) return -2; return -1; @@ -2707,8 +2701,7 @@ qemuAgentGetTimezone(qemuAgentPtr mon, if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL))) return -1; - if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) { + if (qemuAgentCommand(mon, cmd, &reply, true, mon->timeout) < 0) { if (qemuAgentErrorCommandUnsupported(reply)) return -2; return -1; @@ -2737,3 +2730,19 @@ qemuAgentGetTimezone(qemuAgentPtr mon, return 0; } + +int +qemuAgentSetTimeout(qemuAgentPtr mon, + int timeout) +{ + if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) { + virReportError(VIR_ERR_INVALID_ARG, + _("guest agent timeout '%d' is " + "less than the minimum '%d'"), + timeout, VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN); + return -1; + } + + mon->timeout = timeout; + return 0; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 78e648992a..4037934b91 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -140,3 +140,6 @@ int qemuAgentGetTimezone(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); + +int qemuAgentSetTimeout(qemuAgentPtr mon, + int timeout); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e041a8bac..09251cc9e2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -23434,6 +23434,29 @@ qemuDomainGetGuestInfo(virDomainPtr dom, return ret; } +static int +qemuDomainQemuAgentSetTimeout(virDomainPtr dom, + int timeout) +{ + virDomainObjPtr vm = NULL; + qemuAgentPtr agent; + int ret = -1; + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainQemuAgentSetTimeoutEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + agent = qemuDomainObjEnterAgent(vm); + ret = qemuAgentSetTimeout(agent, timeout); + qemuDomainObjExitAgent(vm, agent); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectURIProbe = qemuConnectURIProbe, @@ -23670,6 +23693,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */ .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */ .domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.7.0 */ + .domainQemuAgentSetTimeout = qemuDomainQemuAgentSetTimeout, /* 5.8.0 */ }; diff --git a/src/qemu_protocol-structs b/src/qemu_protocol-structs index 8501543cd9..be9e739bc6 100644 --- a/src/qemu_protocol-structs +++ b/src/qemu_protocol-structs @@ -47,6 +47,13 @@ struct qemu_domain_monitor_event_msg { u_int micros; remote_string details; }; +struct qemu_domain_agent_set_timeout_args { + remote_nonnull_domain dom; + int timeout; +}; +struct qemu_domain_agent_set_timeout_ret { + int result; +}; enum qemu_procedure { QEMU_PROC_DOMAIN_MONITOR_COMMAND = 1, QEMU_PROC_DOMAIN_ATTACH = 2, @@ -54,4 +61,5 @@ enum qemu_procedure { QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_REGISTER = 4, QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_DEREGISTER = 5, QEMU_PROC_DOMAIN_MONITOR_EVENT = 6, + QEMU_PROC_DOMAIN_AGENT_SET_TIMEOUT = 7, }; diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x index 423e8fadaf..9be2cfa5b7 100644 --- a/src/remote/qemu_protocol.x +++ b/src/remote/qemu_protocol.x @@ -80,6 +80,15 @@ struct qemu_domain_monitor_event_msg { remote_string details; }; +struct qemu_domain_agent_set_timeout_args { + remote_nonnull_domain dom; + int timeout; +}; + +struct qemu_domain_agent_set_timeout_ret { + int result; +}; + /* Define the program number, protocol version and procedure numbers here. */ const QEMU_PROGRAM = 0x20008087; const QEMU_PROTOCOL_VERSION = 1; @@ -152,5 +161,12 @@ enum qemu_procedure { * @generate: both * @acl: none */ - QEMU_PROC_DOMAIN_MONITOR_EVENT = 6 + QEMU_PROC_DOMAIN_MONITOR_EVENT = 6, + + /** + * @generate: both + * @priority: low + * @acl: domain:write + */ + QEMU_PROC_DOMAIN_AGENT_SET_TIMEOUT = 7 }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8789c5da00..94688f9c2a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8748,6 +8748,7 @@ static virHypervisorDriver hypervisor_driver = { .domainCheckpointGetParent = remoteDomainCheckpointGetParent, /* 5.6.0 */ .domainCheckpointDelete = remoteDomainCheckpointDelete, /* 5.6.0 */ .domainGetGuestInfo = remoteDomainGetGuestInfo, /* 5.7.0 */ + .domainQemuAgentSetTimeout = remoteDomainQemuAgentSetTimeout, /* 5.8.0 */ }; static virNetworkDriver network_driver = { -- 2.21.0

5 6

[libvirt] Plan for next release
by Daniel Veillard 28 Oct '19

28 Oct '19

So I plan to to enter freeze later today, that way we can have RC2 around Wed and possibly a release on the 1st or this week-end. I expect a quiet week as a lot of the dev will be at KVM Forum Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

[libvirt] [PATCH] tests: make domaincapstest less anoying to debug
by Peter Krempa 25 Oct '19

25 Oct '19

Since 6a077cf2b3 domaincapstest does not run through all cases on failure but terminates right away. This makes it super annoying to debug or use in combination with VIR_TEST_REGNERATE_OUTPUT. Fix it by remembering failure and still running through all cases. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/domaincapstest.c | 36 +++++++++++++++++++++--------------- tests/testutilsqemu.c | 8 ++++---- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index f77accdb76..e4e13c8bb3 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -313,6 +313,8 @@ doTestQemu(const char *inputDir G_GNUC_UNUSED, const char *suffix G_GNUC_UNUSED, void *opaque) { + int ret = 0; + if (STREQ(arch, "x86_64")) { /* For x86_64 we test three combinations: * @@ -321,13 +323,16 @@ doTestQemu(const char *inputDir G_GNUC_UNUSED, * - TCG with default machine */ if (doTestQemuInternal(version, NULL, arch, - VIR_DOMAIN_VIRT_KVM, opaque) < 0 || - doTestQemuInternal(version, "q35", arch, - VIR_DOMAIN_VIRT_KVM, opaque) < 0 || - doTestQemuInternal(version, NULL, arch, - VIR_DOMAIN_VIRT_QEMU, opaque) < 0) { - return -1; - } + VIR_DOMAIN_VIRT_KVM, opaque) < 0) + ret = -1; + + if (doTestQemuInternal(version, "q35", arch, + VIR_DOMAIN_VIRT_KVM, opaque) < 0) + ret = -1; + + if (doTestQemuInternal(version, NULL, arch, + VIR_DOMAIN_VIRT_QEMU, opaque) < 0) + ret = -1; } else if (STREQ(arch, "aarch64")) { /* For aarch64 we test two combinations: * @@ -335,21 +340,22 @@ doTestQemu(const char *inputDir G_GNUC_UNUSED, * - KVM with virt machine */ if (doTestQemuInternal(version, NULL, arch, - VIR_DOMAIN_VIRT_KVM, opaque) < 0 || - doTestQemuInternal(version, "virt", arch, - VIR_DOMAIN_VIRT_KVM, opaque) < 0) { - return -1; - } + VIR_DOMAIN_VIRT_KVM, opaque) < 0) + ret = -1; + + if (doTestQemuInternal(version, "virt", arch, + VIR_DOMAIN_VIRT_KVM, opaque) < 0) + ret = -1; } else if (STRPREFIX(arch, "riscv")) { /* Unfortunately we have to skip RISC-V at the moment */ return 0; } else { if (doTestQemuInternal(version, NULL, arch, VIR_DOMAIN_VIRT_KVM, opaque) < 0) - return -1; + ret = -1; } - return 0; + return ret; } #endif @@ -431,7 +437,7 @@ mymain(void) abs_srcdir "/qemufirmwaredata/home/user/.config/qemu/firmware"); if (testQemuCapsIterate(".xml", doTestQemu, cfg) < 0) - return EXIT_FAILURE; + ret = -1; /* * Run "tests/qemucapsprobe /path/to/qemu/binary >foo.replies" diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 54d6b1a0ac..34a6bd2653 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -902,6 +902,7 @@ testQemuCapsIterate(const char *suffix, DIR *dir = NULL; int rc; int ret = -1; + bool fail = false; if (!callback) return 0; @@ -949,12 +950,11 @@ testQemuCapsIterate(const char *suffix, * the callback. */ if (callback(TEST_QEMU_CAPS_PATH, "caps", version, - archName, suffix + 1, opaque) < 0) { - goto cleanup; - } + archName, suffix + 1, opaque) < 0) + fail = true; } - if (rc < 0) + if (rc < 0 || fail) goto cleanup; ret = 0; -- 2.21.0

2 1

Re: [libvirt] [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
by Eduardo Habkost 25 Oct '19

25 Oct '19

CCing danpb, libvir-list, mskrivanek. On Fri, Oct 25, 2019 at 09:17:46AM +0200, David Hildenbrand wrote: > On 25.10.19 04:25, Eduardo Habkost wrote: > > We had introduced versioned CPU models in QEMU 4.1, including a > > method for querying for CPU model versions using > > Interesting, I was not aware of that. > > On s390x, we somewhat have versioned CPU models, but we decided against > giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't > really seem to be necessary. (and we often implement/add features for older > CPU models, there is a lot of fluctuation) Actually, you would have had to > add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the > features you actually see in all the different virtual environments ("what a > CPU looks like"). Not to talk about QEMU versions in addition to that. So we > decided to always model what you would see under LPAR and are able to > specify for a KVM guest. So you can use "z13" in an up-to-date LPAR > environment, but not in a z/VM environment (you would have to disable > features). > > Each (!base) CPU model has a specific feature set per machine. Libvirt uses > query-cpu-model-expansion() to convert this model+machine to a > machine-independent representation. That representation is sufficient for > all use cases we were aware of (esp. "virsh domcapabilities", the host CPU > model, migration). > > While s390x has versioned CPU models, we have no explicit way of specifying > them for older machines, besides going over query-cpu-model-expansion and > using expanded "base model + features". > > I can see that this might make sense on x86-64, where you only have maybe 3 > versions of a CPU (e.g., the one Intel messed up first - Haswell, the one > Intel messed up next - Haswell-noTSX, and the one that Intel eventually did > right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs. > "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want > to go for "Haswell-noTSX-IBRS", because you can expect to run in updated > environments if I am not wrong, everything else are corner cases. > > Of course, versioned CPU model are neat to avoid "base model + list of > features", but at least for expanding the host model on s390x, it is not > really helpful. When migrating, the model expansion does the trick. > > I haven't looked into details of "how to specify or model versions". Maybe > IBM wants to look into creating versions for all the old models we had. But > again, not sure if that is of any help for s390x. CCing IBM. I'm not sure yet if there are the x86-specific goals and constraints that would make it difficult to follow the same approach followed by s390x. I have the impression we do, but I need to think more carefully about it. Let's discuss that during KVM Forum? The two main goals of versioned CPU models in x86 are: 1) Decoupling CPU model updates from machine-types (users should be able to update a CPU model definition without changing machine type). 2) Letting management software automate CPU model updates. Normally this is necessary when bare metal microcode or software updates add/remove features from CPUs. Sometimes the Virtual CPU model update needs to be performed before the host updates are applied (if features are being removed). The main constraint that makes it difficult to address the above without a new API is: A) Every CPU model in x86 except "host" is already expected to be migration-safe (I don't know how this compares to s390x). > > > query-cpu-definitions. This only has one problem: fetching CPU > > alias information for multiple machine types required restarting > > QEMU for each machine being queried. > > > > This series adds a new `machine` parameter to > > query-cpu-definitions, that can be used to query CPU model alias > > information for multiple machines without restarting QEMU. > > > > Eduardo Habkost (7): > > i386: Use g_autofree at x86_cpu_list_entry() > > i386: Add default_version parameter to CPU version functions > > i386: Don't use default_cpu_version at "-cpu help" > > machine: machine_find_class() function > > i386: Remove x86_cpu_set_default_version() function > > i386: Don't use default_cpu_version() inside query-cpu-definitions > > cpu: Add `machine` parameter to query-cpu-definitions > > > > qapi/machine-target.json | 14 +++- > > include/hw/boards.h | 1 + > > include/hw/i386/pc.h | 5 +- > > target/i386/cpu.h | 6 -- > > hw/core/machine.c | 16 ++++ > > hw/i386/pc.c | 3 - > > target/arm/helper.c | 4 +- > > target/i386/cpu.c | 93 +++++++++++++++------- > > target/mips/helper.c | 4 +- > > target/ppc/translate_init.inc.c | 4 +- > > target/s390x/cpu_models.c | 4 +- > > vl.c | 17 +--- > > tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++ > > 13 files changed, 154 insertions(+), 59 deletions(-) > > > > > -- > > Thanks, > > David / dhildenb -- Eduardo

2 1

Re: [libvirt] [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
by Eduardo Habkost 25 Oct '19

25 Oct '19

CCing mskrivanek, danpb, libvir-list. On Fri, Oct 25, 2019 at 10:02:29AM +0200, David Hildenbrand wrote: > On 25.10.19 09:55, Christian Borntraeger wrote: > > > > > > On 25.10.19 09:17, David Hildenbrand wrote: > > > On 25.10.19 04:25, Eduardo Habkost wrote: > > > > We had introduced versioned CPU models in QEMU 4.1, including a > > > > method for querying for CPU model versions using > > > > > > Interesting, I was not aware of that. > > > > > > On s390x, we somewhat have versioned CPU models, but we decided against giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't really seem to be necessary. (and we often implement/add features for older CPU models, there is a lot of fluctuation) Actually, you would have had to add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the features you actually see in all the different virtual environments ("what a CPU looks like"). Not to talk about QEMU versions in addition to that. So we decided to always model what you would see under LPAR and are able to specify for a KVM guest. So you can use "z13" in an up-to-date LPAR environment, but not in a z/VM environment (you would have to disable features). > > > > > > Each (!base) CPU model has a specific feature set per machine. Libvirt uses query-cpu-model-expansion() to convert this model+machine to a machine-independent representation. That representation is sufficient for all use cases we were aware of (esp. "virsh domcapabilities", the host CPU model, migration). > > > > > > While s390x has versioned CPU models, we have no explicit way of specifying them for older machines, besides going over query-cpu-model-expansion and using expanded "base model + features". > > > > > > I can see that this might make sense on x86-64, where you only have maybe 3 versions of a CPU (e.g., the one Intel messed up first - Haswell, the one Intel messed up next - Haswell-noTSX, and the one that Intel eventually did right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs. "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want to go for "Haswell-noTSX-IBRS", because you can expect to run in updated environments if I am not wrong, everything else are corner cases. > > > > > > Of course, versioned CPU model are neat to avoid "base model + list of features", but at least for expanding the host model on s390x, it is not really helpful. When migrating, the model expansion does the trick. > > > > > > I haven't looked into details of "how to specify or model versions". Maybe IBM wants to look into creating versions for all the old models we had. But again, not sure if that is of any help for s390x. CCing IBM. > > > > I agree that this does not look very helpful. > > Especially as several things depend on the kernel version a QEMU version is > > not sufficient to be guarantee construction success. > > So we would need something like z14-qemu4.0-kernel-5.2-suse-flavour-onLPAR > > > > Instead we do check if we can construct an equivalent model on the migration target. > > And that model is precise. We do even have versions. > > Right now with QEMU on s390 our models are versioned in a way that we fence of > > facilities for old machine versions. > > > > For example > > -machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility > > and > > -machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility. > > As migration does always require the tuple of machine and cpu this is save. I fail > > to see what the benefit of an explicit z14-3.1 would be. > > > > AFAIKS the only real benefit of versioned CPU models is that you can add new > CPU model versions without new QEMU version. > > Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how > versioned CPU model were implemented) on any QEMU machine. Which is the same > as telling your customer "please use z13,featX=on" in case you have a good > reason to not use the host model (along with baselining) but use an explicit > model. Exactly. oVirt, specifically, don't use host-model. > > If you can change the default model of QEMU machines, you can automate this > process. I am pretty sure this is a corner case, though (e.g., IBRS). > Usually you have a new QEMU machine and can simply enable the new feature > from that point on. When -noTSX happened, we thought it was a corner case. Then we had -IBRS & -IBPB. Then we had SSBD (with no new CPU models). Then we had MD_CLEAR (with no new CPU models). It's now very easy to get an insecure VM created if you are not using host-model. -- Eduardo

1 0

Re: [libvirt] [PATCH 7/7] cpu: Add `machine` parameter to query-cpu-definitions
by Eduardo Habkost 25 Oct '19

25 Oct '19

CCing libvir-list. On Fri, Oct 25, 2019 at 08:36:59AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost(a)redhat.com> writes: > > The new parameter can be used by management software to query for > > CPU model alias information for multiple machines without > > restarting QEMU. [...] > > @@ -317,9 +321,17 @@ > ## > # @query-cpu-definitions: > > # > > # Return a list of supported virtual CPU definitions > > # > > +# @machine: Name of machine type. The command returns some data > > +# that machine-specific. This overrides the machine type > > "that is machine-specific" > > > +# used to look up that information. This can be used, > > +# for example, to query machine-specific CPU model aliases > > +# without restarting QEMU (since 4.2) > > +# > > # Returns: a list of CpuDefInfo > > # > > # Since: 1.2.0 > > ## > > -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'], > > +{ 'command': 'query-cpu-definitions', > > + 'data': { '*machine': 'str' }, > > + 'returns': ['CpuDefinitionInfo'], > > 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } > > I'm afraid the doc comment is less than clear. Before I can suggest > improvements, I have questions. > > Looks like @alias-of is the only part of the return value that changes > when I re-run query-cpu-definitions with another @machine argument. > Correct? Yes. > > How is this going to be used? Will management software run > query-cpu-definitions for each machine type returned by query-machines? > Or just for a few of them? I don't know. I'll let Jiri and other libvirt developers answer that. -- Eduardo

1 0

[libvirt] [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces
by Markus Armbruster 24 Oct '19

24 Oct '19

This series is RFC because it's incomplete, and the warning idea in PATCH 18 is half-baked. I'm soliciting feed back from the management application crowd: is this going into a useful direction? The series adresses only deprecated commands and events. Good enough to demonstrate the ideas, I think. A complete solution should additionally cover arguments and return values. Feels feasible to me. New option -compat deprecated-input=<in-policy>,deprecated-output=out-policy configures the policy. Available input policies: * accept: Accept deprecated commands with a warning (default) * reject: Reject deprecated commands * crash: Crash on deprecated command Available output policies: * accept: Emit deprecated events (default) * hide: Suppress deprecated events See also last item of Subject: Minutes of KVM Forum BoF on deprecating stuff Date: Fri, 26 Oct 2018 16:03:51 +0200 Message-ID: <87mur0ls8o.fsf(a)dusky.pond.sub.org> https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html Markus Armbruster (19): tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers tests/test-qmp-cmds: Check responses more thoroughly tests/test-qmp-cmds: Simplify test data setup tests/test-qmp-event: Simplify test data setup tests/test-qmp-event: Use qobject_is_equal() tests/test-qmp-event: Check event is actually emitted qapi: Add feature flags to remaining definitions qapi: Consistently put @features parameter right after @ifcond qapi: Inline do_qmp_dispatch() into qmp_dispatch() qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP qapi: Simplify how qmp_dispatch() gets the request ID qapi: Replace qmp_dispatch()'s TODO comment by an explanation qapi: New special feature flag "deprecated" qemu-options: New -compat to set policy for "funny" interfaces qapi: Mark deprecated QMP commands with feature 'deprecated' qapi: Implement -compat deprecated-input=reject for commands qapi: Implement -compat deprecated-input=crash for commands qapi: Include a warning in the response to a deprecated command qapi: Implement -compat deprecated-output=hide for events docs/devel/qapi-code-gen.txt | 21 +- tests/qapi-schema/doc-good.texi | 32 ++- qapi/common.json | 48 ++++ qapi/introspect.json | 28 ++- qapi/machine.json | 24 +- qapi/migration.json | 36 ++- qapi/misc.json | 25 +- include/qapi/compat-policy.h | 20 ++ include/qapi/qmp/dispatch.h | 1 + qapi/qmp-dispatch.c | 140 +++++++----- tests/test-qmp-cmds.c | 216 +++++++++++------- tests/test-qmp-event.c | 181 ++++----------- vl.c | 17 ++ qemu-options.hx | 24 ++ scripts/qapi/commands.py | 16 +- scripts/qapi/doc.py | 16 +- scripts/qapi/events.py | 16 +- scripts/qapi/expr.py | 11 +- scripts/qapi/introspect.py | 41 ++-- scripts/qapi/schema.py | 138 ++++++----- scripts/qapi/types.py | 8 +- scripts/qapi/visit.py | 8 +- tests/Makefile.include | 1 + tests/qapi-schema/alternate-base.err | 2 +- tests/qapi-schema/doc-good.json | 18 +- tests/qapi-schema/doc-good.out | 20 +- .../qapi-schema/features-deprecated-type.err | 2 + .../qapi-schema/features-deprecated-type.json | 3 + .../qapi-schema/features-deprecated-type.out | 0 tests/qapi-schema/qapi-schema-test.json | 31 ++- tests/qapi-schema/qapi-schema-test.out | 29 ++- tests/qapi-schema/test-qapi.py | 19 +- 32 files changed, 731 insertions(+), 461 deletions(-) create mode 100644 include/qapi/compat-policy.h create mode 100644 tests/qapi-schema/features-deprecated-type.err create mode 100644 tests/qapi-schema/features-deprecated-type.json create mode 100644 tests/qapi-schema/features-deprecated-type.out -- 2.21.0

2 23

[libvirt] [PATCH] internal: Use g_strcmp0 in STR(N)EQ_NULLABLE
by Peter Krempa 24 Oct '19

24 Oct '19

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/internal.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/internal.h b/src/internal.h index 5b0a2335f5..0ff9f496ac 100644 --- a/src/internal.h +++ b/src/internal.h @@ -86,10 +86,8 @@ #define STRCASEPREFIX(a, b) (c_strncasecmp(a, b, strlen(b)) == 0) #define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL) -#define STREQ_NULLABLE(a, b) \ - ((a) ? (b) && STREQ((a), (b)) : !(b)) -#define STRNEQ_NULLABLE(a, b) \ - ((a) ? !(b) || STRNEQ((a), (b)) : !!(b)) +#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0) +#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0) #define NUL_TERMINATE(buf) do { (buf)[sizeof(buf)-1] = '\0'; } while (0) -- 2.21.0

5 6

[libvirt] [PATCH] tests: domaincaps: Fix build when WITH_QEMU is disabled
by Peter Krempa 24 Oct '19

24 Oct '19

doTestQemuInternal and doTestQemu are used only when WITH_QEMU is enabled. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/domaincapstest.c | 5 +++++ 1 file changed, 5 insertions(+) Pushed as a fix for broken build. diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 0197c61c8a..f77accdb76 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -266,6 +266,9 @@ test_virDomainCapsFormat(const void *opaque) return ret; } + +#if WITH_QEMU + static int doTestQemuInternal(const char *version, const char *machine, @@ -349,6 +352,8 @@ doTestQemu(const char *inputDir G_GNUC_UNUSED, return 0; } +#endif + static int mymain(void) { -- 2.21.0

1 0