- Devel - libvirt List Archives

[libvirt] [PATCH v4] vl.c deprecate incorrect CPUs topology
by Igor Mammedov 28 Aug '18

28 Aug '18

-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology so that total number of logical CPUs [sockets * cores * threads] would be equal to [maxcpus], however historically we didn't have such check in QEMU and it is possible to start VM with an invalid topology. Deprecate invalid options combination so we can make sure that the topology VM started with is always correct in the future. Users with an invalid sockets/cores/threads/maxcpus values should fix their CLI to make sure that [sockets * cores * threads] == [maxcpus] Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> --- v4: - missed dot comment, fix it with s/./,/ (Andrew Jones <drjones(a)redhat.com>) v3: - more spelling fixes (Andrew Jones <drjones(a)redhat.com>) - place deprecation check after (sockets * cores * threads > max_cpus) check (Eduardo Habkost <ehabkost(a)redhat.com>) v2: - spelling&&co fixes (Andrew Jones <drjones(a)redhat.com>) --- qemu-deprecated.texi | 11 +++++++++++ vl.c | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 87212b6..094b4ca 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -159,6 +159,17 @@ The 'file' driver for drives is no longer appropriate for character or host devices and will only accept regular files (S_IFREG). The correct driver for these file types is 'host_cdrom' or 'host_device' as appropriate. +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1) + +CPU topology properties should describe whole machine topology including +possible CPUs, but historically it was possible to start QEMU with +an incorrect topology where + sockets * cores * threads >= X && X < maxcpus +which could lead to an incorrect topology enumeration by the guest. +Support for invalid topologies will be removed, the user must ensure +topologies described with -smp include all possible cpus, i.e. + sockets * cores * threads == maxcpus + @section QEMU Machine Protocol (QMP) commands @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0) diff --git a/vl.c b/vl.c index 5ba06ad..7fd700e 100644 --- a/vl.c +++ b/vl.c @@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts) exit(1); } + if (sockets * cores * threads != max_cpus) { + warn_report("Invalid CPU topology deprecated: " + "sockets (%u) * cores (%u) * threads (%u) " + "!= maxcpus (%u)", + sockets, cores, threads, max_cpus); + } + smp_cpus = cpus; smp_cores = cores; smp_threads = threads; -- 2.7.4

2 1

[libvirt] [PATCH v3] vl.c deprecate incorrect CPUs topology
by Igor Mammedov 28 Aug '18

28 Aug '18

-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology so that total number of logical CPUs [sockets * cores * threads] would be equal to [maxcpus], however historically we didn't have such check in QEMU and it is possible to start VM with an invalid topology. Deprecate invalid options combination so we can make sure that the topology VM started with is always correct in the future. Users with an invalid sockets/cores/threads/maxcpus values should fix their CLI to make sure that [sockets * cores * threads] == [maxcpus] Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> --- v3: - more spelling fixes (Andrew Jones <drjones(a)redhat.com>) - place deprecation check after (sockets * cores * threads > max_cpus) check (Eduardo Habkost <ehabkost(a)redhat.com>) v2: - spelling&&co fixes (Andrew Jones <drjones(a)redhat.com>) --- qemu-deprecated.texi | 11 +++++++++++ vl.c | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 87212b6..dc9d9ee 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -159,6 +159,17 @@ The 'file' driver for drives is no longer appropriate for character or host devices and will only accept regular files (S_IFREG). The correct driver for these file types is 'host_cdrom' or 'host_device' as appropriate. +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1) + +CPU topology properties should describe whole machine topology including +possible CPUs. but historically it was possible to start QEMU with +an incorrect topology where + sockets * cores * threads >= X && X < maxcpus +which could lead to an incorrect topology enumeration by the guest. +Support for invalid topologies will be removed, the user must ensure +topologies described with -smp include all possible cpus, i.e. + sockets * cores * threads == maxcpus + @section QEMU Machine Protocol (QMP) commands @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0) diff --git a/vl.c b/vl.c index 5ba06ad..7fd700e 100644 --- a/vl.c +++ b/vl.c @@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts) exit(1); } + if (sockets * cores * threads != max_cpus) { + warn_report("Invalid CPU topology deprecated: " + "sockets (%u) * cores (%u) * threads (%u) " + "!= maxcpus (%u)", + sockets, cores, threads, max_cpus); + } + smp_cpus = cpus; smp_cores = cores; smp_threads = threads; -- 2.7.4

