- Devel - libvirt List Archives

[libvirt] [PATCH 1/1] virStrncpy: fix to successfully copy empty string
by Nikolay Shirokovskiy 16 Sep '19

16 Sep '19

After [1] we got failure on attempt to copy empty string. Before the patch empty string was copied successfuly. Restore the original behaviour. [1] 7d70a63b util: Improve virStrncpy() implementation Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/util/virstring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index 2064944b0b..a4cc7e9c0a 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -786,7 +786,7 @@ virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) if (n == -1) n = src_len; - if (n <= 0 || n > src_len || n > (destbytes - 1)) + if (n > src_len || n > (destbytes - 1)) return -1; memcpy(dest, src, n); -- 2.23.0

2 1

[libvirt] [PATCH 0/2] qemu: Enable -blockdev support (blockdev-add saga)
by Peter Krempa 16 Sep '19

16 Sep '19

To my knowledge, everything in libvirt is now prepared to fully use -blockdev way to configure disks in qemu. There is one known qemu bug though: Internal snapshots don't work with -blockdev: https://bugzilla.redhat.com/show_bug.cgi?id=1658981 Since I can't in good faith ask for merging this patchset yet I'd like to give it some more testing I'm suggesting that we push it and revert it during freeze or add a capability check once qemu is fixed. Any other ideas? Peter Krempa (2): qemu: caps: Add capability for dynamic 'auto-read-only' support for files qemu: enable blockdev support src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + .../caps_4.1.0.x86_64.xml | 2 + .../controller-virtio-scsi.x86_64-latest.args | 35 +++-- .../disk-aio.x86_64-latest.args | 19 ++- ...-backing-chains-noindex.x86_64-latest.args | 145 +++++++++++++++--- .../disk-cache.x86_64-latest.args | 50 ++++-- .../disk-cdrom-network.x86_64-latest.args | 32 ++-- .../disk-cdrom-tray.x86_64-latest.args | 24 ++- .../disk-cdrom.x86_64-latest.args | 21 +-- .../disk-copy_on_read.x86_64-latest.args | 19 ++- .../disk-detect-zeroes.x86_64-latest.args | 17 +- .../disk-error-policy.x86_64-latest.args | 30 ++-- .../disk-floppy-q35-2_11.x86_64-latest.args | 14 +- .../disk-floppy-q35-2_9.x86_64-latest.args | 14 +- .../disk-floppy.x86_64-latest.args | 21 ++- .../disk-network-gluster.x86_64-latest.args | 32 ++-- .../disk-network-iscsi.x86_64-latest.args | 58 ++++--- .../disk-network-nbd.x86_64-latest.args | 41 +++-- .../disk-network-rbd.x86_64-latest.args | 67 +++++--- .../disk-network-sheepdog.x86_64-latest.args | 16 +- ...isk-network-source-auth.x86_64-latest.args | 30 ++-- .../disk-network-tlsx509.x86_64-latest.args | 64 +++++--- .../disk-readonly-disk.x86_64-latest.args | 14 +- .../disk-scsi-device-auto.x86_64-latest.args | 14 +- .../disk-scsi.x86_64-latest.args | 35 +++-- .../disk-shared.x86_64-latest.args | 36 +++-- ...irtio-scsi-reservations.x86_64-latest.args | 20 ++- .../floppy-drive-fat.x86_64-latest.args | 7 +- ...egl-headless-rendernode.x86_64-latest.args | 7 +- .../graphics-egl-headless.x86_64-latest.args | 7 +- ...threads-virtio-scsi-pci.x86_64-latest.args | 25 ++- ...y-hotplug-nvdimm-access.x86_64-latest.args | 7 +- ...ry-hotplug-nvdimm-align.x86_64-latest.args | 7 +- ...ry-hotplug-nvdimm-label.x86_64-latest.args | 7 +- ...ory-hotplug-nvdimm-pmem.x86_64-latest.args | 7 +- ...hotplug-nvdimm-readonly.x86_64-latest.args | 7 +- .../memory-hotplug-nvdimm.x86_64-latest.args | 7 +- ...eo-bochs-display-device.x86_64-latest.args | 10 +- ...virtio-non-transitional.x86_64-latest.args | 7 +- .../virtio-transitional.x86_64-latest.args | 7 +- .../x86_64-pc-graphics.x86_64-latest.args | 8 +- .../x86_64-pc-headless.x86_64-latest.args | 8 +- .../x86_64-q35-graphics.x86_64-latest.args | 8 +- .../x86_64-q35-headless.x86_64-latest.args | 8 +- 45 files changed, 714 insertions(+), 306 deletions(-) -- 2.21.0

