- Devel - libvirt List Archives

[libvirt] [PATCH v3] socket: allow wait=false for client socket
by Marc-André Lureau 15 Apr '19

15 Apr '19

Commit 767abe7 ("chardev: forbid 'wait' option with client sockets") is a bit too strict. Current libvirt always set wait=false, and will thus fail to add client chardev. Make the code more permissive, allowing wait=false with client socket chardevs. Deprecate usage of 'wait' with client sockets. Fixes: 767abe7f49e8be14d29da5db3527817b5d696a52 Cc: Daniel P. Berrangé <berrange(a)redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com> --- chardev/char-socket.c | 10 +++++++--- qemu-deprecated.texi | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 3916505d67..ebc1adb83e 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -1263,9 +1263,13 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock, return false; } if (sock->has_wait) { - error_setg(errp, "%s", - "'wait' option is incompatible with " - "socket in client connect mode"); + warn_report("'wait' option is deprecated with " + "socket in client connect mode"); + if (sock->wait) { + error_setg(errp, "%s", + "'wait' option is incompatible with " + "socket in client connect mode"); + } return false; } } diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 2219386769..842e71b11d 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -105,6 +105,11 @@ details. The ``query-events'' command has been superseded by the more powerful and accurate ``query-qmp-schema'' command. +@subsection chardev client socket with 'wait' option (since 4.0) + +Character devices creating sockets in client mode should not specify +the 'wait' field, which is only applicable to sockets in server mode + @section Human Monitor Protocol (HMP) commands @subsection The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove' (since 3.1) -- 2.21.0.313.ge35b8cb8e2

2 2

[libvirt] [PATCH] tools: Reduce table width in virsh(1)
by Andrea Bolognani 15 Apr '19

15 Apr '19

The table included in the sample output for 'list --title' is unnecessarily wide, which causes man to complain: warning [p 8, 0.5i]: can't break line Make the table narrower. Spotted by Lintian (manpage-has-errors-from-man tag). Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- tools/virsh.pod | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index bb52de3eed..3196636825 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -545,10 +545,10 @@ I<--table> output. Example: B<virsh> list --title - Id Name State Title - -------------------------------------------------------------------------- - 0 Domain-0 running Mailserver 1 - 2 fedora paused + Id Name State Title + ------------------------------------------- + 0 Domain-0 running Mailserver 1 + 2 fedora paused =item B<freecell> [{ [I<--cellno>] B<cellno> | I<--all> }] -- 2.20.1

2 1

[libvirt] [PATCH] tools: Fix grammar
by Andrea Bolognani 15 Apr '19

15 Apr '19

