- Devel - libvirt List Archives

[libvirt] [PATCH] lxc: Fix 'domblkstat' error with attached disk devices.
by jcfaracco＠gmail.com 19 Nov '19

19 Nov '19

From: Julio Faracco <jcfaracco(a)gmail.com> LXC was not working when you attached a new disk that points to block device. See: https://bugzilla.redhat.com/show_bug.cgi?id=1697115. Command line from virsh was showing problems with alias first (this feature is not supported) and after, problems to query block device. It was happening because extra disks were not being included into cgroup blkio.throttle properties and libvirt could not query this info. Applying those devices into 'allowed' list is not enough, libvirt should reset blkio.throttle as a workaround to include disks into container's cgroup directory. Before: virsh # domblkstat CentOS hda error: Failed to get block stats for domain 'CentOS' device 'hda' error: internal error: domain stats query failed Now: virsh # domblkstat CentOS hda hda rd_req 0 hda rd_bytes 0 hda wr_req 0 hda wr_bytes 0 Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com> --- src/lxc/lxc_cgroup.c | 29 ++++++++++++++++++++++++++++- src/lxc/lxc_cgroup.h | 4 ++++ src/lxc/lxc_driver.c | 8 ++++---- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5efb495b56..de6d892521 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -302,6 +302,24 @@ virLXCTeardownHostUSBDeviceCgroup(virUSBDevicePtr dev G_GNUC_UNUSED, return 0; } +int +virLXCCgroupResetBlkioDeviceThrottle(virCgroupPtr cgroup, + const char *path) +{ + if (virCgroupSetBlkioDeviceReadBps(cgroup, path, 0) < 0) + return -1; + + if (virCgroupSetBlkioDeviceWriteBps(cgroup, path, 0) < 0) + return -1; + + if (virCgroupSetBlkioDeviceReadIops(cgroup, path, 0) < 0) + return -1; + + if (virCgroupSetBlkioDeviceWriteIops(cgroup, path, 0) < 0) + return -1; + + return 0; +} static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, virCgroupPtr cgroup) @@ -309,6 +327,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, int capMknod = def->caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD]; int ret = -1; size_t i; + const char *src_path = NULL; static virLXCCgroupDevicePolicy devices[] = { {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL}, {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO}, @@ -346,8 +365,16 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, !virStorageSourceIsBlockLocal(def->disks[i]->src)) continue; + /* Workaround to include disks into blkio.throttle. + * To include it, we need to reset any feature of + * blkio.throttle.* */ + src_path = virDomainDiskGetSource(def->disks[i]); + if (virLXCCgroupResetBlkioDeviceThrottle(cgroup, + src_path) < 0) + goto cleanup; + if (virCgroupAllowDevicePath(cgroup, - virDomainDiskGetSource(def->disks[i]), + src_path, (def->disks[i]->src->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW) | diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h index 63e9e837b0..3d643a4fea 100644 --- a/src/lxc/lxc_cgroup.h +++ b/src/lxc/lxc_cgroup.h @@ -46,3 +46,7 @@ int virLXCTeardownHostUSBDeviceCgroup(virUSBDevicePtr dev, const char *path, void *opaque); + +int +virLXCCgroupResetBlkioDeviceThrottle(virCgroupPtr cgroup, + const char *path); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 204a3ed522..87da55f308 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2339,14 +2339,14 @@ lxcDomainBlockStats(virDomainPtr dom, goto endjob; } - if (!disk->info.alias) { + if (!disk->src->path) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing disk device alias name for %s"), disk->dst); goto endjob; } ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup, - disk->info.alias, + disk->src->path, &stats->rd_bytes, &stats->wr_bytes, &stats->rd_req, @@ -2424,14 +2424,14 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, goto endjob; } - if (!disk->info.alias) { + if (!disk->src->path) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing disk device alias name for %s"), disk->dst); goto endjob; } if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup, - disk->info.alias, + disk->src->path, &rd_bytes, &wr_bytes, &rd_req, -- 2.20.1

3 2

[libvirt] [PATCH v2 0/3] remove 'cleanup: return' labels
by Daniel Henrique Barboza 19 Nov '19

