- Devel - libvirt List Archives

[libvirt] [PATCH] nwfilter: Don't crash when trying to add an nwfilter that's already being removed
by Michal Privoznik 19 Mar '19

19 Mar '19

https://bugzilla.redhat.com/show_bug.cgi?id=1686927 When trying to create a nwfilter binding via nwfilterBindingCreateXML() we may encounter a crash. The sequence of functions called is as follows: 1) nwfilterBindingCreateXML() parses the XML and calls virNWFilterBindingObjListAdd() which calls virNWFilterBindingObjListAddLocked() 2) Here, @binding is not found because binding->remove is set. 3) Therefore, controls continue with creating new @binding, setting its def to the one from 1) and adding it to the hash table. 4) This fails, because the binding is still in the hash table (duplicate key is detected). 5) The control jumps to 'error' label where virNWFilterBindingObjEndAPI() is called which frees the binding definition passed. 6) Error is propagated to the caller, which calls virNWFilterBindingDefFree() over the definition again. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/virnwfilterbindingobjlist.c | 11 ++++++----- src/conf/virnwfilterbindingobjlist.h | 2 +- src/nwfilter/nwfilter_driver.c | 5 ++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 06ccbf53af..87189da642 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -164,23 +164,24 @@ virNWFilterBindingObjListAddObjLocked(virNWFilterBindingObjListPtr bindings, */ static virNWFilterBindingObjPtr virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings, - virNWFilterBindingDefPtr def) + virNWFilterBindingDefPtr *def) { virNWFilterBindingObjPtr binding; /* See if a binding with matching portdev already exists */ if ((binding = virNWFilterBindingObjListFindByPortDevLocked( - bindings, def->portdevname))) { + bindings, (*def)->portdevname))) { virReportError(VIR_ERR_OPERATION_FAILED, _("binding '%s' already exists"), - def->portdevname); + (*def)->portdevname); goto error; } if (!(binding = virNWFilterBindingObjNew())) goto error; - virNWFilterBindingObjSetDef(binding, def); + virNWFilterBindingObjSetDef(binding, *def); + *def = NULL; if (virNWFilterBindingObjListAddObjLocked(bindings, binding) < 0) goto error; @@ -195,7 +196,7 @@ virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings, virNWFilterBindingObjPtr virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings, - virNWFilterBindingDefPtr def) + virNWFilterBindingDefPtr *def) { virNWFilterBindingObjPtr ret; diff --git a/src/conf/virnwfilterbindingobjlist.h b/src/conf/virnwfilterbindingobjlist.h index b0fb90f667..4047453634 100644 --- a/src/conf/virnwfilterbindingobjlist.h +++ b/src/conf/virnwfilterbindingobjlist.h @@ -35,7 +35,7 @@ virNWFilterBindingObjListFindByPortDev(virNWFilterBindingObjListPtr bindings, virNWFilterBindingObjPtr virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings, - virNWFilterBindingDefPtr def); + virNWFilterBindingDefPtr *def); void virNWFilterBindingObjListRemove(virNWFilterBindingObjListPtr bindings, diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index fdfc6f48fa..8c2e987b5d 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -759,7 +759,7 @@ nwfilterBindingCreateXML(virConnectPtr conn, goto cleanup; obj = virNWFilterBindingObjListAdd(driver->bindings, - def); + &def); if (!obj) goto cleanup; @@ -775,8 +775,7 @@ nwfilterBindingCreateXML(virConnectPtr conn, virNWFilterBindingObjSave(obj, driver->bindingDir); cleanup: - if (!obj) - virNWFilterBindingDefFree(def); + virNWFilterBindingDefFree(def); virNWFilterBindingObjEndAPI(&obj); return ret; -- 2.19.2

3 4

[libvirt] [PATCH] conf: Drop unused variable
by Andrea Bolognani 19 Mar '19

19 Mar '19

The refresh_volume_allocation variable in virStoragePoolDefParseXML() has been unused since its introduction in commit 669018bc9cb1, and Clang rightfully complains about this fact. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- Pushed under the build breaker rule. src/conf/storage_conf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 653602f7be..b797a405f7 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -849,7 +849,6 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) VIR_AUTOFREE(char *) type = NULL; VIR_AUTOFREE(char *) uuid = NULL; VIR_AUTOFREE(char *) target_path = NULL; - VIR_AUTOFREE(char *) refresh_volume_allocation = NULL; if (VIR_ALLOC(def) < 0) return NULL; -- 2.20.1

1 0