Apparently "allow(s) to frobnicate" is not correct English, and either "allow(s) one to frobnicate" or "allow(s) frobnicating" should be used instead. Spotted by Lintian (spelling-error-in-{binary,manpage} tags). Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- tools/virsh-domain.c | 3 ++- tools/virsh.pod | 20 ++++++++++---------- tools/virt-admin.pod | 4 ++-- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d99b9afda9..da57d96d1a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8313,7 +8313,8 @@ static const vshCmdInfo info_desc[] = { .data = N_("show or set domain's description or title") }, {.name = "desc", - .data = N_("Allows to show or modify description or title of a domain.") + .data = N_("Allows setting or modifying the description or title of " + "a domain.") }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 3196636825..afc1684db0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -778,7 +778,7 @@ effect on the next boot. New description or title message] Show or modify description and title of a domain. These values are user -fields that allow to store arbitrary textual data to allow easy +fields that allow storing arbitrary textual data to allow easy identification of domains. Title should be short, although it's not enforced. (See also B<metadata> that works with XML based domain metadata.) @@ -1303,7 +1303,7 @@ within a certain range. Specifying I<buf-size> will control how much data can be simultaneously in-flight during the copy; larger values use more memory but may allow faster completion (the default value is usually correct). -I<--transient-job> allows to specify that the user does not require the job to +I<--transient-job> allows specifying that the user does not require the job to be recovered if the VM crashes or is turned off before the job completes. This flag removes the restriction of copy jobs to transient domains if that restriction is applied by the hypervisor. @@ -1450,9 +1450,9 @@ bytes/s. Specifying a negative value is interpreted as an unsigned long value or essentially unlimited. The hypervisor can choose whether to reject the value or convert it to the maximum value allowed. Optionally a scaled positive number may be used as bandwidth (see B<NOTES> above). Using -I<--bytes> with a scaled value allows to use finer granularity. A scaled value -used without I<--bytes> will be rounded down to MiB/s. Note that the -I<--bytes> may be unsupported by the hypervisor. +I<--bytes> with a scaled value permits a finer granularity to be selected. +A scaled value used without I<--bytes> will be rounded down to MiB/s. Note +that the I<--bytes> may be unsupported by the hypervisor. =item B<domblkthreshold> I<domain> I<dev> I<threshold> @@ -1876,7 +1876,7 @@ stats. [I<--edit>] [I<uri>] [I<key>] [I<set>] [I<--remove>] Show or modify custom XML metadata of a domain. The metadata is a user -defined XML that allows to store arbitrary XML data in the domain definition. +defined XML that allows storing arbitrary XML data in the domain definition. Multiple separate custom metadata pieces can be stored in the domain XML. The pieces are identified by a private XML namespace provided via the I<uri> argument. (See also B<desc> that works with textual metadata of @@ -2327,8 +2327,8 @@ either parameter is the same as not specifying that parameter. =item B<screenshot> I<domain> [I<imagefilepath>] [I<--screen> B<screenID>] Takes a screenshot of a current domain console and stores it into a file. -Optionally, if hypervisor supports more displays for a domain, I<screenID> -allows to specify which screen will be captured. It is the sequential number +Optionally, if the hypervisor supports more displays for a domain, I<screenID> +allows specifying which screen will be captured. It is the sequential number of screen. In case of multiple graphics cards, heads are enumerated before devices, e.g. having two graphics cards, both with four heads, screen ID 5 addresses the second head on the second card. @@ -3392,8 +3392,8 @@ expected. Change media of CDROM or floppy drive. I<path> can be the fully-qualified path or the unique target name (<target dev='hdc'>) of the disk device. I<source> -specifies the path of the media to be inserted or updated. Flag I<--block> -allows to set the backing type in case a block device is used as media for the +specifies the path of the media to be inserted or updated. The I<--block> flag +allows setting the backing type in case a block device is used as media for the CDROM or floppy drive instead of a file. I<--eject> indicates the media will be ejected. diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index f06ee9247a..1ea6defa0e 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -149,8 +149,8 @@ libvirtd:///system is used. =head1 DAEMON COMMANDS -The following commands allow to monitor the daemon's state as well as directly -change its internal configuration. +The following commands allow one to monitor the daemon's state as well as +directly change its internal configuration. =over 4 -- 2.20.1

2 1

[libvirt] [PATCH 0/2] util: Fix NAME section for virkey{code, name}-*
by Andrea Bolognani 15 Apr '19

15 Apr '19

Andrea Bolognani (2): keycodemapdb: Update submodule util: Fix NAME section for virkey{code,name}-* src/keycodemapdb | 2 +- src/util/Makefile.inc.am | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) -- 2.20.1

2 6

[libvirt] vcpupin reports bogus vcpu affinities
by Allen, John 15 Apr '19

15 Apr '19

For pinned vcpus, vcpupin will report inaccurate affinity values on machines with high core counts (256 cores in my case). The problem is produced as follows: $ virsh vcpupin myguest 0 4 $ virsh vcpupin myguest 0 VCPU CPU Affinity --------------------------- 0 4,192,194,196-197 Running taskset on the qemu threads shows the correct affinity, so this seems to be a reporting problem. Strangely, the value "192" is significant. If I pin a cpu greater than 192, the problem no longer appears. I believe the cause of the problem in my case is that in this case in src/conf/domain_conf.c:virDomainDefGetVcpuPinInfoHelper: ... if (vcpu && vcpu->cpumask) bitmap = vcpu->cpumask; ... vcpu->cpumask is "shortened" in that it is only long enough to contain the last set bit in the mask. However, when we go to copy the mask to the buffer that is returned, we use the masklen passed to the function which is the "full" masklen with a bit for each cpu. So it seems virBitmapToDataBuf copies some extra data past the end of the bitmask. Why the "192" value is always set and I typically see similar bogus bits set is still unknown. What is the function meant to assume in this case? Is it sane to assume that the bitmask is the full length of the buffer here and it's the responsibility of the setter of vcpu->cpumask to provide the length of the bitmap we're expecting? Or should we assume that we may receive a shortened bitmask here and expand the bitmask before copying to the buffer? -John

2 2

[libvirt] [PATCH] Don't hardcode list of git submodules
by Andrea Bolognani 15 Apr '19

15 Apr '19

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Makefile.ci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.ci b/Makefile.ci index 738da53cc8..864948dc96 100644 --- a/Makefile.ci +++ b/Makefile.ci @@ -44,7 +44,7 @@ CI_CONFIGURE_ARGS = # Avoid pulling submodules over the network by locally # cloning them -CI_SUBMODULES = .gnulib src/keycodemapdb +CI_SUBMODULES = $(shell git submodules | awk '{ print $2 }') # Location of the Docker images we're going to pull # Can be useful to overridde to use a locally built -- 2.20.1

