February 2018 - Devel - libvirt List Archives

[libvirt] [PATCH v2] qemu: Expose rx/tx_queue_size in qemu.conf too
by Michal Privoznik 12 Feb '18

12 Feb '18

In 2074ef6cd4a2 and c56cdf259 (and friends) we've added two attributes to virtio NICs: rx_queue_size and tx_queue_size. However, sysadmins might want to set these on per-host basis but don't necessarily have an access to domain XML (e.g. because they are generated by some other app). So let's expose them under qemu.conf (the settings from domain XML still take precedence as they are more specific ones). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- diff to v1: - Reworded docs and config file comment - Simplified logic in qemuBuildNicDevStr a bit - Make the values require qemu with corresponding capabilities docs/formatdomain.html.in | 14 ++++++++-- src/qemu/libvirtd_qemu.aug | 4 +++ src/qemu/qemu.conf | 6 +++++ src/qemu/qemu_command.c | 55 +++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_conf.c | 4 +++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_hotplug.c | 2 +- src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 9 files changed, 74 insertions(+), 19 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3ec1173c6..6707744bd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5456,7 +5456,12 @@ qemu-kvm -net nic,model=? /dev/null some restrictions on actual value. For instance, latest QEMU (as of 2016-09-01) requires value to be a power of two from [256, 1024] range. - Since 2.3.0 (QEMU and KVM only) + Since 2.3.0 (QEMU and KVM only) + Additionally, since 4.1.0 the + value can be set in the <code>qemu.conf</code> file in order + to override the hypervisor default value. Note that XML has + higher precedence because it's more specific. + In general you should leave this option alone, unless you are very certain you know what you are doing. @@ -5472,7 +5477,12 @@ qemu-kvm -net nic,model=? /dev/null range. In addition to that, this may work only for a subset of interface types, e.g. aforementioned QEMU enables this option only for <code>vhostuser</code> type. - Since 3.7.0 (QEMU and KVM only) + Since 3.7.0 (QEMU and KVM only) + Additionally, since 4.1.0 the + value can be set in the <code>qemu.conf</code> file in order + to override the hypervisor default value. Note that XML has + higher precedence because it's more specific. + In general you should leave this option alone, unless you are very certain you know what you are doing. diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index c19bf3a43..084290296 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -118,6 +118,9 @@ module Libvirtd_qemu = let vxhs_entry = bool_entry "vxhs_tls" | str_entry "vxhs_tls_x509_cert_dir" + let virtio_entry = int_entry "rx_queue_size" + | int_entry "tx_queue_size" + (* Each entry in the config is one of the following ... *) let entry = default_tls_entry | vnc_entry @@ -137,6 +140,7 @@ module Libvirtd_qemu = | gluster_debug_level_entry | memory_entry | vxhs_entry + | virtio_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 43dd561cc..62c4265ea 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -775,3 +775,9 @@ # This directory is used for memoryBacking source if configured as file. # NOTE: big files will be stored here #memory_backing_dir = "/var/lib/libvirt/qemu/ram" + +# The following two values set the default RX/TX ring buffer size for virtio +# interfaces. These values are taken unless overridden in domain XML. For more +# info consult docs to corresponding attributes from domain XML. +#rx_queue_size = 1024 +#tx_queue_size = 1024 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 899f0cbbb..543270028 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3751,7 +3751,8 @@ qemuBuildNicStr(virDomainNetDefPtr net, char * -qemuBuildNicDevStr(virDomainDefPtr def, +qemuBuildNicDevStr(virQEMUDriverConfigPtr cfg, + virDomainDefPtr def, virDomainNetDefPtr net, int vlan, unsigned int bootindex, @@ -3871,21 +3872,41 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2); } } - if (usingVirtio && net->driver.virtio.rx_queue_size) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio rx_queue_size option is not supported with this QEMU binary")); - goto error; + if (usingVirtio) { + unsigned int rx_queue_size = net->driver.virtio.rx_queue_size; + + if (rx_queue_size == 0) + rx_queue_size = cfg->rx_queue_size; + + if (rx_queue_size) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio rx_queue_size option is " + "not supported with this QEMU binary")); + goto error; + } + + net->driver.virtio.rx_queue_size = rx_queue_size; + virBufferAsprintf(&buf, ",rx_queue_size=%u", rx_queue_size); } - virBufferAsprintf(&buf, ",rx_queue_size=%u", net->driver.virtio.rx_queue_size); } - if (usingVirtio && net->driver.virtio.tx_queue_size) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio tx_queue_size option is not supported with this QEMU binary")); - goto error; + if (usingVirtio) { + unsigned int tx_queue_size = net->driver.virtio.tx_queue_size; + + if (tx_queue_size == 0) + tx_queue_size = cfg->tx_queue_size; + + if (tx_queue_size) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio tx_queue_size option is " + "not supported with this QEMU binary")); + goto error; + } + + net->driver.virtio.tx_queue_size = tx_queue_size; + virBufferAsprintf(&buf, ",tx_queue_size=%u", tx_queue_size); } - virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size); } if (usingVirtio && net->mtu) { @@ -8571,7 +8592,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, virCommandAddArg(cmd, netdev); VIR_FREE(netdev); - if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex, + if (!(nic = qemuBuildNicDevStr(cfg, def, net, -1, bootindex, queues, qemuCaps))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error generating NIC -device string")); @@ -8608,6 +8629,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, int **nicindexes, bool chardevStdioLogd) { + virQEMUDriverConfigPtr cfg = NULL; int ret = -1; char *nic = NULL, *host = NULL; int *tapfd = NULL; @@ -8669,6 +8691,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, return -1; } + cfg = virQEMUDriverGetConfig(driver); + switch (actualType) { case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: @@ -8864,7 +8888,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, virCommandAddArgList(cmd, "-netdev", host, NULL); } if (qemuDomainSupportsNicdev(def, net)) { - if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex, + if (!(nic = qemuBuildNicDevStr(cfg, def, net, vlan, bootindex, vhostfdSize, qemuCaps))) goto cleanup; virCommandAddArgList(cmd, "-device", nic, NULL); @@ -8908,6 +8932,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, VIR_FREE(host); VIR_FREE(tapfdName); VIR_FREE(vhostfdName); + virObjectUnref(cfg); return ret; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 31c9da673..644988329 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -90,7 +90,8 @@ char *qemuBuildNicStr(virDomainNetDefPtr net, int vlan); /* Current, best practice */ -char *qemuBuildNicDevStr(virDomainDefPtr def, +char *qemuBuildNicDevStr(virQEMUDriverConfigPtr cfg, + virDomainDefPtr def, virDomainNetDefPtr net, int vlan, unsigned int bootindex, diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index af503d31c..2fa96431f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -912,6 +912,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virConfGetValueString(conf, "memory_backing_dir", &cfg->memoryBackingDir) < 0) goto cleanup; + if (virConfGetValueUInt(conf, "rx_queue_size", &cfg->rx_queue_size) < 0 || + virConfGetValueUInt(conf, "tx_queue_size", &cfg->tx_queue_size) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a553e30e2..3f38a76c2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -206,6 +206,9 @@ struct _virQEMUDriverConfig { bool vxhsTLS; char *vxhsTLSx509certdir; + + unsigned int rx_queue_size; + unsigned int tx_queue_size; }; /* Main driver state */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2aa927e62..0672ed130 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1120,7 +1120,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, for (i = 0; i < vhostfdSize; i++) VIR_FORCE_CLOSE(vhostfd[i]); - if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0, + if (!(nicstr = qemuBuildNicDevStr(cfg, vm->def, net, vlan, 0, queueSize, priv->qemuCaps))) goto try_remove; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 688e5b9fd..4fc4e2f4e 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -100,3 +100,5 @@ module Test_libvirtd_qemu = { "1" = "mount" } } { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" } +{ "rx_queue_size" = "1024" } +{ "tx_queue_size" = "1024" } -- 2.13.6

