- Devel - libvirt List Archives

[libvirt] [PATCH 0/2] Avoid issues due to qemu dropping osxsave and ospke
by Christian Ehrhardt 14 May '19

14 May '19

Hi, this series tries to address a drop of commandline options by qemu in regard to osxsave [1] and ospke [2]. This was already discussed in [3] late last year but got forgotten afterwards. The Ubuntu bug is at [4] and an older Fedora bug is at [5]. TL;DR: - osxsave/ospke features were never really configurable - KVM never returned the bits on GET_SUPPORTED_CPUID - very rare to be seen in the wild - avoid issues with newer qemu and old/odd XMLs to be sure Details: I checked various use cases from virt-install to openstack and some in between. The only cases I found that would define osxsave/ospke is virt-install pior to version 2.0 and even there only when used with --cpu=host-model or --cpu=host-copy. If you ever really enabled the feature you'd have got: error: the CPU is incompatible with host CPU: Host CPU does not provide required features: ospke The problem lies in domain XMLs that explicitly disable it. That would be <feature policy='disable' name='osxsave'/> But due to almost (or actually none) no host exposing this the following also triggers: <feature policy='optional' name='ospke'/> This will make libvirt add it to the qemu commandline like: -cpu ...,osxsave=off,ospke=off And that will crash when qemu starts with: error: internal error: process exited while connecting to monitor: 2019-04-25T12:12:01.698646Z qemu-system-x86_64: can't apply global core2duo-x86_64-cpu.osxsave=off: Property '.osxsave' not found There are much more long term discussions about demoting and dropping qemu features and I'd like to avoid those discussions being mixed. The reason to drop it more or less without notice was that it never did anything to begin with. Due to that our solution might in a similar fashion be more trivial - just stop defining those two features to qemu commandline. [1]: https://git.qemu.org/?p=qemu.git;a=commit;h=f1a23522b03a569f13aad49294bb4c4… [2]: https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb9784b57804f5c74434ad6ccb666… [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg561877.html [4]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1825195 [5]: https://bugzilla.redhat.com/show_bug.cgi?id=1644848 Christian Ehrhardt (2): qemu: do not define known no-op features qemuxml2argvtest: add test for remove cpu features src/qemu/qemu_command.c | 23 +++++++++++++++ .../qemuxml2argvdata/cpu-host-model-cmt.args | 2 +- .../cpu-no-removed-features.args | 29 +++++++++++++++++++ .../cpu-no-removed-features.xml | 23 +++++++++++++++ tests/qemuxml2argvdata/cpu-tsc-frequency.args | 4 +-- tests/qemuxml2argvtest.c | 1 + 6 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.args create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.xml -- 2.17.1

2 11

[libvirt] [PATCH 0/2] Mitigation for Microarchitectural Data Sampling CPU flaws
by Jiri Denemark 14 May '19

14 May '19

This series introduces the libvirt side of mitigations for Microarchitectural Data Sampling microprocessor flaws (CVE-2018-12126, CVE-2018-12127, CVE-2018-12130, CVE-2019-11091) which were published earlier today. To protect your system against possible attacks exploiting these flaws updates to the CPU microcode, Linux kernel, and virtualization stack (QEMU, libvirt, and higher management apps) are required. See https://access.redhat.com/security/vulnerabilities/mds for more details and additional links. Both patches have already been pushed. Jiri Denemark (2): cputest: Add data for Intel(R) Xeon(R) CPU E3-1225 v5 cpu_map: Define md-clear CPUID bit src/cpu_map/x86_features.xml | 3 + tests/cputest.c | 1 + .../x86_64-cpuid-Xeon-E3-1225-v5-disabled.xml | 7 + .../x86_64-cpuid-Xeon-E3-1225-v5-enabled.xml | 8 + .../x86_64-cpuid-Xeon-E3-1225-v5-guest.xml | 29 + .../x86_64-cpuid-Xeon-E3-1225-v5-host.xml | 30 + .../x86_64-cpuid-Xeon-E3-1225-v5-json.xml | 12 + .../x86_64-cpuid-Xeon-E3-1225-v5.json | 652 ++++++++++++++++++ .../x86_64-cpuid-Xeon-E3-1225-v5.sig | 4 + .../x86_64-cpuid-Xeon-E3-1225-v5.xml | 47 ++ .../x86_64-cpuid-Xeon-Platinum-8268-guest.xml | 1 + .../x86_64-cpuid-Xeon-Platinum-8268-host.xml | 1 + 12 files changed, 795 insertions(+) create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-disabled.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-enabled.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-guest.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-host.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5-json.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5.json create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5.sig create mode 100644 tests/cputestdata/x86_64-cpuid-Xeon-E3-1225-v5.xml -- 2.21.0

