January 2020 - Devel - libvirt List Archives

[PATCH 0/2] Simple fixes for run.in
by Richard W.M. Jones 21 Jan '20

21 Jan '20

The ./run script is a very useful way to run the libguestfs and virt-v2v test suites against a locally compiled copy of libvirt. These two patches contain some minor fixes. Rich.

3 6

[PATCH] Fix syntax-check 'always true header test' for virvsock.c
by Daniel Henrique Barboza 20 Jan '20

20 Jan '20

Commit 05a38d4c4a ("src: conditionalize / remove use of sys/ioctl.h") aimed to remove imports of ioctl.h and conditionalize them for compilation on Windows. However, the change made in virvsock.c: -#include <sys/ioctl.h> +#ifdef HAVE_SYS_IOCTL_H +# include <sys/ioctl.h> +#endif Makes the 'sc_prohibit_always_true_header_tests' rule complain about the redundancy of this test when building it on Linux. ../src/util/virvsock.c:20:#ifdef HAVE_SYS_IOCTL_H build-aux/syntax-check.mk: do not test the above HAVE_<header>_H symbol(s); with the corresponding gnulib module, they are always true make: *** [../build-aux/syntax-check.mk:1679: sc_prohibit_always_true_header_tests] Error 1 make: *** Waiting for unfinished jobs.... This patch fixes it by changing the conditional to #ifndef WIN32 instead, like the other changes made by that patch. CC: Daniel P. Berrangé <berrange(a)redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com> --- src/util/virvsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virvsock.c b/src/util/virvsock.c index 2638c5095a..1e2eb553d2 100644 --- a/src/util/virvsock.c +++ b/src/util/virvsock.c @@ -17,7 +17,7 @@ #include <config.h> -#ifdef HAVE_SYS_IOCTL_H +#ifndef WIN32 # include <sys/ioctl.h> #endif -- 2.24.1

1 1

[libvirt RFC 0/2] qemu: handle 'raw' format options 'size' and 'offset'
by Peter Krempa 20 Jan '20

20 Jan '20

See patch 2 please. Peter Krempa (2): tests: storage: Add test data for json specified raw image with offset/size qemu: Handle 'offset' and 'size' parameters of 'raw' driver src/conf/domain_conf.c | 19 +++++++++++++++++++ src/qemu/qemu_block.c | 10 ++++------ src/util/virstoragefile.c | 21 +++++++++++++++++---- src/util/virstoragefile.h | 4 ++++ tests/virstoragetest.c | 15 +++++++++++++++ 5 files changed, 59 insertions(+), 10 deletions(-) -- 2.24.1

3 5

[PATCH] qemu: snapshot: Prevent too-nested domain XML when doing inactive snapshot
by Peter Krempa 20 Jan '20

20 Jan '20

Similarly to 510d154a0b41aa70aadabc0918d16dee22882394 we need to prevent doing too deeply nested backing chains and reject them with a sane error message. Add a loop to go through the snapshots prior to attempting actually creating them to prevent some possible inconsistent scenarios. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2f66d7cd9a..4cebb54913 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14618,6 +14618,16 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, if (!(created = virBitmapNew(snapdef->ndisks))) goto cleanup; + for (i = 0; i < snapdef->ndisks && !reuse; i++) { + snapdisk = &(snapdef->disks[i]); + defdisk = snapdef->parent.dom->disks[snapdisk->idx]; + if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + continue; + + if (qemuDomainStorageSourceValidateDepth(defdisk->src, 1, defdisk->dst) < 0) + return -1; + } + /* If reuse is true, then qemuDomainSnapshotPrepare already * ensured that the new files exist, and it was up to the user to * create them correctly. */ -- 2.24.1

2 1

[PATCH] qemu: monitor: Improve error message when QEMU reply is too large
by Peter Krempa 20 Jan '20

20 Jan '20