4 9

[libvirt] [PATCH v2 00/11] hostdev: handle usb detach/attach on node
by Nikolay Shirokovskiy 16 Sep '19

16 Sep '19

*Notes* Deleting usb device from qemu is synchronous operation (although it is not stated in qemu API). I did not used this knowledge in the series. The last patch is remnant of previus version of the series yet it is useful. Diff to previous[1] version: - don't use dummy device while host usb device is unplugged [1] https://www.redhat.com/archives/libvir-list/2019-August/msg01413.html Nikolay Shirokovskiy (11): qemu: track hostdev delete intention qemu: support host usb device unplug qemu: support usb hostdev plugging back qemu: handle host usb device add/del udev events qemu: handle libvirtd restart after host usb device unplug qemu: handle race on device deletion and usb host device plugging qemu: hotplug: update device list on device deleted event qemu: handle host usb device plug/unplug when libvirtd is down qemu: don't mess with non mandatory hostdevs on reattaching qemu: handle detaching of unplugged hostdev conf: parse hostdev missing flag src/conf/domain_conf.c | 32 +++ src/conf/domain_conf.h | 15 ++ src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 431 ++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 104 ++++++++-- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_process.c | 59 ++++++ src/util/virhostdev.c | 2 + tests/qemuhotplugtest.c | 2 +- 12 files changed, 637 insertions(+), 20 deletions(-) -- 2.23.0

3 31

[libvirt] [PATCH] qemu_blockjob: Remove image metadata on blockcommit
by Michal Privoznik 15 Sep '19

15 Sep '19

When doing a blockcommit, the base and possibly top parent are relabelled in qemuDomainBlockCommit(). However, once the block job is finished, we need to remove the secdriver metadata (XATTRs) created at the beginning. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_blockjob.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index a991309ee7..47715f12f6 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -677,6 +677,25 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver, } virObjectUnref(disk->mirror); + } else if (job->type == QEMU_BLOCKJOB_TYPE_COMMIT) { + if (qemuSecurityMoveImageMetadata(driver, vm, + job->data.commit.base, NULL) < 0) { + VIR_WARN("Unable to remove disk metadata on " + "vm %s from %s (disk target %s)", + vm->def->name, + NULLSTR(job->data.commit.base->path), + disk->dst); + } + if (job->data.commit.topparent && + job->data.commit.topparent != disk->src && + qemuSecurityMoveImageMetadata(driver, vm, + job->data.commit.topparent, NULL) < 0) { + VIR_WARN("Unable to remove disk metadata on " + "vm %s from %s (disk target %s)", + vm->def->name, + NULLSTR(job->data.commit.topparent->path), + disk->dst); + } } } -- 2.21.0

2 3

[libvirt] [PATCH 0/2] qemu: Fix blockjob assignment collision (blockdev-add saga)
by Peter Krempa 15 Sep '19

15 Sep '19

We leaked a block job object in the tests because two jobs with distinct names referenced the same disk. Fix the test data and add code to prevent this issue. Peter Krempa (2): tests: qemustatusxml2xml: Fix disk target mess qemu: blockjob: Refuse to register blockjob if disk already has one src/qemu/qemu_blockjob.c | 6 ++++++ .../qemustatusxml2xmldata/blockjob-blockdev-in.xml | 14 +++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) -- 2.21.0

3 6

[libvirt] [PATCH 0/3] qemu: make runtime validation of NetDefs more consistent
by Laine Stump 15 Sep '19

15 Sep '19