[libvirt] [PATCH] spec: Only call ldconfig on RHEL7
by Cole Robinson 19 Mar '19

19 Mar '19

Since Fedora 28 (our minimum supported build), ldconfig is called automatically for us: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets These changes appear to be implemented for rhel > 7 as well, so only run ldconfig on rhel7 Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- libvirt.spec.in | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 1497cad3d2..445fddc801 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1477,12 +1477,16 @@ exit 0 %post client +%if 0%{?rhel} == 7 /sbin/ldconfig +%endif %systemd_post libvirt-guests.service %postun client +%if 0%{?rhel} == 7 /sbin/ldconfig +%endif %systemd_postun libvirt-guests.service %triggerun client -- libvirt < 0.9.4 -- 2.21.0

2 5

[libvirt] [PATCH v3 0/3] rbd: improvements to actual disk-usage calculation
by jdillama＠redhat.com 19 Mar '19

19 Mar '19

From: Jason Dillaman <dillaman(a)redhat.com> The RBD fast-diff feature can vastly reduce the amount of time needed to calculate actual disk usage of volumes, but it might still be a slow operation for large RBD pools or pools with large RBD images. Therefore, this feature should be able to be optionally disabled if needed. Additionally, the fast-diff feature can only be used if the fast-diff map isn't flagged as invalid. Otherwise, librbd will silently perform a costly block-by-block scan to calculate the disk usage. since v1: - Moved RBD-unique refresh volume allocation override to shared storage pool attribute sice v2: - Moved attribute under new "<refresh><volume allocation='...'/></refresh>" elements. Jason Dillaman (3): rbd: do not attempt to use fast-diff if it's marked invalid storage: optional 'refresh' elemement on pool rbd: optionally compute volume allocation from capacity docs/formatstorage.html.in | 27 ++++++++++++ docs/schemas/storagecommon.rng | 7 ++++ docs/schemas/storagepool.rng | 23 ++++++++++ src/conf/storage_conf.c | 27 ++++++++++++ src/conf/storage_conf.h | 9 ++++ src/storage/storage_backend_rbd.c | 42 +++++++++++++++++-- .../pool-rbd-refresh-volume-allocation.xml | 15 +++++++ .../pool-rbd-refresh-volume-allocation.xml | 18 ++++++++ tests/storagepoolxml2xmltest.c | 1 + 9 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-refresh-volume-allocation.xml create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-refresh-volume-allocation.xml -- 2.20.1

2 7

[libvirt] [PATCH 00/34] conf: refactor virStorageSource parsing and formatting (blockdev-add saga)
by Peter Krempa 19 Mar '19

19 Mar '19

Simplify parsing and formatting of virStorageSource into/from XML. This series contains the refactors which unify the various parsers into one with a neat cleanup. The plan is to reuse the parser for parsing job backing chains when the disk frontend will be unplugged by the guest so that we don't lose track of the backing chain members. This series adds support for backing chain of the <mirror> subelement which has a similar reason. If we open a user-formatted file (--reuse-external) and do a shallow copy into it we will need to keep around the backing chain for that as well. It feels fitting to format the backing chain under <mirror> as it might become the disk source later. Peter Krempa (34): conf: Invert 'skipSeclabels' argument of virDomainDiskSourceFormatInternal conf: Move formatting of 'index' and 'startupPolicy' for virStorageSource conf: Introduce virDomainStorageSourceFormatFull qemu: domain: Replace qemuDomainObjPrivateXMLFormatNBDMigrationSource conf: Unexport virDomainStorageSourceFormat conf: domain: Merge virDomainDiskSourceFormatInternal into the wrapper conf: Simplify control flow in virDomainDiskSourceFormat conf: Avoid temporary variable in virDomainDiskBackingStoreFormat conf: Use virXMLFormatElement in virDomainDiskBackingStoreFormat conf: Move virDomainDiskBackingStoreFormat up to avoid forward declarations conf: Move backingStore formating into virDomainDiskSourceFormat conf: Add possibility to format full chain with virDomainStorageSourceFormatFull conf: Simplify error paths in storage source component parsers cleanup error path in virDomainStorageSourceParse util: xml: Introduce VIR_AUTOPTR functions for xmlDoc and xmlXPathContext tests: Use full force of our VIR_AUTO* machinery in testBackingXMLjsonXML tests: Refactor control flow in testBackingXMLjsonXML conf: Refactor control flow in virDomainDiskBackingStoreParse conf: Fold private data parsing into virDomainStorageSourceParse conf: Introduce modular parser for virStorageSource qemu: Use virDomainStorageSourceParseFull when parsing NBD migration data conf: Unexport virDomainStorageSourceParse tests: qemublock: Use new source formatter and parser in testBackingXMLjsonXML conf: snapshot: Use virDomainStorageSourceParseFull for snapshots conf: Replace virDomainDiskSourceParse by virDomainStorageSourceParse conf: Use virDomainStorageSourceParseFull in virDomainDiskBackingStoreParse conf: Use virDomainStorageSourceParseFull to parse disk source in virDomainDiskDefParseXML conf: Use virDomainStorageSourceParseFull in virDomainDiskDefMirrorParse conf: Parse <backingStore> in virDomainStorageSourceParseFull conf: Parse and format 'backingStore' for disk <mirror> conf: Add 'index' attribute for <disk><mirror><source> conf: Format seclabels for <backingStore> conf: Remove @seclabels from virDomainDiskSourceFormat conf: Remove @seclabels from virDomainStorageSourceFormat docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 494 ++++++++++-------- src/conf/domain_conf.h | 32 +- src/conf/snapshot_conf.c | 40 +- src/libvirt_private.syms | 4 +- src/qemu/qemu_domain.c | 104 +--- src/util/virxml.h | 3 + tests/qemublocktest.c | 179 +++---- .../blockjob-mirror-in.xml | 13 + .../qemuxml2argvdata/disk-backing-chains.xml | 6 +- tests/qemuxml2argvdata/disk-mirror.xml | 8 +- .../disk-backing-chains-active.xml | 6 +- .../disk-backing-chains-inactive.xml | 6 +- .../qemuxml2xmloutdata/disk-mirror-active.xml | 8 +- tests/virstoragetest.c | 2 +- 15 files changed, 443 insertions(+), 463 deletions(-) -- 2.20.1