Don't use ERANGE as it doesn't make much sense in the error message. Also point out that the reply from qemu was too large which is not obvious from the original error: error: No complete monitor response found in 10485760 bytes: Numerical result out of range The new message will read: error: internal error: QEMU monitor reply exceeds buffer size (10485760 bytes) Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_monitor.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ccd20b3740..e3bd000ccb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -484,9 +484,9 @@ qemuMonitorIORead(qemuMonitorPtr mon) if (avail < 1024) { if (mon->bufferLength >= QEMU_MONITOR_MAX_RESPONSE) { - virReportSystemError(ERANGE, - _("No complete monitor response found in %d bytes"), - QEMU_MONITOR_MAX_RESPONSE); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QEMU monitor reply exceeds buffer size (%d bytes)"), + QEMU_MONITOR_MAX_RESPONSE); return -1; } if (VIR_REALLOC_N(mon->buffer, -- 2.24.1

2 1

[libvirt PATCH] qemu: end the agent job in qemuDomainSetTimeAgent
by Ján Tomko 20 Jan '20

20 Jan '20

This function grabs an agent job but ends a monitor job. End the agent job instead. https://bugzilla.redhat.com/show_bug.cgi?id=1792723 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> Reported-by: Dan Zheng <dzheng(a)redhat.com> Fixes: e005c95f56fee9ed780be7f8db103d690bd34cbd --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7e379fe83a..2f66d7cd9a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20464,7 +20464,7 @@ qemuDomainSetTimeAgent(virQEMUDriverPtr driver, qemuDomainObjExitAgent(vm, agent); endjob: - qemuDomainObjEndJob(driver, vm); + qemuDomainObjEndAgentJob(vm); return ret; } -- 2.21.0

2 1

[libvirt] [PATCH] Don't check if the path exists for LVM
by ebenner 17 Jan '20

17 Jan '20

I'm not entirely sure why Libvirt is checking to see if the target file exists for LVM but it is a problem. Empty volume groups do not appear in /dev. When libvirt starts these pools then fail to autostart because their path does not exist. This is not useful, this is the patch we are running to resolve this issue. --- src/storage/storage_backend_logical.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 42dec05ba0..2c60719b8c 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -641,11 +641,9 @@ virStorageBackendLogicalCheckPool(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - /* If we can find the target.path as well as ensure that the - * pool's def source - */ - *isActive = virFileExists(def->target.path) && - virStorageBackendLogicalMatchPoolSource(pool); + /* Ensure that the pool's def source is correct */ + *isActive = virStorageBackendLogicalMatchPoolSource(pool); + return 0; } -- 2.24.1

3 2

[PATCH] Re: storage: Cleaning up pools is not always ideal
by ebenner＠vultr.com 17 Jan '20

17 Jan '20

From: Eric Benner <ebenner(a)vultr.com> Didn't have a sign off and was formatted by a mail client. This is a functional copy of this patch with the requirements. Signed-off-by: Eric Benner <ebenner(a)vultr.com> --- src/remote/libvirtd.conf.in | 7 +++++++ src/storage/storage_driver.c | 38 ++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index 34741183cc..5600c26eca 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -506,3 +506,10 @@ # potential infinite waits blocking libvirt. # #ovs_timeout = 5 + +################################################################### +# This decides whether to disable pools that errors in some way such as during a refresh +# This can negatively affect LVM pools. 1 = disable the pools, 2 = don't disable the pools +# Do not use this if you are not using only LVM pools +# +cleanup_pools = 1 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 0bb116cf08..5f8bc8a835 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -55,6 +55,7 @@ VIR_LOG_INIT("storage.storage_driver"); static virStorageDriverStatePtr driver; +static int cleanup_pools = -1; static int storageStateCleanup(void); @@ -74,6 +75,20 @@ static void storageDriverUnlock(void) virMutexUnlock(&driver->lock); } +static int cleanupPools(void) +{ + if (cleanup_pools == -1) { + g_autoptr(virConf) libvirtConf = NULL; + virConfLoadConfig(&libvirtConf, "libvirtd.conf"); + + if (!virConfGetValueInt(libvirtConf, "cleanup_pools", &cleanup_pools) + || (cleanup_pools != 0 && cleanup_pools != 1)) + cleanup_pools = 1; + } + + return cleanup_pools; +} + static void storagePoolRefreshFailCleanup(virStorageBackendPtr backend, @@ -81,14 +96,26 @@ storagePoolRefreshFailCleanup(virStorageBackendPtr backend, const char *stateFile) { virErrorPtr orig_err; - virErrorPreserveLast(&orig_err); virStoragePoolObjClearVols(obj); if (stateFile) unlink(stateFile); - if (backend->stopPool) - backend->stopPool(obj); + + if (!cleanupPools()) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to refresh storage pool '%s'. Would have disabled this pool but clean_pools = 0: %s"), + def->name, virGetLastErrorMessage()); + } else { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to refresh storage pool '%s'. Disabled this pool because clean_pools = 1: %s"), + def->name, virGetLastErrorMessage()); + if (backend->stopPool) { + backend->stopPool(obj); + } + } virErrorRestore(&orig_err); } @@ -101,7 +128,10 @@ storagePoolRefreshImpl(virStorageBackendPtr backend, virStoragePoolObjClearVols(obj); if (backend->refreshPool(obj) < 0) { storagePoolRefreshFailCleanup(backend, obj, stateFile); - return -1; + if (!cleanupPools()) + return 0; + else + return -1; } return 0; -- 2.24.1

1 0

[RFC PATCH] libxl: support configuring paravirtualized keyboard
by Marek Marczykowski-Górecki 17 Jan '20

17 Jan '20

Xen since 4.13 allows to configure whether PV vkb device is created or not - instead of always creating it. Plug it into <input/> device configuration and give the user a choice - setting PS2 keyboard disables PV one, and setting XEN keyboard enables PV. There is one issue with this approach: there are implicit keyboard and mouse device added with bus 'ps2'. This is at least done by the xen-xl -> libvirt xml config converter (why?). When one use a config generated this way, this commit will make behavior change - PV keyboard will get disabled. I'm not sure if that's a big issue - both keyboards works. Can it be this way? Or maybe some better idea how to represent vkb_device option in libvirt xml? Once the above is sorted out (or chosen to ignore), I'll send updated patch with tests and config converter support. Because the implicit keyboard is added, I also avoided to reject keybord device on too old Xen versions. This means <input type='keyboard' bus='xen'/> will be _silently_ ignored on older Xen. Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com> --- src/libxl/libxl_conf.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 27de67d266..0999fb5678 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -637,6 +637,24 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, for (i = 0; i < def->ninputs; i++) { char **usbdevice; +#ifdef LIBXL_HAVE_BUILDINFO_VKB_DEVICE + if (def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_KBD) { + switch (def->inputs[i]->bus) { + case VIR_DOMAIN_INPUT_BUS_PS2: + libxl_defbool_set(&b_info->u.hvm.vkb_device, false); + break; + case VIR_DOMAIN_INPUT_BUS_XEN: + libxl_defbool_set(&b_info->u.hvm.vkb_device, true); + break; + case VIR_DOMAIN_INPUT_BUS_VIRTIO: + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("libxenlight supports only ps2/xen keyboard device")); + return -1; + } + } +#endif + if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) continue; -- 2.21.0

1 0

[libvirt] [rust PATCH v2 0/5] Map more functions in stream module
by Zixing Liu 17 Jan '20

17 Jan '20

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. Version 2: Addressed comments Zixing Liu (5): 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 libvirt-rust: stream: addressed comments src/domain.rs | 2 +- src/stream.rs | 129 +++++++++++++++++++++++++++++++++++++++++------- tests/stream.rs | 40 +++++++++++++++ 3 files changed, 153 insertions(+), 18 deletions(-) create mode 100644 tests/stream.rs -- 2.24.1

2 15