There is some validation of NetDefs (<interface>) that can't be done until runtime, due to not knowing the exact configuration until that time. This needs to happen in 3 places: 1) in the qemu commandline when a new guest is started, 2) when an <interface> is hotplugged, and 3) when an <interface> is updated. Until now, there some (but not all) validation copy-pasted between (1) and (2), and none of that was present for (3). These patches move all the validation from (1) (which is the most complete) into a separate function, and then call that function from all three places, so the exact same validation is done in all cases. (These patches are a followup to a patch I sent *long* ago in a naive attempt to fix https://bugzilla.redhat.com/1502754 - my original patch did fix the problem when starting a guest with an invalid <interface> config, but not when hotplugging or updating such an interface.) NB: I noticed that if someone tries to specify <bandwidth> for an interface type that doesn't support it, we only give a warning, not an error, due to a fear that there are existing management apps that add <bandwidth> to all types of interfaces, and we don't want them to suddenly fail to run (I got this info from the BZ associated with the commit that added the warning). This seems to me to be going a bit too far - I think that (at least upstream) we should turn this into an error, and let the regression testing of said management apps catch the behavior change so they can fix their code. (insert Kermit-drinking-coffee meme here) Laine Stump (3): conf: make arg to virDomainNetGetActualVirtPortProfile() a const qemu: move runtime netdev validation into a separate function qemu: call common NetDef validation for hotplug and device update src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 52 +-------------------------- src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_hotplug.c | 32 +++++------------ 6 files changed, 95 insertions(+), 77 deletions(-) -- 2.21.0

2 8

[libvirt] Potential bug: spice port collision on different addresses
by Michael Edlinger 14 Sep '19

14 Sep '19

Hi all, I think I've found a bug in either libvirt or spice. I'll give a short description here, but I have a stack exchange question that also has information here: https://unix.stackexchange.com/questions/541127/how-to-listen-on-same-port-… Basically, have spice for VM1 listen to port 5900 on 10.0.0.2 and spice for VM2 listen to port 5900 on 10.0.0.3, and you'll get an error message saying the ports collide, even though they shouldn't be (they are listening on different addresses). I've tried going to the #virt irc channel with very limited responses, and would appreciate hearing what you all have to say about this issue. Thanks! - Michael

2 2

[libvirt] [PATCH] qemu_blockjob: Don't leak old job when registering a new one
by Michal Privoznik 13 Sep '19

13 Sep '19

If a disk that a block job is registered with already has a job (stored in private data) it needs to be unrefed to keep the refcounter in balance (and properly free the object): 2,577 (104 direct, 2,473 indirect) bytes in 1 blocks are definitely lost in loss record 168 of 174 at 0x4838B86: calloc (vg_replace_malloc.c:752) by 0x4A0AD42: virAllocVar (viralloc.c:346) by 0x4A84D12: virObjectNew (virobject.c:243) by 0x21EE13: qemuBlockJobDataNew (qemu_blockjob.c:111) by 0x184EBC: qemuDomainObjPrivateXMLParseBlockjobData (qemu_domain.c:3120) by 0x1854B5: qemuDomainObjPrivateXMLParseBlockjobs (qemu_domain.c:3191) by 0x186D7D: qemuDomainObjPrivateXMLParse (qemu_domain.c:3622) by 0x4B198CC: virDomainObjParseXML (domain_conf.c:21490) by 0x4B19DDF: virDomainObjParseNode (domain_conf.c:21630) by 0x4B19E7D: virDomainObjParseFile (domain_conf.c:21649) by 0x13FDD5: testCompareStatusXMLToXMLFiles (qemuxml2xmltest.c:61) by 0x16F7D2: virTestRun (testutils.c:115) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_blockjob.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index a991309ee7..67b1f677f8 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -151,6 +151,7 @@ qemuBlockJobRegister(qemuBlockJobDataPtr job, if (disk) { job->disk = disk; job->chain = virObjectRef(disk->src); + virObjectUnref(QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob); QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); } -- 2.21.0

3 2

[libvirt] [PATCH] network: fix connection usage counts after restart
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