2 1

[libvirt] [PATCH] qemuargv2xmltest: Fix caps loading in VPATH build
by Jiri Denemark 28 Aug '18

28 Aug '18

Broken by v4.7.0-rc1-9-g6700062fb0. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> Pushed as build-breaker fix. --- tests/qemuargv2xmltest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 966e75ef5f..7e250b7b9d 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -159,7 +159,7 @@ mymain(void) do { \ virQEMUCapsPtr qemuCaps; \ qemuCaps = qemuTestParseCapabilitiesArch(VIR_ARCH_X86_64, \ - "qemucapabilitiesdata/caps_2.12.0." arch ".xml"); \ + abs_srcdir "/qemucapabilitiesdata/caps_2.12.0." arch ".xml"); \ if (virFileCacheInsertData(driver.qemuCapsCache, \ "/usr/bin/qemu-system-" arch, \ qemuCaps) < 0) \ -- 2.18.0

1 0

[libvirt] question about job handling across migration protocol phases
by Jim Fehlig 28 Aug '18

28 Aug '18

While investigating a bug [1] found by Xen's osstest I realized I don't quite understand how to handle modify jobs (e.g. BeginJob/EndJob) on virDomainObj across the various phases of V3 migration protocol. E.g. on the src host the Begin, Perform, and Confirm phases are performed. Should a modify job start (BeginJob) in the Begin phase and stop (EndJob) in the Confirm phase? Or should each phase, if necessary, do BeginJob/EndJob? Same question for dst host. IMO the job should be held across the phases on each host, preventing any modifications during the overall migration process. Although I do worry about orphaned jobs, e.g. a missed EndJob caused by some obscure error in the migration machinery. Regards, Jim [1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01945.html

2 1

[libvirt] [PATCH] util: json: Allow converting a virTristate(Bool|Switch) into JSON
by Peter Krempa 28 Aug '18

28 Aug '18

Add a new modifier letter for virJSONValueObjectAddVArgs which will add a boolean value with our tristate semantics. The value is omitted when the _ABSENT value is used. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/util/virjson.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/util/virjson.c b/src/util/virjson.c index 29530dcb15..e45f1fab06 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -142,6 +142,8 @@ virJSONValueGetType(const virJSONValue *value) * * b: boolean value * B: boolean value, omitted if false + * T: boolean value specified by a virTristate(Bool|Switch) value, omitted on + * the _ABSENT value * * d: double precision floating point number * n: json null value @@ -266,12 +268,23 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj, } break; case 'B': + case 'T': case 'b': { int val = va_arg(args, int); if (!val && type == 'B') continue; + if (type == 'T') { + if (val == VIR_TRISTATE_SWITCH_ABSENT) + continue; + + if (val == VIR_TRISTATE_BOOL_NO) + val = 0; + else + val = 1; + } + rc = virJSONValueObjectAppendBoolean(obj, key, val); } break; -- 2.16.2

2 2

[libvirt] [PATCH] qemu: initialize variables in qemuParseCommandLine
by Ján Tomko 28 Aug '18

28 Aug '18