3 4

[libvirt] [PATCH 0/5] Add support for setting the emulator scheduler parameters
by Martin Kletzander 15 Apr '19

15 Apr '19

I/O threads and vCPU threads already support setting schedulers, but until now it was impossible to do so for the main QEMU thread (emulator thread in the libvirt naming). This is, however, requested for some very specific scenarios, for example when vCPU threads are running at such priority that could starve the main thread. Martin Kletzander (5): conf: Parse common scheduler attributes in separate function conf: Format thread IDs optionally docs: Mention iothreadsched element in the docs and reword conf: Add support for emulatorsched qemu: Add support for emulatorsched docs/formatdomain.html.in | 25 +++-- docs/news.xml | 12 +++ docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 136 +++++++++++++++++------- src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 2 +- tests/genericxml2xmlindata/cputune.xml | 37 +++++++ tests/genericxml2xmloutdata/cputune.xml | 40 +++++++ tests/genericxml2xmltest.c | 2 + 9 files changed, 212 insertions(+), 48 deletions(-) create mode 100644 tests/genericxml2xmlindata/cputune.xml create mode 100644 tests/genericxml2xmloutdata/cputune.xml -- 2.21.0

1 6

[libvirt] [keycodemapdb PATCH] keymap-gen: Name sections in pod output
by Andrea Bolognani 15 Apr '19

15 Apr '19

Properly formatted man pages have at least a NAME section, which tools like apropos(1) parse and index. The current implementation of PodLanguageGenerator doesn't generate this section, and even if the user were to try and work around the limitation by passing --varname=NAME, there would still be nothing in the section to actually parse. Change the generator so that varname will be used as the contents of the NAME section, add a DESCRIPTION section in which values are listed. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Dan said on IRC he would accept keycodemapdb patches sent to the mailing list of any project using the module, so here we are :) I need this to fix generation of virkey{code,name}-* man pages anyway. tools/keymap-gen | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/keymap-gen b/tools/keymap-gen index 7965d37..78676a9 100755 --- a/tools/keymap-gen +++ b/tools/keymap-gen @@ -873,7 +873,11 @@ class PodLanguageGenerator(LanguageDocGenerator): print("#") def _array_start_name_doc(self, varname, namemap): - print("=head1 %s" % varname) + print("=head1 NAME") + print("") + print("%s" % varname) + print("") + print("=head1 DESCRIPTION") print("") print("List of %s key code names, with corresponding key code values" % namemap) print("") @@ -881,7 +885,11 @@ class PodLanguageGenerator(LanguageDocGenerator): print("") def _array_start_code_doc(self, varname, codemap, namemap): - print("=head1 %s" % varname) + print("=head1 NAME") + print("") + print("%s" % varname) + print("") + print("=head1 DESCRIPTION") print("") print("List of %s key code values, with corresponding %s key code names" % (codemap, namemap)) print("") -- 2.20.1

2 1

[libvirt] [PATCH] qemu: Don't cache microcode version
by Jiri Denemark 15 Apr '19

15 Apr '19