4 4

[libvirt] [PATCH] tests: fix running of qemuxml2argvtest program
by Daniel P. Berrangé 12 Feb '18

12 Feb '18

The previous commit: commit a455d41e3e1c1af3a36ccdbb2e3f2356cc58993e Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Thu Jan 25 09:35:50 2018 +0000 conf: expand network device callbacks to cover resolving NIC type mistakenly dropped qemuxml2argvtest from the tests due to a typo. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index d013aed5eb..4ff3fa742a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -291,7 +291,7 @@ test_programs += qemuxml2xmltest \ if WITH_NETWORK # Dep on the network driver callback for resolving NIC # actual type. XXX remove this dep. -test_programs += qemuxml2xmltest +test_programs += qemuxml2argvtest endif WITH_NETWORK test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ -- 2.14.3

2 1

[libvirt] [PATCH] virutil: Introduce virGetHostnameSimple()
by Michal Privoznik 12 Feb '18

12 Feb '18

After 759b4d1b0fe5f we are getting hostname in virLogOnceInit(). Problem with this approach is in the NSS module because the module calls some internal APIs which occasionally want to log something. This results in virLogInitialize() to be called which in turn ends up calling virGetHostnameQuiet() and effectively the control gets to NSS plugin again which calls some internal APIs which occasionally want to log something. You can see the deadlock now. One way out of this is to call only gethostname() and not whole virGetHostnameQuiet() machinery. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/libvirt_private.syms | 1 + src/util/virlog.c | 2 +- src/util/virutil.c | 46 ++++++++++++++++++++++++++++++++++------------ src/util/virutil.h | 1 + 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b14d7d15..3ef55f809 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3016,6 +3016,7 @@ virGetGroupList; virGetGroupName; virGetHostname; virGetHostnameQuiet; +virGetHostnameSimple; virGetListenFDs; virGetSelfLastChanged; virGetSystemPageSize; diff --git a/src/util/virlog.c b/src/util/virlog.c index 8f1e4800d..fc854ffeb 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -276,7 +276,7 @@ virLogOnceInit(void) * it might not be possible to load NSS modules via getaddrinfo() * (e.g. at container startup the host filesystem will not be * accessible anymore. */ - virLogHostname = virGetHostnameQuiet(); + virLogHostname = virGetHostnameSimple(); virLogUnlock(); return 0; diff --git a/src/util/virutil.c b/src/util/virutil.c index cd6fbf2f3..22adecd53 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -684,26 +684,23 @@ static char * virGetHostnameImpl(bool quiet) { int r; - char hostname[HOST_NAME_MAX+1], *result = NULL; + char *result; struct addrinfo hints, *info; - r = gethostname(hostname, sizeof(hostname)); - if (r == -1) { + if (!(result = virGetHostnameSimple())) { if (!quiet) virReportSystemError(errno, "%s", _("failed to determine host name")); return NULL; } - NUL_TERMINATE(hostname); - if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) { + if (STRPREFIX(result, "localhost") || strchr(result, '.')) { /* in this case, gethostname returned localhost (meaning we can't * do any further canonicalization), or it returned an FQDN (and * we don't need to do any further canonicalization). Return the * string as-is; it's up to callers to check whether "localhost" * is allowed. */ - ignore_value(VIR_STRDUP_QUIET(result, hostname)); goto cleanup; } @@ -714,12 +711,11 @@ virGetHostnameImpl(bool quiet) memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_CANONNAME|AI_CANONIDN; hints.ai_family = AF_UNSPEC; - r = getaddrinfo(hostname, NULL, &hints, &info); + r = getaddrinfo(result, NULL, &hints, &info); if (r != 0) { if (!quiet) VIR_WARN("getaddrinfo failed for '%s': %s", - hostname, gai_strerror(r)); - ignore_value(VIR_STRDUP_QUIET(result, hostname)); + result, gai_strerror(r)); goto cleanup; } @@ -727,15 +723,16 @@ virGetHostnameImpl(bool quiet) sa_assert(info); if (info->ai_canonname == NULL || - STRPREFIX(info->ai_canonname, "localhost")) + STRPREFIX(info->ai_canonname, "localhost")) { /* in this case, we tried to canonicalize and we ended up back with * localhost. Ignore the canonicalized name and just return the * original hostname */ - ignore_value(VIR_STRDUP_QUIET(result, hostname)); - else + } else { /* Caller frees this string. */ + VIR_FREE(result); ignore_value(VIR_STRDUP_QUIET(result, info->ai_canonname)); + } freeaddrinfo(info); @@ -760,6 +757,31 @@ virGetHostnameQuiet(void) } +/** + * virGetHostnameSimple: + * + * Plain wrapper over gethostname(). The difference to + * virGetHostname() is that this function doesn't try to + * canonicalize the hostname. + * + * Returns: hostname string (caller must free), + * NULL on error. + */ +char * +virGetHostnameSimple(void) +{ + char hostname[HOST_NAME_MAX+1]; + char *ret; + + if (gethostname(hostname, sizeof(hostname)) == -1) + return NULL; + + NUL_TERMINATE(hostname); + ignore_value(VIR_STRDUP_QUIET(ret, hostname)); + return ret; +} + + char * virGetUserDirectory(void) { diff --git a/src/util/virutil.h b/src/util/virutil.h index be0f6b0ea..57148374b 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -136,6 +136,7 @@ static inline int pthread_sigmask(int how, char *virGetHostname(void); char *virGetHostnameQuiet(void); +char *virGetHostnameSimple(void); char *virGetUserDirectory(void); char *virGetUserDirectoryByUID(uid_t uid); -- 2.13.6

2 2

[libvirt] [PATCH v3 00/15] Misc build refactoring / isolation work
by Daniel P. Berrangé 12 Feb '18

12 Feb '18

This was triggered by the recent Fedora change to add '-z defs' to RPM builds by default which breaks libvirt. Various make rule changes can fix much of the problem, but it also requires source refactoring to get rid of places where virt drivers directly call into the storage/network drivers. Co-incidentally this work will also be useful in allowing us to separate out drivers to distinct daemons. In v3: - Fixed a few build problems identified by travis In v2: - Fixed header file name comment - Resolve conflicts - Fix unit tests - Fix bisectable build by moving libvirt_lxc build patch earlier - Update syntax check header include rule Daniel P. Berrangé (15): storage: extract storage file backend from main storage driver backend storage: move storage file backend framework into util directory rpc: don't link in second copy of RPC code to libvirtd & lockd plugin build: link libvirt_lxc against libvirt.so conf: introduce callback registration for domain net device allocation conf: expand network device callbacks to cover bandwidth updates qemu: replace networkGetNetworkAddress with public API calls conf: expand network device callbacks to cover resolving NIC type network: remove conditional declarations conf: move virStorageTranslateDiskSourcePool into domain conf storage: export virStoragePoolLookupByTargetPath as a public API build: explicitly link all modules with libvirt.so build: provide a AM_FLAGS_MOD for loadable modules build: passing the "-z defs" linker flag to prevent undefined symbols cfg: forbid includes of headers in network and storage drivers again cfg.mk | 2 +- configure.ac | 1 + daemon/Makefile.am | 3 +- include/libvirt/libvirt-storage.h | 2 + m4/virt-linker-no-undefined.m4 | 32 ++ po/POTFILES.in | 2 +- src/Makefile.am | 150 ++++---- src/bhyve/bhyve_command.c | 7 +- src/conf/domain_conf.c | 355 +++++++++++++++++++ src/conf/domain_conf.h | 71 ++++ src/driver-storage.h | 5 + src/libvirt-storage.c | 40 +++ src/libvirt_private.syms | 29 ++ src/libvirt_public.syms | 6 + src/libvirt_remote.syms | 11 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 7 +- src/lxc/lxc_driver.c | 5 +- src/lxc/lxc_process.c | 7 +- src/network/bridge_driver.c | 124 +------ src/network/bridge_driver.h | 72 ---- src/qemu/qemu_alias.c | 3 +- src/qemu/qemu_command.c | 1 - src/qemu/qemu_domain.c | 3 - src/qemu/qemu_domain_address.c | 3 +- src/qemu/qemu_driver.c | 15 +- src/qemu/qemu_hotplug.c | 18 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_process.c | 115 +++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 17 +- src/remote_protocol-structs | 7 + src/security/virt-aa-helper.c | 2 - src/storage/storage_backend.c | 66 ---- src/storage/storage_backend.h | 75 ---- src/storage/storage_backend_fs.c | 8 +- src/storage/storage_backend_gluster.c | 4 +- src/storage/storage_driver.c | 256 +------------- src/storage/storage_driver.h | 3 - src/storage/storage_source.c | 645 ---------------------------------- src/storage/storage_source.h | 59 ---- src/util/virstoragefile.c | 609 +++++++++++++++++++++++++++++++- src/util/virstoragefile.h | 32 ++ src/util/virstoragefilebackend.c | 108 ++++++ src/util/virstoragefilebackend.h | 104 ++++++ src/vz/vz_sdk.c | 1 - tests/Makefile.am | 11 +- tests/qemuxml2argvtest.c | 4 + tests/virstoragetest.c | 1 - tools/Makefile.am | 1 + 50 files changed, 1679 insertions(+), 1432 deletions(-) create mode 100644 m4/virt-linker-no-undefined.m4 delete mode 100644 src/storage/storage_source.c delete mode 100644 src/storage/storage_source.h create mode 100644 src/util/virstoragefilebackend.c create mode 100644 src/util/virstoragefilebackend.h -- 2.14.3

3 19

[libvirt] [PATCH] virlog: Allow opting out from logging
by Michal Privoznik 12 Feb '18

12 Feb '18

After 759b4d1b0fe5f we are getting hostname in virLogOnceInit(). Problem with this approach is in the NSS module because the module calls some internal APIs which occasionally want to log something. This results in virLogInitialize() to be called which in turn ends up calling virGetHostnameQuiet() and effectively the control gets to NSS plugin again which calls some internal APIs which occasionally want to log something. You can see the deadlock now. One way out from this is to turn logging into no-op. Which kind of makes sense - the NSS module shouldn't log anything. To achieve this, the virLogVMessage() function is provided with separate no-op implementation if DISABLE_LOGGING_FOR_NSS macro is set. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- config-post.h | 1 + src/util/virlog.c | 119 ++++++++++++++++++++++++++++++++---------------------- 2 files changed, 71 insertions(+), 49 deletions(-) diff --git a/config-post.h b/config-post.h index f7eba0d7c..95e834f8e 100644 --- a/config-post.h +++ b/config-post.h @@ -72,6 +72,7 @@ # undef WITH_SECDRIVER_SELINUX # undef WITH_SECDRIVER_APPARMOR # undef WITH_CAPNG +# define DISABLE_LOGGING_FOR_NSS #endif /* LIBVIRT_NSS */ #ifndef __GNUC__ diff --git a/src/util/virlog.c b/src/util/virlog.c index 8f1e4800d..3284bdc03 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -241,23 +241,6 @@ virLogGetDefaultOutput(void) } -static const char * -virLogPriorityString(virLogPriority lvl) -{ - switch (lvl) { - case VIR_LOG_DEBUG: - return "debug"; - case VIR_LOG_INFO: - return "info"; - case VIR_LOG_WARN: - return "warning"; - case VIR_LOG_ERROR: - return "error"; - } - return "unknown"; -} - - static int virLogOnceInit(void) { @@ -429,6 +412,60 @@ virLogOutputListFree(virLogOutputPtr *list, int count) } +/** + * virLogMessage: + * @source: where is that message coming from + * @priority: the priority level + * @filename: file where the message was emitted + * @linenr: line where the message was emitted + * @funcname: the function emitting the (debug) message + * @metadata: NULL or metadata array, terminated by an item with NULL key + * @fmt: the string format + * @...: the arguments + * + * Call the libvirt logger with some information. Based on the configuration + * the message may be stored, sent to output or just discarded + */ +void +virLogMessage(virLogSourcePtr source, + virLogPriority priority, + const char *filename, + int linenr, + const char *funcname, + virLogMetadataPtr metadata, + const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + virLogVMessage(source, priority, + filename, linenr, funcname, + metadata, fmt, ap); + va_end(ap); +} + + +/* For the library we compile in logging code and then turn it + * on/off depending on user settings. However, for the NSS plugin + * we might get into deadlock and logging there is not really + * needed. */ +#ifndef DISABLE_LOGGING_FOR_NSS +static const char * +virLogPriorityString(virLogPriority lvl) +{ + switch (lvl) { + case VIR_LOG_DEBUG: + return "debug"; + case VIR_LOG_INFO: + return "info"; + case VIR_LOG_WARN: + return "warning"; + case VIR_LOG_ERROR: + return "error"; + } + return "unknown"; +} + + static int virLogFormatString(char **msg, int linenr, @@ -514,38 +551,6 @@ virLogSourceUpdate(virLogSourcePtr source) virLogUnlock(); } -/** - * virLogMessage: - * @source: where is that message coming from - * @priority: the priority level - * @filename: file where the message was emitted - * @linenr: line where the message was emitted - * @funcname: the function emitting the (debug) message - * @metadata: NULL or metadata array, terminated by an item with NULL key - * @fmt: the string format - * @...: the arguments - * - * Call the libvirt logger with some information. Based on the configuration - * the message may be stored, sent to output or just discarded - */ -void -virLogMessage(virLogSourcePtr source, - virLogPriority priority, - const char *filename, - int linenr, - const char *funcname, - virLogMetadataPtr metadata, - const char *fmt, ...) -{ - va_list ap; - va_start(ap, fmt); - virLogVMessage(source, priority, - filename, linenr, funcname, - metadata, fmt, ap); - va_end(ap); -} - - /** * virLogVMessage: * @source: where is that message coming from @@ -678,6 +683,22 @@ virLogVMessage(virLogSourcePtr source, errno = saved_errno; } +#else /* DISABLE_LOGGING_FOR_NSS */ + +void +virLogVMessage(virLogSourcePtr source ATTRIBUTE_UNUSED, + virLogPriority priority ATTRIBUTE_UNUSED, + const char *filename ATTRIBUTE_UNUSED, + int linenr ATTRIBUTE_UNUSED, + const char *funcname ATTRIBUTE_UNUSED, + virLogMetadataPtr metadata ATTRIBUTE_UNUSED, + const char *fmt ATTRIBUTE_UNUSED, + va_list vargs ATTRIBUTE_UNUSED) +{ + return; +} + +#endif /* DISABLE_LOGGING_FOR_NSS */ static void virLogStackTraceToFd(int fd) -- 2.13.6

2 1

[libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX
by Daniel P. Berrangé 12 Feb '18

12 Feb '18

From: Lubomir Rintel <lkundrak(a)v3.sk> At later point it might not be possible or even safe to use getaddrinfo(). It can in turn result in a load of NSS module. Notably, on a LXC container startup we may find ourselves with the guest filesystem already having replaced the host one. Loading a NSS module from the guest tree could allow a malicous guest to escape the confinement of its container environment because libvirt will not yet have locked it down. --- NB, we're still awaiting CVE allocation before pushing to git src/util/virlog.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 68439b9194..9105337ce6 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -64,6 +64,7 @@ VIR_LOG_INIT("util.log"); static regex_t *virLogRegex; +static char *virLogHostname; #define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}" @@ -271,6 +272,12 @@ virLogOnceInit(void) VIR_FREE(virLogRegex); } + /* We get and remember the hostname early, because at later time + * it might not be possible to load NSS modules via getaddrinfo() + * (e.g. at container startup the host filesystem will not be + * accessible anymore. */ + virLogHostname = virGetHostnameQuiet(); + virLogUnlock(); return 0; } @@ -466,17 +473,14 @@ static int virLogHostnameString(char **rawmsg, char **msg) { - char *hostname = virGetHostnameQuiet(); char *hoststr; - if (!hostname) + if (!virLogHostname) return -1; - if (virAsprintfQuiet(&hoststr, "hostname: %s", hostname) < 0) { - VIR_FREE(hostname); + if (virAsprintfQuiet(&hoststr, "hostname: %s", virLogHostname) < 0) { return -1; } - VIR_FREE(hostname); if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) { VIR_FREE(hoststr); -- 2.14.3

3 4

[libvirt] [PATCH] qemu: Alter condition to avoid possible NULL deref
by John Ferlan 12 Feb '18

12 Feb '18

Commit 'f0f2a5ec2' neglected to adjust the if condition to split out the possibility that the @watchdog is NULL when altering the message to add detail about the model. Just split out the condition and use previous/original message, but with the new message code. Found by Coverity Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/qemu/qemu_hotplug.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c7bf25eee..3291ce613 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5159,11 +5159,16 @@ qemuDomainDetachWatchdog(virQEMUDriverPtr driver, virDomainWatchdogDefPtr watchdog = vm->def->watchdog; qemuDomainObjPrivatePtr priv = vm->privateData; + if (!watchdog) { + virReportError(VIR_ERR_DEVICE_MISSING, "%s", + _("watchdog device not present in domain configuration")); + return -1; + } + /* While domains can have up to one watchdog, the one supplied by the user * doesn't necessarily match the one domain has. Refuse to detach in such * case. */ - if (!(watchdog && - watchdog->model == dev->model && + if (!(watchdog->model == dev->model && watchdog->action == dev->action && virDomainDeviceInfoAddressIsEqual(&dev->info, &watchdog->info))) { virReportError(VIR_ERR_DEVICE_MISSING, -- 2.13.6

3 2

[libvirt] [PATCH] qemu: don't leak in qemuGetDHCPInterfaces when failing to alloc
by Chen Hanxiao 12 Feb '18

12 Feb '18

From: Chen Hanxiao <chenhanxiao(a)gmail.com> We forgot to free alloced mem when failed to dup ifname or macaddr. Also use VIR_STEAL_PTR to simplify codes. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- src/qemu/qemu_agent.c | 3 +-- src/qemu/qemu_driver.c | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5d125c413..0f36054a6 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2190,8 +2190,7 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, iface->naddrs = addrs_count; } - *ifaces = ifaces_ret; - ifaces_ret = NULL; + VIR_STEAL_PTR(*ifaces, ifaces_ret); ret = ifaces_count; cleanup: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 978ecd4e0..60cdd237a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20569,10 +20569,10 @@ qemuGetDHCPInterfaces(virDomainPtr dom, goto error; if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0) - goto cleanup; + goto error; if (VIR_STRDUP(iface->hwaddr, macaddr) < 0) - goto cleanup; + goto error; } for (j = 0; j < n_leases; j++) { @@ -20592,8 +20592,7 @@ qemuGetDHCPInterfaces(virDomainPtr dom, VIR_FREE(leases); } - *ifaces = ifaces_ret; - ifaces_ret = NULL; + VIR_STEAL_PTR(*ifaces, ifaces_ret); rv = ifaces_count; cleanup: -- 2.14.3

2 1

[libvirt] [PATCH] util: netlink: fix the mismatch parameter description of functions
by Chen Hanxiao 09 Feb '18

09 Feb '18

From: Chen Hanxiao <chenhanxiao(a)gmail.com> Some of netlink functions don't have the right @parameters description according to the declaration of function. This patch fix them. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- src/util/virnetlink.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index d732fe8cf..6a6508fa4 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -278,14 +278,14 @@ virNetlinkSendRequest(struct nl_msg *nl_msg, uint32_t src_pid, /** * virNetlinkCommand: - * @nlmsg: pointer to netlink message - * @respbuf: pointer to pointer where response buffer will be allocated + * @nl_msg: pointer to netlink message + * @resp: pointer to pointer where response buffer will be allocated * @respbuflen: pointer to integer holding the size of the response buffer - * on return of the function. - * @src_pid: the pid of the process to send a message - * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel - * @protocol: netlink protocol - * @groups: the group identifier + * on return of the function. + * @src_pid: the pid of the process to send a message + * @dst_pid: the pid of the process to talk to, i.e., pid = 0 for kernel + * @protocol: netlink protocol + * @groups: the group identifier * * Send the given message to the netlink layer and receive response. * Returns 0 on success, -1 on error. In case of error, no response @@ -387,9 +387,9 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, * * @ifname: The name of the interface; only use if ifindex <= 0 * @ifindex: The interface index; may be <= 0 if ifname is given - * @data: Gets a pointer to the raw data from netlink. + * @nlData: Gets a pointer to the raw data from netlink. MUST BE FREED BY CALLER! - * @nlattr: Pointer to a pointer of netlink attributes that will contain + * @tb: Pointer to a pointer of netlink attributes that will contain * the results * @src_pid: pid used for nl_pid of the local end of the netlink message * (0 == "use getpid()") @@ -505,7 +505,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex, /** * virNetlinkDelLink: * - * @ifname: Name of the link + * @ifname: Name of the link * @fallback: pointer to an alternate function that will * be called to perform the delete if RTM_DELLINK fails * with EOPNOTSUPP (any other error will simply be treated @@ -648,7 +648,7 @@ virNetlinkEventServerUnlock(virNetlinkEventSrvPrivatePtr driver) /** * virNetlinkEventRemoveClientPrimitive: * - * @i: index of the client to remove from the table + * @i: index of the client to remove from the table * @protocol: netlink protocol * * This static function does the low level removal of a client from @@ -837,7 +837,8 @@ int virNetlinkEventServiceLocalPid(unsigned int protocol) * This registers a netlink socket with the event interface. * * @protocol: netlink protocol - * @groups: broadcast groups to join in + * @groups: broadcast groups to join in + * * Returns -1 if the monitor cannot be registered, 0 upon success */ int @@ -925,9 +926,9 @@ virNetlinkEventServiceStart(unsigned int protocol, unsigned int groups) * * @handleCB: callback to invoke when an event occurs * @removeCB: callback to invoke when removing a client - * @opaque: user data to pass to callback - * @macaddr: macaddr to store with the data. Used to identify callers. - * May be null. + * @opaque: user data to pass to callback + * @macaddr: macaddr to store with the data. Used to identify callers. + * May be null. * @protocol: netlink protocol * * register a callback for handling of netlink messages. The @@ -1003,8 +1004,8 @@ virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB, /** * virNetlinkEventRemoveClient: * - * @watch: watch whose handle to remove - * @macaddr: macaddr whose handle to remove + * @watch: watch whose handle to remove + * @macaddr: macaddr whose handle to remove * @protocol: netlink protocol * * Unregister a callback from a netlink monitor. -- 2.14.3

2 1

[libvirt] [PATCH] qemu: Remove redundancy from qemuBuildControllerDevStr()
by Andrea Bolognani 09 Feb '18

09 Feb '18

Several PCI controllers are handled the same and can thus be squashed together. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- src/qemu/qemu_command.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ee4e0b20d..040ea65b6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2722,6 +2722,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s", modelName, pciopts->busNr, def->info.alias); @@ -2730,29 +2731,15 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, pciopts->numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", - modelName, pciopts->port, - pciopts->chassis, def->info.alias); - break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: virBufferAsprintf(&buf, "%s,id=%s", modelName, def->info.alias); break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", modelName, pciopts->port, pciopts->chassis, def->info.alias); break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - virBufferAsprintf(&buf, "%s,bus_nr=%d,id=%s", - modelName, pciopts->busNr, - def->info.alias); - if (pciopts->numaNode != -1) - virBufferAsprintf(&buf, ",numa_node=%d", - pciopts->numaNode); - break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: /* Skip the implicit one */ if (pciopts->targetIndex == 0) -- 2.14.3

2 1