Since the introduction of the virNetworkPort object, the network driver has a persistent record of ports that have been created against the networks. Thus the hypervisor drivers no longer communicate to the network driver during libvirtd restart. This change, however, meant that the connection usage counts were no longer re-initialized during a libvirtd restart. To deal with this we must iterate over all virNetworkPortDefPtr objects we have and invoke the notify callback to record the connection usage count. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/conf/virnetworkobj.c | 33 +++++++++++++++++++++++++++++++++ src/conf/virnetworkobj.h | 9 +++++++++ src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 17 +++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index d63ead7fac..ca1d598cf9 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1844,6 +1844,39 @@ virNetworkObjPortListExport(virNetworkPtr net, } +typedef struct _virNetworkObjPortListForEachData virNetworkObjPortListForEachData; +struct _virNetworkObjPortListForEachData { + virNetworkPortListIter iter; + void *opaque; + bool err; +}; + +static int +virNetworkObjPortForEachCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNetworkObjPortListForEachData *data = opaque; + + if (!data->iter(payload, data->opaque)) + data->err = true; + + return 0; +} + +int +virNetworkObjPortForEach(virNetworkObjPtr obj, + virNetworkPortListIter iter, + void *opaque) +{ + virNetworkObjPortListForEachData data = { iter, opaque, false }; + virHashForEach(obj->ports, virNetworkObjPortForEachCallback, &data); + if (data.err) + return -1; + return 0; +} + + static int virNetworkObjLoadAllPorts(virNetworkObjPtr net, const char *stateDir) diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h index 1c28f0888c..a91b4304c6 100644 --- a/src/conf/virnetworkobj.h +++ b/src/conf/virnetworkobj.h @@ -190,6 +190,15 @@ virNetworkObjPortListExport(virNetworkPtr net, virNetworkPortPtr **ports, virNetworkPortListFilter filter); +typedef bool +(*virNetworkPortListIter)(virNetworkPortDefPtr portdef, + void *opaque); + +int +virNetworkObjPortForEach(virNetworkObjPtr obj, + virNetworkPortListIter iter, + void *opaque); + int virNetworkObjSaveStatus(const char *statusDir, virNetworkObjPtr net, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7fe10d2286..37afb07e21 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1097,6 +1097,7 @@ virNetworkObjLookupPort; virNetworkObjMacMgrAdd; virNetworkObjMacMgrDel; virNetworkObjNew; +virNetworkObjPortForEach; virNetworkObjPortListExport; virNetworkObjRemoveInactive; virNetworkObjReplacePersistentDef; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7b44184616..0fee153cb8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -527,6 +527,21 @@ networkBridgeDummyNicName(const char *brname) } +static int +networkNotifyPort(virNetworkObjPtr obj, + virNetworkPortDefPtr port); + +static bool +networkUpdatePort(virNetworkPortDefPtr port, + void *opaque) +{ + virNetworkObjPtr obj = opaque; + + networkNotifyPort(obj, port); + + return false; +} + static int networkUpdateState(virNetworkObjPtr obj, void *opaque) @@ -591,6 +606,8 @@ networkUpdateState(virNetworkObjPtr obj, goto cleanup; } + virNetworkObjPortForEach(obj, networkUpdatePort, obj); + /* Try and read dnsmasq/radvd pids of active networks */ if (virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) { pid_t radvdPid; -- 2.21.0

2 3

[libvirt] [PATCH] network: apply bandwidth settings for forward mode=bridge
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

We previously allowed bandwidth settings when attaching NICs to networks with forward mode=bridge: commit 42a92ee93d5432ebd9ebfd409903b5287fc7d7ff Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Tue Nov 20 11:30:05 2018 +0000 network: add missing bandwidth limits for bridge forward type In the case of a network with forward=bridge, which has a bridge device listed, we are capable of setting bandwidth limits but fail to call the function to register them. Reviewed-by: Cole Robinson <crobinso(a)redhat.com> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> Unfortunately the wrong version of this patch was posted and reviewed and thus it lacked the code to actually apply the bandwidth settings to the bridge itself. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0fee153cb8..6a97bb17e2 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2743,21 +2743,39 @@ networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver, static int networkStartNetworkBridge(virNetworkObjPtr obj) { + virNetworkDefPtr def = virNetworkObjGetDef(obj); + /* put anything here that needs to be done each time a network of * type BRIDGE, is started. On failure, undo anything you've done, * and return -1. On success return 0. */ - return networkStartHandleMACTableManagerMode(obj, NULL); + if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) + goto error; + + if (networkStartHandleMACTableManagerMode(obj, NULL) < 0) + goto error; + + return 0; + + error: + if (def->bandwidth) + virNetDevBandwidthClear(def->bridge); + return -1; } static int networkShutdownNetworkBridge(virNetworkObjPtr obj ATTRIBUTE_UNUSED) { + virNetworkDefPtr def = virNetworkObjGetDef(obj); + /* put anything here that needs to be done each time a network of * type BRIDGE is shutdown. On failure, undo anything you've done, * and return -1. On success return 0. */ + if (def->bandwidth) + virNetDevBandwidthClear(def->bridge); + return 0; } -- 2.21.0

2 1