19 Nov '19

changes from v1 [1], all suggested by Ján Tomko: - qemu patch (former patch 10): * qemu_driver.c: using 'return -1' instead of 'return ret' in qemuDumpToFd() and qemuDomainBlockPivot() * qemu_hotplug.c: removed the use of 'ret' variable in qemuDomainChangeGraphics(), returning -1 in all error conditions and returning 0 in the end - vbox patch (former patch 13): * vbox_common.c: changed 'return -2' to 'return -1' in vboxDomainGetInfo() - tests patch (former patch 20): * domainconftest.c: rolled back the change since the 'goto cleanup' is more readable than copying EXIT_SUCCESS multiple times. [1] https://www.redhat.com/archives/libvir-list/2019-October/msg01420.html Daniel Henrique Barboza (3): qemu: remove unneeded cleanup labels vbox: remove unneeded cleanup labels tests: remove unneeded cleanup labels src/qemu/qemu_cgroup.c | 15 ++-- src/qemu/qemu_domain.c | 84 ++++++------------ src/qemu/qemu_domain_address.c | 31 +++---- src/qemu/qemu_driver.c | 145 +++++++++++-------------------- src/qemu/qemu_hotplug.c | 75 +++++++--------- src/qemu/qemu_interface.c | 26 +++--- src/qemu/qemu_migration.c | 38 +++----- src/qemu/qemu_process.c | 69 +++++---------- src/qemu/qemu_tpm.c | 18 ++-- src/vbox/vbox_common.c | 12 ++- src/vbox/vbox_snapshot_conf.c | 75 +++++++--------- tests/commandtest.c | 17 ++-- tests/networkxml2firewalltest.c | 10 +-- tests/nsstest.c | 27 +++--- tests/nwfilterebiptablestest.c | 4 +- tests/nwfilterxml2firewalltest.c | 16 ++-- tests/qemuhotplugtest.c | 19 ++-- tests/qemuxml2argvtest.c | 11 +-- tests/virauthconfigtest.c | 11 +-- tests/vircgroupmock.c | 11 +-- tests/virendiantest.c | 58 ++++++------- tests/virkeycodetest.c | 14 +-- tests/virmacmaptest.c | 5 +- tests/virnetdevtest.c | 11 +-- tests/virpcimock.c | 31 +++---- tests/virpcitest.c | 3 +- tests/virpolkittest.c | 65 +++++--------- tests/virstringtest.c | 18 ++-- 28 files changed, 333 insertions(+), 586 deletions(-) -- 2.21.0

2 5

[libvirt] [PATCH 0/3] tests: schema: Try to improve debugability
by Peter Krempa 19 Nov '19

19 Nov '19

Peter Krempa (3): tests: schema: Simplify memory handling using g_autofree tests: utils: Introduce helper for dispatching libvirt errors tests: virschema: Propagate errors from directory traversal in testSchemaDir tests/testutils.c | 29 +++++++++++++++++++++++++---- tests/testutils.h | 1 + tests/virschematest.c | 34 +++++++++++++--------------------- 3 files changed, 39 insertions(+), 25 deletions(-) -- 2.23.0

2 4

[libvirt] [PATCH 1/3] qemuProcessStop: Set @def early
by Michal Privoznik 19 Nov '19

19 Nov '19

