November 2019 - Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Warn verbosely if using old loader:nvram pairs
by Michal Privoznik 14 Nov '19

14 Nov '19

There are two ways for specifying loader:nvram pairs: 1) --with-loader-nvram configure option 2) nvram variable in qemu.conf Since we have FW descriptors, using this old style is discouraged, but not as strong as one would expect. Produce more warnings: 1) produce a warning if somebody tries the configure option 2) produce a warning if somebody sets nvram variable and at least on FW descriptor was found The reason for producing warning in case 1) is that package maintainers, who set the configure option in the first place should start moving towards FW descriptors and abandon the configure option. After all, the warning is printed into config output only in this case. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1763477 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- m4/virt-loader-nvram.m4 | 10 +++++++++- src/qemu/qemu.conf | 3 +++ src/qemu/qemu_conf.c | 19 +++++++++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/m4/virt-loader-nvram.m4 b/m4/virt-loader-nvram.m4 index d7e0c8ca18..ed2ae0cf27 100644 --- a/m4/virt-loader-nvram.m4 +++ b/m4/virt-loader-nvram.m4 @@ -30,6 +30,8 @@ AC_DEFUN([LIBVIRT_CHECK_LOADER_NVRAM], [ l=$(echo $with_loader_nvram | tr ':' '\n' | wc -l) if test $(expr $l % 2) -ne 0 ; then AC_MSG_ERROR([Malformed --with-loader-nvram argument]) + elif test $l -gt 0 ; then + AC_MSG_WARN([Note that --with-loader-nvram is obsolete and will be removed soon]) fi AC_DEFINE_UNQUOTED([DEFAULT_LOADER_NVRAM], ["$with_loader_nvram"], [List of loader:nvram pairs]) @@ -37,5 +39,11 @@ AC_DEFUN([LIBVIRT_CHECK_LOADER_NVRAM], [ ]) AC_DEFUN([LIBVIRT_RESULT_LOADER_NVRAM], [ - LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram]) + if test "x$with_loader_nvram" != "xno" && \ + test "x$with_loader_nvram" != "x" ; then + LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram], + [!!! Using this configure option is strongly discouraged !!!]) + else + LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram]) + fi ]) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index b3a3428e4c..7a056b037e 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -761,6 +761,9 @@ # source tree. These metadata files are distributed alongside any # firmware images intended for use with QEMU. # +# NOTE: if ANY firmware metadata files are detected, this setting +# will be COMPLETELY IGNORED. +# # ------------------------------------------ # # When a domain is configured to use UEFI instead of standard diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fae697a2ef..293f2635cc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -32,6 +32,7 @@ #include "qemu_conf.h" #include "qemu_capabilities.h" #include "qemu_domain.h" +#include "qemu_firmware.h" #include "qemu_security.h" #include "viruuid.h" #include "virbuffer.h" @@ -799,7 +800,8 @@ virQEMUDriverConfigLoadLogEntry(virQEMUDriverConfigPtr cfg, static int virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg, - virConfPtr conf) + virConfPtr conf, + bool privileged) { VIR_AUTOSTRINGLIST nvram = NULL; size_t i; @@ -807,8 +809,21 @@ virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0) return -1; if (nvram) { + VIR_AUTOSTRINGLIST fwList = NULL; + virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); + if (qemuFirmwareFetchConfigs(&fwList, privileged) < 0) + return -1; + + if (fwList) { + VIR_WARN("Obsolete nvram variable is set while firmware metadata " + "files found. Note that the nvram config file variable is " + "going to be ignored."); + cfg->nfirmwares = 0; + return 0; + } + cfg->nfirmwares = virStringListLength((const char *const *)nvram); if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) < 0) return -1; @@ -1041,7 +1056,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virQEMUDriverConfigLoadLogEntry(cfg, conf) < 0) return -1; - if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf) < 0) + if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf, privileged) < 0) return -1; if (virQEMUDriverConfigLoadGlusterDebugEntry(cfg, conf) < 0) -- 2.23.0

3 7

[libvirt] [PATCH] docs: mention lifted vCPUs restriction for esx
by Pino Toscano 14 Nov '19

14 Nov '19

It was lifted with c92b6023e8eb670e01571e299a85e9da9bd4844c. Signed-off-by: Pino Toscano <ptoscano(a)redhat.com> --- docs/drvesx.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 901d65958a..ac7bc645d1 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -337,7 +337,9 @@ error: invalid argument in libvirt was built without the 'esx' driver Memory size has to be a multiple of 4096 </li> <li> - Number of virtual CPU has to be 1 or a multiple of 2 + Number of virtual CPU has to be 1 or a multiple of 2. + <span class="since">Since 4.10.0</span> any number of vCPUs is + supported. </li> <li> Valid MAC address prefixes are <code>00:0c:29</code> and -- 2.21.0

2 1

[libvirt] [PATCH 0/5] PATCH 0/8] Remove use of 'strsep' gnulib module (kill-a-gnulib-module-a-day initiative)
by Peter Krempa 14 Nov '19

