- Devel - libvirt List Archives

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

16 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 16 Sep '19

16 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 16 Sep '19

16 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 14 Sep '19

14 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

[libvirt] [PATCH] network: add debug when bandwidth settings are not applied
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

To aid in troubleshooting add some debug messages wrt bandwidth settings and networks. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6a97bb17e2..c54be96407 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5192,9 +5192,14 @@ networkCheckBandwidth(virNetworkObjPtr obj, return -1; } + if (!netBand || !netBand->in) { + VIR_DEBUG("No network bandwidth controls present"); + /* no QoS required, claim success */ + return 1; + } if (((!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor) && - (!oldBandwidth || !oldBandwidth->in || !oldBandwidth->in->floor)) || - !netBand || !netBand->in) { + (!oldBandwidth || !oldBandwidth->in || !oldBandwidth->in->floor))) { + VIR_DEBUG("No old/new interface bandwidth floor"); /* no QoS required, claim success */ return 1; } -- 2.21.0

2 1

[libvirt] [PATCH 0/9] Pass identity information between daemons
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

This was previously posted as part of the daemon split series: https://www.redhat.com/archives/libvir-list/2019-July/msg01754.html The patches were not merged at the time since they needed more work which is now done by this series. This is all about enabling the polkit fine grain auth checks to work correctly with the split daemons A mgmt app running non-root will connect to virtqemud. virtqemud sees the client identity as the non-root user. virtqemud in turn may connect to virtnetworkd, forwarding API calls that the mgmt app makes. virtnetworkd sees the client identity as the root user. This series allows virtqemud to pass on the non-root identity of the mgmt app to virtnetworkd, so that polkit checks are done against the correct identity. Daniel P. Berrangé (9): api: introduce virConnectSetIdentity for passing uid, gid, selinux info util: change identity class attribute names tests: fix debug messages wrt selinux context when test fails util: make generic identity accessors private util: removed unused virIdentityIsEqual method util: sanitize return values for virIdentity getters util: store identity attrs as virTypedParameter internally util: allow identity to be imported/exported as typed parameters remote: pass identity across to newly opened daemons include/libvirt/libvirt-host.h | 74 +++++ src/access/viraccessdriverpolkit.c | 22 +- src/admin/admin_server.c | 52 +-- src/driver-hypervisor.h | 7 + src/libvirt-host.c | 51 +++ src/libvirt_private.syms | 21 +- src/libvirt_public.syms | 4 + src/libvirt_remote.syms | 1 + src/remote/remote_daemon_dispatch.c | 112 ++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 +- src/remote_protocol-structs | 8 + src/rpc/virnetserverclient.c | 20 +- src/rpc/virnetserverclient.h | 2 + src/util/viridentity.c | 488 ++++++++++++++++------------ src/util/viridentity.h | 71 ++-- tests/viridentitytest.c | 115 ++----- tests/virnetserverclienttest.c | 40 +-- 18 files changed, 672 insertions(+), 433 deletions(-) -- 2.21.0

2 12

[libvirt] [PATCH] tests: remove use of virTestOOMActive from bhyve testsuite
by Daniel P. Berrangé 13 Sep '19

13 Sep '19

The virTestOOMActive method was deleted in commit 2c52ecd96086b4643b99b4570b5823d40ce2787b Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Thu Aug 29 13:04:07 2019 +0100 util: purge all code for testing OOM handling Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a CI build fix for FreeBSD tests/bhyveargv2xmltest.c | 44 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/tests/bhyveargv2xmltest.c b/tests/bhyveargv2xmltest.c index 1684f03355..d108366682 100644 --- a/tests/bhyveargv2xmltest.c +++ b/tests/bhyveargv2xmltest.c @@ -39,38 +39,36 @@ testCompareXMLToArgvFiles(const char *xmlfile, if (!(vmdef = bhyveParseCommandLineString(cmd, driver.bhyvecaps, driver.xmlopt))) { - if ((flags & FLAG_EXPECT_FAILURE) && !virTestOOMActive()) { - VIR_TEST_DEBUG("Got expected failure from " - "bhyveParseCommandLineString."); + if ((flags & FLAG_EXPECT_FAILURE)) { + VIR_TEST_DEBUG("Got expected failure from " + "bhyveParseCommandLineString."); } else { goto fail; } - } else if ((flags & FLAG_EXPECT_FAILURE) && !virTestOOMActive()) { + } else if ((flags & FLAG_EXPECT_FAILURE)) { VIR_TEST_DEBUG("Did not get expected failure from " "bhyveParseCommandLineString."); goto fail; } - if (!virTestOOMActive()) { - if ((log = virTestLogContentAndReset()) == NULL) + if ((log = virTestLogContentAndReset()) == NULL) + goto fail; + if (flags & FLAG_EXPECT_WARNING) { + if (*log) { + VIR_TEST_DEBUG("Got expected warning from " + "bhyveParseCommandLineString:\n%s", + log); + } else { + VIR_TEST_DEBUG("bhyveParseCommandLineString " + "should have logged a warning"); + goto fail; + } + } else { /* didn't expect a warning */ + if (*log) { + VIR_TEST_DEBUG("Got unexpected warning from " + "bhyveParseCommandLineString:\n%s", + log); goto fail; - if (flags & FLAG_EXPECT_WARNING) { - if (*log) { - VIR_TEST_DEBUG("Got expected warning from " - "bhyveParseCommandLineString:\n%s", - log); - } else { - VIR_TEST_DEBUG("bhyveParseCommandLineString " - "should have logged a warning"); - goto fail; - } - } else { /* didn't expect a warning */ - if (*log) { - VIR_TEST_DEBUG("Got unexpected warning from " - "bhyveParseCommandLineString:\n%s", - log); - goto fail; - } } } -- 2.21.0

1 0