2 3

[libvirt] [PATCH] qemuDomainSnapshotCreateXML: Don't leak parsed snapshot definition
by Michal Privoznik 14 May '19

14 May '19

This function gets snapshot XML (provided by used) as an argument. It parses it into a local variable @def and then sets some more members (e.g. it creates a copy of live domain XML). Then it proceeds to checking if snapshot XML is valid (e.g. it contains as many disks as currently in the domain). If this fails then the control jumps to endjob label and subsequently return from the function. This is where AUTOFREE function for @def is ran. Well, because the code says to run plain VIR_FREE() we leak some memory because @def is actually an object and therefore it should have been declared as AUTOUNREF. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- 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 f01282a037..0a425b82e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15563,7 +15563,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virDomainSnapshotState state; - VIR_AUTOFREE(virDomainSnapshotDefPtr) def = NULL; + VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | -- 2.21.0

3 2

[libvirt] [PATCH 0/4] Couple of allocation fixes
by Michal Privoznik 14 May '19

14 May '19

These stem from me playing with OOM testing. I've discovered more bugs, but they're mostly in tests themselves, e.g. use without check. Michal Prívozník (4): virstorageobj: Don't clear vols if they weren't initialized virNetServerPreExecRestart: Check for retval of virJSONValueNewArray() virCommand: Make virCommandPassFDGetFDIndex fail if passed command is in error state storagepoolxml2argvtest: Avoid double free src/conf/virstorageobj.c | 6 ++++-- src/rpc/virnetserver.c | 8 ++++++-- src/util/vircommand.c | 3 +++ tests/storagepoolxml2argvtest.c | 1 - 4 files changed, 13 insertions(+), 5 deletions(-) -- 2.21.0

3 9

[libvirt] [PATCH 0/2] node device cleanup
by Pavel Hrdina 13 May '19

13 May '19

Pavel Hrdina (2): node_device_udev: remove deprecated logging function src: remove HAL node device driver configure.ac | 3 +- docs/drvnodedev.html.in | 3 +- docs/hvsupport.pl | 2 +- libvirt.spec.in | 1 - m4/virt-hal.m4 | 10 +- m4/virt-udev.m4 | 5 - po/POTFILES | 1 - src/node_device/Makefile.inc.am | 12 - src/node_device/node_device_driver.c | 10 +- src/node_device/node_device_driver.h | 5 - src/node_device/node_device_hal.c | 804 --------------------------- src/node_device/node_device_hal.h | 22 - src/node_device/node_device_udev.c | 41 -- 13 files changed, 9 insertions(+), 910 deletions(-) delete mode 100644 src/node_device/node_device_hal.c delete mode 100644 src/node_device/node_device_hal.h -- 2.21.0

3 4

[libvirt] [PATCH] virsh: Don't leak disk targets in cmdDomBlkError
by Michal Privoznik 13 May '19

13 May '19