My earlier commit be46f61326 was incomplete. It removed caching of microcode version in the CPU driver, which means the capabilities XML will see the correct microcode version. But it is also cached in the QEMU capabilities cache where it is used to detect whether we need to reprobe QEMU. By missing the second place, the original commit be46f61326 made the situation even worse since libvirt would report correct microcode version while still using the old host CPU model (visible in domain capabilities XML). Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_capabilities.c | 12 ++++++++---- src/qemu/qemu_capabilities.h | 3 +-- src/qemu/qemu_driver.c | 9 +-------- tests/testutilsqemu.c | 2 +- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index da90cd666e..a74dc84056 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4542,7 +4542,7 @@ virQEMUCapsNewData(const char *binary, priv->libDir, priv->runUid, priv->runGid, - priv->microcodeVersion, + virHostCPUGetMicrocodeVersion(), priv->kernelVersion); } @@ -4625,8 +4625,7 @@ virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t runUid, - gid_t runGid, - unsigned int microcodeVersion) + gid_t runGid) { char *capsCacheDir = NULL; virFileCachePtr cache = NULL; @@ -4650,7 +4649,6 @@ virQEMUCapsCacheNew(const char *libDir, priv->runUid = runUid; priv->runGid = runGid; - priv->microcodeVersion = microcodeVersion; priv->kvmUsable = VIR_TRISTATE_BOOL_ABSENT; if (uname(&uts) == 0 && @@ -4672,8 +4670,11 @@ virQEMUCapsPtr virQEMUCapsCacheLookup(virFileCachePtr cache, const char *binary) { + virQEMUCapsCachePrivPtr priv = virFileCacheGetPriv(cache); virQEMUCapsPtr ret = NULL; + priv->microcodeVersion = virHostCPUGetMicrocodeVersion(); + ret = virFileCacheLookup(cache, binary); VIR_DEBUG("Returning caps %p for %s", ret, binary); @@ -4727,6 +4728,7 @@ virQEMUCapsPtr virQEMUCapsCacheLookupByArch(virFileCachePtr cache, virArch arch) { + virQEMUCapsCachePrivPtr priv = virFileCacheGetPriv(cache); virQEMUCapsPtr ret = NULL; const char *binaryFilters[] = { "qemu-system-", @@ -4739,6 +4741,8 @@ virQEMUCapsCacheLookupByArch(virFileCachePtr cache, size_t i; size_t j; + priv->microcodeVersion = virHostCPUGetMicrocodeVersion(); + for (i = 0; i < ARRAY_CARDINALITY(binaryFilters); i++) { for (j = 0; j < ARRAY_CARDINALITY(archs); j++) { struct virQEMUCapsSearchData data = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index fd7bec7589..8f5656a89b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -590,8 +590,7 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid, - gid_t gid, - unsigned int microcodeVersion); + gid_t gid); virQEMUCapsPtr virQEMUCapsCacheLookup(virFileCachePtr cache, const char *binary); virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virFileCachePtr cache, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7e334d0d8d..4510b0ce60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -584,8 +584,6 @@ qemuStateInitialize(bool privileged, char *hugepagePath = NULL; char *memoryBackingPath = NULL; size_t i; - virCPUDefPtr hostCPU = NULL; - unsigned int microcodeVersion = 0; if (VIR_ALLOC(qemu_driver) < 0) return -1; @@ -808,15 +806,10 @@ qemuStateInitialize(bool privileged, run_gid = cfg->group; } - if ((hostCPU = virCPUProbeHost(virArchFromHost()))) - microcodeVersion = hostCPU->microcodeVersion; - virCPUDefFree(hostCPU); - qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir, cfg->cacheDir, run_uid, - run_gid, - microcodeVersion); + run_gid); if (!qemu_driver->qemuCapsCache) goto error; diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 3ee8a0c6c6..763f511bcf 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -740,7 +740,7 @@ int qemuTestDriverInit(virQEMUDriver *driver) /* Using /dev/null for libDir and cacheDir automatically produces errors * upon attempt to use any of them */ - driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0, 0); + driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0); if (!driver->qemuCapsCache) goto error; -- 2.21.0

2 1

[libvirt] Cleaning up libvirt.git tags
by Andrea Bolognani 15 Apr '19

15 Apr '19

We have four types of tags in libvirt.git right now: * vX.Y.Z: we have one of these per release, starting with v0.1.0 from April 2006 and going all the way to v5.2.0 from April 2019 (but see below). Most of them[1] are PGP-signed. * LIBVIR(T)_X_Y_Z: these point to the same commit as the ones above, but the last one we created is for v0.6.5 from July 2009; not a single one is PGP-signed. * CVE-YYYY-*: these point to the commit fixing the corresponding vulnerability. We seem to have stopped creating them in 2017, even though we definitely have had to deal with CVEs since ;) * LIBXEN_FIRST_COMMIT: one-of-a-kind tag pointing to the second ever (!) libvirt (!) commit. Of course the project was still called libxen back then, but this is the only surviving tag to reference the old name. Interestingly[2] enough, a few releases seem to have partially or completely slipped through the cracks: version commit tag tarball --------- -------------- ----- --------- v0.1.2 | 567b42ce6a07 | no | no v0.1.5 | 786e029cd743 | no | yes v0.4.0 | 6cb028991705 | no | yes v0.4.3 | 7db4c905d745 | no | yes v0.4.5 | 9d3d43436eac | no | yes Note that I stopped checking at v0.6.5, so there might actually be more. Anyway, since the LIBVIR(T)_X_Y_Z tags are a strict subset of the vX.Y.Z tags, and we have abandoned that naming scheme a decade ago, all they're doing right now is clutter the output of 'git tag' and I would suggest getting rid of them; same goes for that lonely, lonely LIBXEN_FIRST_COMMIT tag, which serves no real purpose other than reminding us that the project was named differently for, like, an entire month. As for the missing release commits, I see no harm in creating them retroactively for completeness' sake, but if nobody can be bothered doing that I'll also fully understand :) The CVE tags... I really don't have an opinion there. Thoughts? [1] Though not all of them: I'm pointing my finger at you, Cole! :P [2] Well, to me at least. -- Andrea Bolognani / Red Hat / Virtualization

3 4