The @def variable holds pointer to the domain defintion, but is set only somewhere in the middle of the function. This is suboptimal. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 90ebdb43a0..8cb12ac9a6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7365,6 +7365,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, goto endjob; } + def = vm->def; + qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false); if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) @@ -7386,7 +7388,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainConfVMNWFilterTeardown(vm); if (cfg->macFilter) { - def = vm->def; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; if (net->ifname == NULL) @@ -7483,7 +7484,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuHostdevReAttachDomainDevices(driver, vm->def); - def = vm->def; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; vport = virDomainNetGetActualVirtPortProfile(net); -- 2.23.0

2 2

[libvirt] [PATCH] Add news for virDomainAgentSetResponseTimeout()
by Jonathon Jongsma 18 Nov '19

18 Nov '19

Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- docs/news.xml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 240d288c37..72c0edb317 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -66,6 +66,19 @@ initialized the vGPU. </description> </change> + <change> + <summary> + Add API to change the response timeout for guest agent commands + </summary> + <description> + By default, when a command is sent to the guest agent, libvirt waits + forever for a response from the guest agent. If the guest is + unresponsive for any reason, this can block the calling thread + indefinitely. By setting a custom timeout using + <code>virDomainAgentSetResponseTimeout()</code>, API users can change + this behavior. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.21.0

2 1

[libvirt] [libvirt-dbus][PATCH v4 0/2] Implement snapshots
by Simon Kobyda 18 Nov '19

18 Nov '19

Changes since v2: Implement and use virtDBusDomainSnapshotGetVirDomainSnapshot Add domain snapshot xml file to data/meson.build Fix flake8 style errors Simon Kobyda (2): Introduce Domain Snapshot Interface Implement snapshots APIs data/meson.build | 1 + data/org.libvirt.Domain.xml | 26 +++ data/org.libvirt.DomainSnapshot.xml | 41 ++++ src/connect.c | 6 + src/connect.h | 1 + src/domain.c | 154 +++++++++++++++ src/domainsnapshot.c | 291 ++++++++++++++++++++++++++++ src/domainsnapshot.h | 9 + src/meson.build | 1 + src/util.c | 53 +++++ src/util.h | 16 ++ tests/libvirttest.py | 12 ++ tests/meson.build | 1 + tests/test_domain.py | 8 + tests/test_snapshot.py | 45 +++++ tests/xmldata.py | 6 + 16 files changed, 671 insertions(+) create mode 100644 data/org.libvirt.DomainSnapshot.xml create mode 100644 src/domainsnapshot.c create mode 100644 src/domainsnapshot.h create mode 100755 tests/test_snapshot.py -- 2.21.0

2 3

[libvirt] [PATCH] g_mkostemp_full: pass O_RDWR
by Ján Tomko 18 Nov '19

18 Nov '19

This flag is not implied by g_mkostemp_full, only by g_mkostemp. Signed-off-by: Ján Tomko <jtomko(a)redhat.com> Reported-by: Bjoern Walk <bwalk(a)linux.ibm.com> Fixes: 4ac47730408eaf91683f6502ec10541f4f711a5c --- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_util.c | 2 +- src/util/virlog.c | 2 +- src/vbox/vbox_common.c | 2 +- tests/virfiletest.c | 2 +- tools/vsh.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 60f5732708..7fd87a9d76 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4024,7 +4024,7 @@ qemuDomainScreenshot(virDomainPtr dom, if (!(tmp = g_strdup_printf("%s/qemu.screendump.XXXXXX", cfg->cacheDir))) goto endjob; - if ((tmp_fd = g_mkstemp_full(tmp, O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { + if ((tmp_fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { virReportSystemError(errno, _("g_mkstemp(\"%s\") failed"), tmp); goto endjob; } @@ -11970,7 +11970,7 @@ qemuDomainMemoryPeek(virDomainPtr dom, goto endjob; /* Create a temporary filename. */ - if ((fd = g_mkstemp_full(tmp, O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { + if ((fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { virReportSystemError(errno, _("g_mkstemp(\"%s\") failed"), tmp); goto endjob; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 6d41df8d7e..f2d8810813 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1215,7 +1215,7 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, if (!(secretPath = virStoragePoolObjBuildTempFilePath(pool, vol))) goto cleanup; - if ((fd = g_mkstemp_full(secretPath, O_CLOEXEC, S_IRUSR | S_IWUSR)) < 0) { + if ((fd = g_mkstemp_full(secretPath, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) < 0) { virReportSystemError(errno, "%s", _("failed to open secret file for write")); goto error; diff --git a/src/util/virlog.c b/src/util/virlog.c index 05052e9d09..d45e2dd316 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -992,7 +992,7 @@ virLogOutputToJournald(virLogSourcePtr source, * and pass an FD to the journal */ - if ((buffd = g_mkstemp_full(path, O_CLOEXEC, S_IRUSR | S_IWUSR)) < 0) + if ((buffd = g_mkstemp_full(path, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) < 0) return; if (unlink(path) < 0) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 88ef41b5df..9e41b6767a 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -7385,7 +7385,7 @@ vboxDomainScreenshot(virDomainPtr dom, tmp = g_strdup_printf("%s/vbox.screendump.XXXXXX", cacheDir); - if ((tmp_fd = g_mkstemp_full(tmp, O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { + if ((tmp_fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) { virReportSystemError(errno, _("g_mkstemp(\"%s\") failed"), tmp); VIR_FREE(tmp); VBOX_RELEASE(machine); diff --git a/tests/virfiletest.c b/tests/virfiletest.c index 193c5bedd4..781c640e2b 100644 --- a/tests/virfiletest.c +++ b/tests/virfiletest.c @@ -133,7 +133,7 @@ makeSparseFile(const off_t offsets[], off_t len = 0; size_t i; - if ((fd = g_mkstemp_full(path, O_CLOEXEC, S_IRUSR | S_IWUSR)) < 0) + if ((fd = g_mkstemp_full(path, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) < 0) goto error; if (unlink(path) < 0) diff --git a/tools/vsh.c b/tools/vsh.c index b5de06f26b..1076c8254b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2400,7 +2400,7 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc) tmpdir = getenv("TMPDIR"); if (!tmpdir) tmpdir = "/tmp"; ret = g_strdup_printf("%s/virshXXXXXX.xml", tmpdir); - fd = g_mkstemp_full(ret, O_CLOEXEC, S_IRUSR | S_IWUSR); + fd = g_mkstemp_full(ret, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR); if (fd == -1) { vshError(ctl, _("g_mkstemp_full: failed to create temporary file: %s"), virStrerror(errno, ebuf, sizeof(ebuf))); -- 2.21.0

2 1

[libvirt] Bug: virsh edit broken on master
by Bjoern Walk 18 Nov '19

18 Nov '19

Hey all, commit 4ac47730: Use g_mkstemp_full instead of mkostemp(s) broke `virsh edit` (at least on S390): virsh # edit test error: write: /tmp/virshQ7SXB0.xml: failed to write to temporary file: Bad file descriptorx I didn't find out why, everything looks normal: # ls -l /proc/$(pgrep virsh)/fd/ total 0 [...] lr-x------. 1 root root 64 Nov 18 16:58 10 -> /tmp/virshAEKYB0.xml # ls -l /tmp/virshAEKYB0.xml -rw-------. 1 root root 0 Nov 18 16:59 /tmp/virshAEKYB0.xml Best, Bjoern -- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

3 2

[libvirt] [PATCH 0/3] A few more Coverity patches
by John Ferlan 18 Nov '19

18 Nov '19

Finally got Coverity to build with the recent build env changes. With doing that Coverity re-examines everything and bridge_driver generated a couple of longer term things. The virFileRewrite is from more recent changes. John Ferlan (3): util: Remove unnecessary check in virFileRewrite network: Use local variables in networkUpdatePortBandwidth network: Check for QOS before blindly using it src/network/bridge_driver.c | 31 ++++++++++++++++++++++--------- src/util/virfile.c | 3 +-- 2 files changed, 23 insertions(+), 11 deletions(-) -- 2.20.1

4 10

[libvirt] [PATCH 0/3] Fix remote dispatch code trying to allocate 0-sized return buffers
by Erik Skultety 18 Nov '19

18 Nov '19

This happens because of the switch to glib whose method g_malloc0 actually correctly returns NULL on size 0 which doesn't make sense to do. The outcome of that is that because virAllocN always returns 0, the dispatch code never fails allocation and passes the NULL pointer to the server-side public API which actually checks that and fails. https://bugzilla.redhat.com/show_bug.cgi?id=1772842 Erik Skultety (3): rpc: gendispatch: Fix a couple of places adding trailing spaces rpc: gendispatch: Add a check for zero size client-side buffers libvirt-domain: virConnectListDomains: Return 0 on zero-size buffer src/libvirt-domain.c | 3 +++ src/rpc/gendispatch.pl | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) -- 2.23.0

2 8