The virDomainGetDiskErrors() API copies disk targets into @disks array that we allocate. But we forgot to free it: ==140828== 16 bytes in 4 blocks are definitely lost in loss record 41 of 242 ==140828== at 0x4C2F08F: malloc (vg_replace_malloc.c:299) ==140828== by 0x8C406D9: strdup (in /lib64/libc-2.28.so) ==140828== by 0x5377DD3: virStrdup (virstring.c:966) ==140828== by 0x54C112F: testDomainGetDiskErrors (test_driver.c:3068) ==140828== by 0x55C863D: virDomainGetDiskErrors (libvirt-domain.c:10988) ==140828== by 0x15D1FA: cmdDomBlkError (virsh-domain-monitor.c:1215) ==140828== by 0x17F1A8: vshCommandRun (vsh.c:1335) ==140828== by 0x13489E: main (virsh.c:920) Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed under trivial rule. tools/virsh-domain-monitor.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index d87475f6f6..d2b7e5a059 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1229,6 +1229,8 @@ cmdDomBlkError(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + for (i = 0; i < count; i++) + VIR_FREE(disks[i].disk); VIR_FREE(disks); virshDomainFree(dom); return ret; -- 2.21.0

1 0

[libvirt] [PATCH 0/4] docs: Ship and install a few more files
by Andrea Bolognani 13 May '19

13 May '19

Our handling of documentation is far from perfect, and a few issues in particular are causing me quite a bit of grief on the Debian packaging side of things. This series takes care of the more pressing matters; I have further cleanups planned for later. Andrea Bolognani (4): docs: Introduce $(modules) docs: Introduce $(devhelphtml_generated) docs: Install documentation under $(docdir) examples: Install under $(docdir) configure.ac | 13 ------------- docs/Makefile.am | 38 ++++++++++++++++++++++++-------------- examples/Makefile.am | 11 +++++++++++ libvirt.spec.in | 14 +------------- 4 files changed, 36 insertions(+), 40 deletions(-) -- 2.20.1

3 7

[libvirt] [PATCH] util: fix a bug found in sorting cache monitor results
by Wang Huaqiang 13 May '19

13 May '19

From: Huaqiang <huaqiang.wang(a)intel.com> The qsort element is a pointer of virResctrlMonitorStats, and the comparing function's arguments have a type of pointer of virResctrlMonitorStatsPtr. Signed-off-by: Huaqiang <huaqiang.wang(a)intel.com> --- src/util/virresctrl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index b845f36..a190977 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2659,8 +2659,8 @@ static int virResctrlMonitorStatsSorter(const void *a, const void *b) { - return ((virResctrlMonitorStatsPtr)a)->id - - ((virResctrlMonitorStatsPtr)b)->id; + return (*(virResctrlMonitorStatsPtr *)a)->id + - (*(virResctrlMonitorStatsPtr *)b)->id; } @@ -2758,7 +2758,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, /* Sort in id's ascending order */ if (*nstats) - qsort(*stats, *nstats, sizeof(*stat), virResctrlMonitorStatsSorter); + qsort(*stats, *nstats, sizeof(stat), virResctrlMonitorStatsSorter); ret = 0; cleanup: -- 2.7.4

3 2

[libvirt] [PATCH RFC] network: Delay creating private chains until starting network
by Jim Fehlig 13 May '19

13 May '19

Automated performance tests found that network-centric workloads suffered a 20 percent decrease when the host libvirt was updated from 5.0.0 to 5.1.0. On the test hosts libvirtd is enabled to start at boot and the "default" network is defined, but it is not set to autostart. libvirt 5.1.0 introduced private firewall chains with commit 5f1e6a7d. The chains are created at libvirtd start, which triggers loading the conntrack kernel module. Subsequent tracking and processing of connections resulted in the performance hit. Prior to commit 5f1e6a7d the conntrack module would not be loaded until starting a network, when libvirt added rules to the builtin chains. Restore the behavior of previous libvirt versions by delaying the creation of private chains until the first network is started. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- I briefly discussed this issue with Daniel on IRC and just now finding time to bring it to the list for broader discussion. The adjustment to the test file feels hacky. The whole approach might by hacky, hence the RFC. src/network/bridge_driver_linux.c | 64 +++------- .../nat-default-linux.args | 116 ++++++++++++++++++ 2 files changed, 131 insertions(+), 49 deletions(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index f2827543ca..a3a09caeba 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -35,44 +35,10 @@ VIR_LOG_INIT("network.bridge_driver_linux"); #define PROC_NET_ROUTE "/proc/net/route" -static virErrorPtr errInitV4; -static virErrorPtr errInitV6; +static bool pvtChainsCreated; void networkPreReloadFirewallRules(bool startup) { - bool created = false; - int rc; - - /* We create global rules upfront as we don't want - * the perf hit of conditionally figuring out whether - * to create them each time a network is started. - * - * Any errors here are saved to be reported at time - * of starting the network though as that makes them - * more likely to be seen by a human - */ - rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4); - if (rc < 0) { - errInitV4 = virSaveLastError(); - virResetLastError(); - } else { - virFreeError(errInitV4); - errInitV4 = NULL; - } - if (rc) - created = true; - - rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6); - if (rc < 0) { - errInitV6 = virSaveLastError(); - virResetLastError(); - } else { - virFreeError(errInitV6); - errInitV6 = NULL; - } - if (rc) - created = true; - /* * If this is initial startup, and we just created the * top level private chains we either @@ -86,8 +52,8 @@ void networkPreReloadFirewallRules(bool startup) * rules will be present. Thus we can safely just tell it * to always delete from the builin chain */ - if (startup && created) - iptablesSetDeletePrivate(false); + if (startup) + iptablesSetDeletePrivate(true); } @@ -701,19 +667,19 @@ int networkAddFirewallRules(virNetworkDefPtr def) virFirewallPtr fw = NULL; int ret = -1; - if (errInitV4 && - (virNetworkDefGetIPByIndex(def, AF_INET, 0) || - virNetworkDefGetRouteByIndex(def, AF_INET, 0))) { - virSetError(errInitV4); - return -1; - } + if (!pvtChainsCreated) { + if (iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4) < 0 && + (virNetworkDefGetIPByIndex(def, AF_INET, 0) || + virNetworkDefGetRouteByIndex(def, AF_INET, 0))) + return -1; - if (errInitV6 && - (virNetworkDefGetIPByIndex(def, AF_INET6, 0) || - virNetworkDefGetRouteByIndex(def, AF_INET6, 0) || - def->ipv6nogw)) { - virSetError(errInitV6); - return -1; + if (iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6) < 0 && + (virNetworkDefGetIPByIndex(def, AF_INET6, 0) || + virNetworkDefGetRouteByIndex(def, AF_INET6, 0) || + def->ipv6nogw)) + return -1; + + pvtChainsCreated = true; } if (def->bridgeZone) { diff --git a/tests/networkxml2firewalldata/nat-default-linux.args b/tests/networkxml2firewalldata/nat-default-linux.args index c9d523d043..61d620b592 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.args +++ b/tests/networkxml2firewalldata/nat-default-linux.args @@ -1,5 +1,121 @@ iptables \ --table filter \ +--list-rules +iptables \ +--table nat \ +--list-rules +iptables \ +--table mangle \ +--list-rules +iptables \ +--table filter \ +--new-chain LIBVIRT_INP +iptables \ +--table filter \ +--insert INPUT \ +--jump LIBVIRT_INP +iptables \ +--table filter \ +--new-chain LIBVIRT_OUT +iptables \ +--table filter \ +--insert OUTPUT \ +--jump LIBVIRT_OUT +iptables \ +--table filter \ +--new-chain LIBVIRT_FWO +iptables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWO +iptables \ +--table filter \ +--new-chain LIBVIRT_FWI +iptables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWI +iptables \ +--table filter \ +--new-chain LIBVIRT_FWX +iptables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWX +iptables \ +--table nat \ +--new-chain LIBVIRT_PRT +iptables \ +--table nat \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +iptables \ +--table mangle \ +--new-chain LIBVIRT_PRT +iptables \ +--table mangle \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +ip6tables \ +--table filter \ +--list-rules +ip6tables \ +--table nat \ +--list-rules +ip6tables \ +--table mangle \ +--list-rules +ip6tables \ +--table filter \ +--new-chain LIBVIRT_INP +ip6tables \ +--table filter \ +--insert INPUT \ +--jump LIBVIRT_INP +ip6tables \ +--table filter \ +--new-chain LIBVIRT_OUT +ip6tables \ +--table filter \ +--insert OUTPUT \ +--jump LIBVIRT_OUT +ip6tables \ +--table filter \ +--new-chain LIBVIRT_FWO +ip6tables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWO +ip6tables \ +--table filter \ +--new-chain LIBVIRT_FWI +ip6tables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWI +ip6tables \ +--table filter \ +--new-chain LIBVIRT_FWX +ip6tables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWX +ip6tables \ +--table nat \ +--new-chain LIBVIRT_PRT +ip6tables \ +--table nat \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +ip6tables \ +--table mangle \ +--new-chain LIBVIRT_PRT +ip6tables \ +--table mangle \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +iptables \ +--table filter \ --insert LIBVIRT_INP \ --in-interface virbr0 \ --protocol tcp \ -- 2.21.0

3 4

[libvirt] [PATCH] qemu: Fix regression with undefine --snapshots-metadata
by Eric Blake 10 May '19

10 May '19

In refactoring the snapshot code to prepare for checkpoints, I changed qemuDomainMomentDiscardAll to take a callback that would handle the cleanup of either a snapshot or a checkpoint, but failed to set the callback on one of the two snapshot callers. As a result, 'virsh undefine $dom --snapshots-metadata' crashed on a NULL function dereference. Fixes: a487890d371b8cc3662c1717dfe07eea3f1ef1c0 Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2af17816c6..3290c5d490 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8664,7 +8664,8 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, virQEMUMomentRemove rem = { .driver = driver, .vm = vm, - .metadata_only = true + .metadata_only = true, + .momentDiscard = qemuDomainSnapshotDiscard, }; virDomainSnapshotForEach(vm->snapshots, qemuDomainMomentDiscardAll, &rem); -- 2.20.1

2 1