14 Nov '19

Peter Krempa (5): rpc: use virStringSplit instead of strsep util: file: Use g_autofree in virFindFileInPath util: file: Simplify logic of shortuct cases in virFindFileInPath util: file: Replace use of 'strsep' with virStringSplit gnulib: Remove use of 'strsep' module bootstrap.conf | 1 - src/rpc/virnetsocket.c | 28 ++++++++++++---------------- src/util/virfile.c | 38 ++++++++++++++++---------------------- 3 files changed, 28 insertions(+), 39 deletions(-) -- 2.23.0

3 18

[libvirt] [PATCH] libxl: Fix lock manager lock ordering
by Jim Fehlig 14 Nov '19

14 Nov '19

The ordering of lock manager locks in the libxl driver has a flaw that was uncovered by a migration error path. In the perform phase of migration, the source host calls virDomainLockProcessPause to release the lock before sending the VM to the destination host. If the send fails an attempt is made to reacquire the lock with virDomainLockProcessResume, but that too can fail if the destination host has not finished cleaning up the failed VM and releasing the lock it acquired when starting to receive the VM. This change delays calling virDomainLockProcessResume in libxlDomainStart until the VM is successfully created, but before it is unpaused. A similar approach is used by the qemu driver, avoiding the need to release the lock if VM creation fails. In the migration perform phase, releasing the lock with virDomainLockProcessPause is delayed until the VM is successfully sent to the destination, which avoids reacquiring the lock if the send fails. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/libxl/libxl_domain.c | 14 +++++++------- src/libxl/libxl_migration.c | 14 +++++--------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 4073bf8d46..a830a19b99 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1364,13 +1364,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, NULL) < 0) goto cleanup; - if (virDomainLockProcessResume(driver->lockManager, - "xen:///system", - vm, - priv->lockState) < 0) - goto cleanup; - VIR_FREE(priv->lockState); - if (libxlNetworkPrepareDevices(vm->def) < 0) goto cleanup_dom; @@ -1453,6 +1446,13 @@ libxlDomainStart(libxlDriverPrivatePtr driver, libxlLoggerOpenFile(cfg->logger, domid, vm->def->name, config_json); + if (virDomainLockProcessResume(driver->lockManager, + "xen:///system", + vm, + priv->lockState) < 0) + goto destroy_dom; + VIR_FREE(priv->lockState); + /* Always enable domain death events */ if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW)) goto destroy_dom; diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index a1021d499b..8e64dc5d04 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -1253,20 +1253,16 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivatePtr driver, sockfd = virNetSocketDupFD(sock, true); virObjectUnref(sock); - if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) - VIR_WARN("Unable to release lease on %s", vm->def->name); - VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); - /* suspend vm and send saved data to dst through socket fd */ virObjectUnlock(vm); ret = libxlDoMigrateSrcSend(driver, vm, flags, sockfd); virObjectLock(vm); - if (ret < 0) { - virDomainLockProcessResume(driver->lockManager, - "xen:///system", - vm, - priv->lockState); + if (ret == 0) { + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); + } else { /* * Confirm phase will not be executed if perform fails. End the * job started in begin phase. -- 2.23.0

2 2

[libvirt] [PATCH v2 0/3] use virStringParseYesNo helper
by Mao Zhongyi 14 Nov '19

14 Nov '19

A function virStringParseYesNo was added to convert string 'yes' to true and 'no' to false, so use this helper to replace 'STREQ(.*, \"yes\")' and 'STREQ(.*, \"no\")' as it allows us to drop several repetitive if-then-else string->bool conversion blocks. v2->v1 p1: - ignore the return value of virStringParseYesNo. - update the commit message. [Michal Privoznik] p2: - add the Acked-by tag. p3: - pass return value of helper to rc directly. [Michal Privoznik] Mao Zhongyi (3): conf/domain_conf: use virStringParseYesNo helper conf/network_conf: use virStringParseYesNo helper qemu/qemu_migration_params: use virStringParseYesNo helper src/conf/domain_conf.c | 35 ++++++++++++-------------------- src/conf/network_conf.c | 4 +--- src/qemu/qemu_migration_params.c | 7 +------ 3 files changed, 15 insertions(+), 31 deletions(-) -- 2.17.1

2 4

[libvirt] [PATCH v2 0/5] Fix up some issues from x and y resolution patches
by Jonathon Jongsma 14 Nov '19

14 Nov '19

This is a follow-up series responding to some comments from Jan Tomko. Most importantly, the fact that the errors are not propagated up to the caller, they're only logged. To fix this, the function signatures were changed to return a error status. The patch series was also re-ordered slightly to improve readability (I hope). Jonathon Jongsma (5): qemu: fix documentation for video resolution conf: remove unnecessary NULL checks conf: use glib allocation when parsing video props conf: report errors when parsing video resolution conf: report errors when parsing video acceleration docs/formatdomain.html.in | 12 +++-- src/conf/domain_conf.c | 108 ++++++++++++++++++++++---------------- 2 files changed, 71 insertions(+), 49 deletions(-) -- 2.21.0

2 9

[libvirt] [PATCH 0/8] Remove use of 'areadlink' gnulib module (kill-a-gnulib-module-a-day initiative)
by Peter Krempa 14 Nov '19

14 Nov '19

This removes use of 'areadlink' by using 'g_file_read_link' and fixes a few insane uses. Note that I'll post the actual module deletion from bootstrap.conf separately as touching that file results in lengthy rebuilds. Peter Krempa (8): qemu: domain: Use g_file_read_link instead of virFileReadLink util: file: Remove virFileReadLink qemu: tpm: Use g_autofree in qemuTPMEmulatorGetPid qemu: tpm: Sanitize error values in qemuTPMEmulatorGetPid qemu: gpu: Sanitize error values in qemuVhostUserGPUGetPid util: pidfile: Sanitize return values of virPidFileReadIfAlive util: pidfile: Sanitize return values of virPidFileReadPathIfAlive util: pidfile: Replace 'areadlink' by 'g_file_read_link' src/libvirt_private.syms | 1 - src/qemu/qemu_domain.c | 18 +++++----- src/qemu/qemu_tpm.c | 16 ++++----- src/qemu/qemu_vhost_user_gpu.c | 12 ++++--- src/util/virfile.c | 13 ------- src/util/virfile.h | 3 -- src/util/virpidfile.c | 64 +++++++++++++++++----------------- 7 files changed, 56 insertions(+), 71 deletions(-) -- 2.23.0

2 10

[libvirt] [PATCH 0/2] qemu: block: Fix two bugs related to usage of blockdev (blockdev-add saga)
by Peter Krempa 14 Nov '19

14 Nov '19

Peter Krempa (2): qemu: blockjob: Transfer 'readonly' state of images after active layer block commit qemu: snapshot: Fix inactive external snapshots when backing chain is present src/qemu/qemu_blockjob.c | 2 ++ src/qemu/qemu_driver.c | 25 +++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) -- 2.23.0

2 3

[libvirt] [PATCH Rust 0/4] Map more functions in stream module
by Zixing Liu 14 Nov '19

14 Nov '19

This set of patches will add more functions to the Rust bindings. Newly mapped functions from C library: virStreamNew virStreamEventUpdateCallback virStreamEventRemoveCallback virStreamEventAddCallback. virStreamEventAddCallback can accept normal fn functions or closures (can capture variables outside) The changes are not very thoroughly tested since event module is not implemented at all so the virStreamEventAddCallback will always return "unsupported by the connection driver". Also the changes have run though the rustfmt to ensure the format conforms to Rust officially recommended code style. Zixing Liu (4): libvirt-rust: stream: add more functions in stream libvirt-rust: stream: add more functions in stream libvirt-rust: stream: automated lint libvirt-rust: use reference instead of moving src/domain.rs | 2 +- src/stream.rs | 128 ++++++++++++++++++++++++++++++++++++++++++------ tests/stream.rs | 40 +++++++++++++++ 3 files changed, 153 insertions(+), 17 deletions(-) create mode 100644 tests/stream.rs -- 2.24.0

2 7

[libvirt] [RFC] default video device type
by Pavel Mores 14 Nov '19

14 Nov '19

Hi, I'm looking into fixing https://bugzilla.redhat.com/show_bug.cgi?id=1668141 (as a short summary, if a graphics device is added to XML that has no video device, libvirt automatically adds a video device which is always of type 'cirrus' - even if the underlying qemu doesn't support cirrus). I'm able to affect the behaviour in question by using qemu capabilities in qemuDomainDeviceVideoDefPostParse(), see proof-of-concept change in [1]. I have a couple of questions though: 1) is this a proper place and approach to fix the bug? 2) what would be the full specification of expected behaviour? The bug report only states that the video type shouldn't be cirrus but doesn't say what it should be. [2] gives some information about the order of preference of video device types but I was wondering if there are any opinions about this on this list? Cheers, pvl [1] diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b4175a846e..0de491b79f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7821,7 +7821,8 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net, static int qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr video, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { if (ARCH_IS_PPC64(def->os.arch)) @@ -7830,8 +7831,16 @@ qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr video, qemuDomainIsRISCVVirt(def) || ARCH_IS_S390(def->os.arch)) video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO; - else - video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; + else { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE) + && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) { + video->type = VIR_DOMAIN_VIDEO_TYPE_QXL; + video->vgamem = QEMU_QXL_VGAMEM_DEFAULT; + } else { + video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; + } + } } if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && @@ -7926,7 +7935,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, break; case VIR_DOMAIN_DEVICE_VIDEO: - ret = qemuDomainDeviceVideoDefPostParse(dev->data.video, def); + ret = qemuDomainDeviceVideoDefPostParse(dev->data.video, def, qemuCaps); break; case VIR_DOMAIN_DEVICE_PANIC: [2] https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/

3 3