2 68

[libvirt] [PATCH 0/2] numa: deprecate -numa node, mem and default memory distribution
by Igor Mammedov 19 Mar '19

19 Mar '19

1) "I'm considering to deprecating -mem-path/prealloc CLI options and replacing them with a single memdev Machine property to allow interested users to pick used backend for initial RAM (fixes mixed -mem-path+hostmem backends issues) and as a transition step to modeling initial as a Device instead of (ab)using MemoryRegion APIs." (for more details see: https://www.mail-archive.com/qemu-devel@nongnu.org/msg596314.html) However there is a couple of roadblocks on the way (s390x and numa memory handling). I think I finally thought out a way to hack s390x in migration compatible manner, but I don't see a way to do it for -numa node,mem and default RAM assignement to nodes. Considering both numa usecases aren't meaningfully using NUMA (aside guest side testing), and could be replaced with explicitly used memdev parameter, I'd like to propose removing these fake NUMA friends, hence this deprecation. As result of removing deprecated options and replacing initial RAM allocation with 'memdev's (1), QEMU will allocate guest RAM in consistent way, fixing mixed use-case and allowing boards to move towards modelling initial RAM as Device(s). Which in its own turn should allow to cleanup NUMA/HMP/memory accounting code more by dropping ad-hoc node_mem tracking and reusing device enumeration instead. Igor Mammedov (2): numa: deprecate 'mem' parameter of '-numa node' option numa: deprecate implict memory distribution between nodes numa.c | 5 +++++ qemu-deprecated.texi | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) -- 2.7.4

6 39

[libvirt] [PATCH v2 0/3] rbd: improvements to actual disk-usage calculation
by jdillama＠redhat.com 19 Mar '19

19 Mar '19

From: Jason Dillaman <dillaman(a)redhat.com> The RBD fast-diff feature can vastly reduce the amount of time needed to calculate actual disk usage of volumes, but it might still be a slow operation for large RBD pools or pools with large RBD images. Therefore, this feature should be able to be optionally disabled if needed. Additionally, the fast-diff feature can only be used if the fast-diff map isn't flagged as invalid. Otherwise, librbd will silently perform a costly block-by-block scan to calculate the disk usage. since v1: - Moved RBD-unique refresh volume allocation override to shared storage pool attribute Jason Dillaman (3): rbd: do not attempt to use fast-diff if it's marked invalid storage: optional 'refresh_volume_allocation' attribute on pool rbd: optionally compute volume allocation from capacity docs/formatstorage.html.in | 15 +++++-- docs/schemas/storagecommon.rng | 7 ++++ docs/schemas/storagepool.rng | 5 +++ src/conf/storage_conf.c | 23 ++++++++++ src/conf/storage_conf.h | 9 ++++ src/storage/storage_backend_rbd.c | 42 +++++++++++++++++-- .../pool-rbd-refresh-volume-allocation.xml | 12 ++++++ .../pool-rbd-refresh-volume-allocation.xml | 15 +++++++ tests/storagepoolxml2xmltest.c | 1 + 9 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-refresh-volume-allocation.xml create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-refresh-volume-allocation.xml -- 2.20.1

3 7

[libvirt] [PATCH] network: avoid trying to create global firewall rules if unprivileged
by Daniel P. Berrangé 19 Mar '19

19 Mar '19

The unprivileged libvirtd does not have permission to create firewall rules, or bridge devices, or do anything to the host network in general. Historically we still activate the network driver though and let the network start API call fail. The startup code path which reloads firewall rules on active networks would thus effectively be a no-op when unprivileged as it is impossible for there to be any active networks With the change to use a global set of firewall chains, however, we now have code that is run unconditionally. Ideally we would not register the network driver at all when unprivileged, but the entanglement with the virt drivers currently makes that impractical. As a temporary hack, we just make the firewall reload into a no-op. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c3e1381124..7d95675623 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2095,6 +2095,10 @@ static void networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup) { VIR_INFO("Reloading iptables rules"); + /* Ideally we'd not even register the driver when unprivilegd + * but until we untangle the virt driver that's not viable */ + if (!driver->privileged) + return; if (networkPreReloadFirewallRules(startup) < 0) return; virNetworkObjListForEach(driver->networks, -- 2.20.1

2 2

[libvirt] [PATCH 0/2] network: improve firewall rule creation error handling
by Daniel P. Berrangé 19 Mar '19

19 Mar '19

This is a different approach to solving the problem describd in: https://www.redhat.com/archives/libvir-list/2019-March/msg00584.html That patch would treat each chain creation attempt as non-fatal. This means ipv4 chains still get created if ipv6 is missing, or if a subset of ip[6]tables modules are missing (eg "mangle" chain). This series takes a different approach of splitting IPv4 and IPv6 chain creation. Setup for either address family can succeed/fail independently, however, within an address family everything must still succeed. Improved error reporting means that users will see the root cause error when trying to start an error. So with this series, 'mangle' support is still compulsory for any address family, but if IPv6 lacks mangle support, this won't break IPv4 support. This is good for the default network which only does IPv4 out of the box. Daniel P. Berrangé (2): network: improve error report when firewall chain creation fails network: split setup of ipv4 and ipv6 top level chains src/network/bridge_driver.c | 3 +- src/network/bridge_driver_linux.c | 51 ++++++++++++++++++++++++---- src/network/bridge_driver_nop.c | 3 +- src/network/bridge_driver_platform.h | 2 +- src/util/viriptables.c | 14 +++----- src/util/viriptables.h | 2 +- 6 files changed, 53 insertions(+), 22 deletions(-) -- 2.20.1

3 5

[libvirt] [PATCH] spec: Remove libvirt < 0.9.4 upgrade compat
by Cole Robinson 19 Mar '19

19 Mar '19

These blocks are only triggered when updating from a libvirt version less than 0.9.4, which was released in august 2011. I think it's been long enough that we can say this upgrade path is unsupported without an intermediate step Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- libvirt.spec.in | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 1497cad3d2..dd725baf24 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1444,16 +1444,6 @@ fi rm -rf %{_localstatedir}/lib/rpm-state/libvirt || : -%triggerun -- libvirt < 0.9.4 -%{_bindir}/systemd-sysv-convert --save libvirtd >/dev/null 2>&1 ||: - -# If the package is allowed to autostart: -/bin/systemctl --no-reload enable libvirtd.service >/dev/null 2>&1 ||: - -# Run these because the SysV package being removed won't do them -/sbin/chkconfig --del libvirtd >/dev/null 2>&1 || : -/bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || : - %if %{with_qemu} %pre daemon-driver-qemu # We want soft static allocation of well-known ids, as disk images @@ -1485,15 +1475,6 @@ exit 0 /sbin/ldconfig %systemd_postun libvirt-guests.service -%triggerun client -- libvirt < 0.9.4 -%{_bindir}/systemd-sysv-convert --save libvirt-guests >/dev/null 2>&1 ||: - -# If the package is allowed to autostart: -/bin/systemctl --no-reload enable libvirt-guests.service >/dev/null 2>&1 ||: - -# Run this because the SysV package being removed won't do them -/sbin/chkconfig --del libvirt-guests >/dev/null 2>&1 || : - %if %{with_sanlock} %post lock-sanlock if getent group sanlock > /dev/null ; then -- 2.21.0

2 1