Commit 6700062 introduced a jump to error which skipped the initialization of def: qemu/qemu_parse_command.c:1870:9: error: variable 'def' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized] if (!(qemuCaps = virQEMUCapsCacheLookup(capsCache, progargv[0]))) Initialize def to fix this warning and qemuCaps, to prevent a future error like this. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- Pushed as a build breaker fix. src/qemu/qemu_parse_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index c0bf27f800..5b4a378a18 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1838,7 +1838,7 @@ qemuParseCommandLine(virFileCachePtr capsCache, virDomainChrSourceDefPtr *monConfig, bool *monJSON) { - virDomainDefPtr def; + virDomainDefPtr def = NULL; size_t i; bool nographics = false; bool fullscreen = false; @@ -1852,7 +1852,7 @@ qemuParseCommandLine(virFileCachePtr capsCache, virDomainDiskDefPtr disk = NULL; const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS"); bool have_sdl = false; - virQEMUCapsPtr qemuCaps; + virQEMUCapsPtr qemuCaps = NULL; if (pidfile) *pidfile = NULL; -- 2.16.4

1 0

[libvirt] [PATCH] conf: report enum errors in virDomainInputDefValidate
by Ján Tomko 28 Aug '18

28 Aug '18

Commit deb057f added a switch without a default case. Add it and call virReportEnumRangeError for _LAST too. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/conf/domain_conf.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bde9fef914..8cb28a0880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5760,7 +5760,6 @@ virDomainInputDefValidate(const virDomainInputDef *input) case VIR_DOMAIN_INPUT_TYPE_MOUSE: case VIR_DOMAIN_INPUT_TYPE_TABLET: case VIR_DOMAIN_INPUT_TYPE_KBD: - case VIR_DOMAIN_INPUT_TYPE_LAST: if (input->source.evdev) { virReportError(VIR_ERR_XML_ERROR, "%s", _("setting source evdev path only supported for " @@ -5771,6 +5770,11 @@ virDomainInputDefValidate(const virDomainInputDef *input) case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: break; + + case VIR_DOMAIN_INPUT_TYPE_LAST: + default: + virReportEnumRangeError(virDomainInputType, input->type); + return -1; } return 0; -- 2.16.4

2 1

[libvirt] [PATCH 0/4] tests: qemuxml2argv: More unused files cleanup
by Peter Krempa 28 Aug '18

28 Aug '18

Cleanup most output files unused by our test suite. See justification in individual commits. Peter Krempa (4): tests: qemuxml2argv: Remove 'args' files for tests expecting failure tests: qemuxml2argv: Make use of 'vram64' QXL device tests tests: qemuxml2argv: Remove the 'no-shutdown' test completely tests: qemuxml2argv: Remove 'args' for tests only used in xml2xmltest .../hugepages-default-1G-nodeset-2M.args | 25 ---------------- tests/qemuxml2argvdata/lease.args | 23 --------------- tests/qemuxml2argvdata/metadata.args | 22 -------------- tests/qemuxml2argvdata/no-shutdown.args | 27 ----------------- tests/qemuxml2argvdata/no-shutdown.xml | 24 --------------- tests/qemuxml2argvdata/pcihole64-none.args | 20 ------------- .../qemuxml2argvdata/video-qxl-device-vram64.args | 25 ---------------- .../video-qxl-device-vram64.x86_64-latest.args | 32 ++++++++++++++++++++ tests/qemuxml2argvdata/video-qxl-device-vram64.xml | 6 ---- .../video-qxl-sec-device-vram64.args | 27 ----------------- .../video-qxl-sec-device-vram64.x86_64-latest.args | 34 ++++++++++++++++++++++ .../video-qxl-sec-device-vram64.xml | 6 ---- tests/qemuxml2argvtest.c | 2 ++ 13 files changed, 68 insertions(+), 205 deletions(-) delete mode 100644 tests/qemuxml2argvdata/hugepages-default-1G-nodeset-2M.args delete mode 100644 tests/qemuxml2argvdata/lease.args delete mode 100644 tests/qemuxml2argvdata/metadata.args delete mode 100644 tests/qemuxml2argvdata/no-shutdown.args delete mode 100644 tests/qemuxml2argvdata/no-shutdown.xml delete mode 100644 tests/qemuxml2argvdata/pcihole64-none.args delete mode 100644 tests/qemuxml2argvdata/video-qxl-device-vram64.args create mode 100644 tests/qemuxml2argvdata/video-qxl-device-vram64.x86_64-latest.args delete mode 100644 tests/qemuxml2argvdata/video-qxl-sec-device-vram64.args create mode 100644 tests/qemuxml2argvdata/video-qxl-sec-device-vram64.x86_64-latest.args -- 2.16.2

3 9

[libvirt] [PATCH v2] qemu: Introduce state_lock_timeout to qemu.conf
by Yi Wang 28 Aug '18

28 Aug '18

When doing some job holding state lock for a long time, we may come across error: "Timed out during operation: cannot acquire state change lock" Well, sometimes it's not a problem and users wanner continue to wait, and this patch allow users decide how long time they can wait the state lock. Signed-off-by: Yi Wang <wang.yi59(a)zte.com.cn> Reviewed-by: Xi Xu <xu.xi8(a)zte.com.cn> --- changes in v2: - change default value to 30 in qemu.conf - set the default value in virQEMUDriverConfigNew() --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 8 ++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 5 +---- src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index ddc4bbf..f7287ae 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -93,6 +93,7 @@ module Libvirtd_qemu = | limits_entry "max_core" | bool_entry "dump_guest_core" | str_entry "stdio_handler" + | int_entry "state_lock_timeout" let device_entry = bool_entry "mac_filter" | bool_entry "relaxed_acs_check" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index cd57b3c..4284786 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -813,3 +813,9 @@ # #swtpm_user = "tss" #swtpm_group = "tss" + +# The timeout (in seconds) waiting for acquiring state lock. +# +# Default is 30 +# +#state_lock_timeout = 60 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a4f545e..31e0013 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -129,6 +129,9 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def) #endif +/* Give up waiting for mutex after 30 seconds */ +#define QEMU_JOB_WAIT_TIME (1000ull * 30) + virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) { virQEMUDriverConfigPtr cfg; @@ -346,6 +349,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->glusterDebugLevel = 4; cfg->stdioLogD = true; + cfg->stateLockTimeout = QEMU_JOB_WAIT_TIME; + if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST))) goto error; @@ -863,6 +868,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virConfGetValueUInt(conf, "keepalive_count", &cfg->keepAliveCount) < 0) goto cleanup; + if (virConfGetValueInt(conf, "state_lock_timeout", &cfg->stateLockTimeout) < 0) + goto cleanup; + if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0) goto cleanup; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a8d84ef..97cf2e1 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -190,6 +190,8 @@ struct _virQEMUDriverConfig { int keepAliveInterval; unsigned int keepAliveCount; + int stateLockTimeout; + int seccompSandbox; char *migrateHost; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 886e3fb..5a2ca52 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6652,9 +6652,6 @@ qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv, priv->job.agentActive == QEMU_AGENT_JOB_NONE)); } -/* Give up waiting for mutex after 30 seconds */ -#define QEMU_JOB_WAIT_TIME (1000ull * 30) - /** * qemuDomainObjBeginJobInternal: * @driver: qemu driver @@ -6714,7 +6711,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, } priv->jobs_queued++; - then = now + QEMU_JOB_WAIT_TIME; + then = now + cfg->stateLockTimeout; retry: if ((!async && job != QEMU_JOB_DESTROY) && diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index f1e8806..dc5de96 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -105,3 +105,4 @@ module Test_libvirtd_qemu = { "pr_helper" = "/usr/bin/qemu-pr-helper" } { "swtpm_user" = "tss" } { "swtpm_group" = "tss" } +{ "state_lock_timeout" = "60" } -- 1.8.3.1

5 5

[libvirt] [PATCH] vsh: Fix broken build on mingw
by Simon Kobyda 28 Aug '18

28 Aug '18

The function wcwidth() doesn't exist on mingw. However we can implement this function from gnulib. Broken gnulib on CentOS and RHEL still persists. Signed-off-by: Simon Kobyda <skobyda(a)redhat.com> --- bootstrap.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/bootstrap.conf b/bootstrap.conf index 698a52216b..d9e2b0662c 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -126,6 +126,7 @@ vc-list-files vsnprintf waitpid warnings +wcwidth ' SKIP_PO=true -- 2.17.1

2 1