April 2019 - Devel - libvirt List Archives

[libvirt] [PATCH v1] qemu: Convert to virErrorPreserveLast/virErrorRestore
by Syed Humaid 20 May '19

20 May '19

Replaced usage of virSaveLastError and virSetError/virFreeError with virErrorPreserveLast and virErrorRestore respectively. Signed-off-by: Syed Humaid <syedhumaidbinharoon(a)gmail.com> --- src/qemu/qemu_cgroup.c | 7 ++----- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 28 ++++++++-------------------- 4 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ca76c4fdfa..c8d9ae4808 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1123,10 +1123,7 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, if (period) { virErrorPtr saved = virSaveLastError(); ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period)); - if (saved) { - virSetError(saved); - virFreeError(saved); - } + virErrorRestore(&saved); } return -1; @@ -1334,7 +1331,7 @@ qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data) if (!data) return; - err = virSaveLastError(); + virErrorPreserveLast(&err); virCgroupSetCpusetMems(data->emulatorCgroup, data->emulatorMemMask); virSetError(err); virFreeError(err); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1425d97b1e..9772e4f9af 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9119,7 +9119,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, error: /* free up any resources in the network driver * but don't overwrite the original error */ - originalError = virSaveLastError(); + virErrorPreserveLast(&originalError); for (i = 0; last_good_net != -1 && i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); virSetError(originalError); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 960aaff3c7..dff1d7b1a5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8042,7 +8042,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver, /* We don't care about errors logging taint info, so * preserve original error, and clear any error that * is raised */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); if (!(timestamp = virTimeStringNow())) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f48d9256e4..51b16f5aa1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6844,7 +6844,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, * must manually kill it and ignore any error related to * the process */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); VIR_FORCE_CLOSE(intermediatefd); VIR_FORCE_CLOSE(*fd); } @@ -6855,10 +6855,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); } VIR_FORCE_CLOSE(intermediatefd); @@ -14398,7 +14395,7 @@ qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, agent = qemuDomainObjEnterAgent(vm); if (!report) - err = virSaveLastError(); + virErrorPreserveLast(&err); thawed = qemuAgentFSThaw(agent); if (!report) virSetError(err); @@ -15308,7 +15305,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, error: if (ret < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); for (i = 0; i < snapdef->ndisks; i++) { if (!diskdata[i].src) continue; @@ -15351,10 +15348,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virJSONValueFree(actions); virObjectUnref(cfg); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); return ret; } @@ -17773,7 +17767,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; if (ret < 0) { - monitor_error = virSaveLastError(); + virErrorPreserveLast(&monitor_error); qemuDomainDiskChainElementRevoke(driver, vm, mirror); goto endjob; } @@ -17795,10 +17789,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, VIR_WARN("%s", _("unable to remove just-created copy target")); virStorageFileDeinit(mirror); qemuDomainObjEndJob(driver, vm); - if (monitor_error) { - virSetError(monitor_error); - virFreeError(monitor_error); - } + virErrorRestore(&monitor_error); qemuBlockJobStartupFinalize(job); cleanup: @@ -18196,10 +18187,7 @@ qemuDomainBlockCommit(virDomainPtr dom, if (top_parent && top_parent != disk->src) qemuDomainDiskChainElementPrepare(driver, vm, top_parent, true, false); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); } qemuBlockJobStartupFinalize(job); qemuDomainObjEndJob(driver, vm); -- 2.20.1

2 1

[libvirt] [PATCH v2] qemu: Add entry for balloon stats stat-htlb-pgalloc and stat-htlb-pgfail
by Han Han 20 May '19

20 May '19

Qemu added reporting of virtio balloon new statistics stat-htlb-pgalloc and stat-htlb-pgfail since qemu-3.0 commit b7b12644297. The value of stat-htlb-pgalloc represents the number of successful hugetlb page allocations while stat-htlb-pgfail represents the number of failed ones. Add this statistics reporting to libvirt. To enable this feature for vm, guest kenel >= 4.17 is required because the exporting hugetlb page allocation for virtio balloon is introduced since 6c64fe7f. Signed-off-by: Han Han <hhan(a)redhat.com> --- include/libvirt/libvirt-domain.h | 14 +++++++++++++- src/libvirt-domain.c | 5 ++++- src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_monitor_json.c | 5 +++++ tools/virsh-domain-monitor.c | 4 ++++ tools/virsh.pod | 2 ++ 6 files changed, 30 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7d36820b5a..192d25c1db 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -636,11 +636,23 @@ typedef enum { */ VIR_DOMAIN_MEMORY_STAT_DISK_CACHES = 10, + /* + * The amount of successful hugetlb(Huge Page Tables) allocations via + * virtio balloon. + */ + VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC = 11, + + /* + * The amount of failed hugetlb(Huge Page Tables) allocations via + * virtio balloon. + */ + VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL = 12, + /* * The number of statistics supported by this version of the interface. * To add new statistics, add them to the enum and increase this value. */ - VIR_DOMAIN_MEMORY_STAT_NR = 11, + VIR_DOMAIN_MEMORY_STAT_NR = 13, # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 80284a99f0..a95690e8a4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -5734,7 +5734,10 @@ virDomainGetInterfaceParameters(virDomainPtr domain, * VIR_DOMAIN_MEMORY_STAT_DISK_CACHES * Memory that can be reclaimed without additional I/O, typically disk * caches (in kb). - * + * VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC + * The amount of successful hugetlb(Huge Page Tables) allocations + * VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL + * The amount of failed hugetlb(Huge Page Tables) allocations * Returns: The number of stats provided or -1 in case of failure. */ int diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f48d9256e4..2d849beac8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20245,6 +20245,8 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver, STORE_MEM_RECORD(LAST_UPDATE, "last-update") STORE_MEM_RECORD(USABLE, "usable") STORE_MEM_RECORD(DISK_CACHES, "disk_caches") + STORE_MEM_RECORD(HUGETLB_PGALLOC, "hugetlb_pgalloc") + STORE_MEM_RECORD(HUGETLB_PGFAIL, "hugetlb_pgfail") } #undef STORE_MEM_RECORD diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 908967f46c..5e3df5e759 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2150,6 +2150,11 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 1); GET_BALLOON_STATS(statsdata, "stat-disk-caches", VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 1024); + GET_BALLOON_STATS(statsdata, "stat-htlb-pgalloc", + VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC, 1); + GET_BALLOON_STATS(statsdata, "stat-htlb-pgfail", + VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL, 1); + ret = got; cleanup: virJSONValueFree(cmd); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index d87475f6f6..0092a9786e 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -376,6 +376,10 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "last_update %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_DISK_CACHES) vshPrint(ctl, "disk_caches %llu\n", stats[i].val); + if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC) + vshPrint(ctl, "hugetlb_pgalloc %llu\n", stats[i].val); + if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL) + vshPrint(ctl, "hugetlb_pgfail %llu\n", stats[i].val); } ret = true; diff --git a/tools/virsh.pod b/tools/virsh.pod index afc1684db0..ef27c527d6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -938,6 +938,8 @@ without causing host swapping (in KiB) last-update - Timestamp of the last update of statistics (in seconds) disk_caches - The amount of memory that can be reclaimed without additional I/O, typically disk caches (in KiB) + hugetlb_pgalloc - The number of successful huge page table allocations + hugetlb_pgfail - The number of failed huge page table allocations For QEMU/KVM with a memory balloon, setting the optional I<--period> to a value larger than 0 in seconds will allow the balloon driver to return -- 2.20.1

2 3

[libvirt] [PATCH] lib: Avoid double free when passing FDs with virCommandPassFD()
by Michal Privoznik 17 May '19

17 May '19

If an FD is passed into a child using: virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); then the parent should refrain from touching @fd thereafter. This is even documented in virCommandPassFD() comment. The reason is that either at virCommandRun()/virCommandRunAsync() or virCommandFree() time the @fd will be closed. Closing it earlier, e.g. right after virCommandPassFD() call might result in undesired results. Another thread might open a file and receive the same FD which is then unexpectedly closed by virCommandFree() or virCommandRun(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_command.c | 10 ++++++---- src/util/virpolkit.c | 1 + tests/commandtest.c | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bf1fb539b1..92bd1524db 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8978,17 +8978,19 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, if (qemuSecuritySetTapFDLabel(driver->securityManager, def, tapfd[i]) < 0) goto cleanup; - virCommandPassFD(cmd, tapfd[i], - VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0) goto cleanup; + virCommandPassFD(cmd, tapfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + tapfd[i] = -1; } for (i = 0; i < vhostfdSize; i++) { - virCommandPassFD(cmd, vhostfd[i], - VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) goto cleanup; + virCommandPassFD(cmd, vhostfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + vhostfd[i] = -1; } if (chardev) diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index 25eaad2c63..634b46e82b 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -187,6 +187,7 @@ virPolkitAgentCreate(void) virCommandSetOutputFD(agent->cmd, &outfd); virCommandSetErrorFD(agent->cmd, &errfd); virCommandPassFD(agent->cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + pipe_fd[1] = -1; if (virCommandRunAsync(agent->cmd, NULL) < 0) goto error; diff --git a/tests/commandtest.c b/tests/commandtest.c index 816a70860f..146cc4c1bf 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1024,6 +1024,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) virCommandDaemonize(cmd); virCommandPassFD(cmd, newfd2, VIR_COMMAND_PASS_FD_CLOSE_PARENT); virCommandPassFD(cmd, newfd3, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + newfd2 = newfd3 = -1; virCommandPassListenFDs(cmd); if (virCommandRun(cmd, NULL) < 0) { @@ -1053,7 +1054,6 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) VIR_FREE(prefix); virCommandFree(cmd); VIR_FORCE_CLOSE(newfd1); - /* coverity[double_close] */ VIR_FORCE_CLOSE(newfd2); VIR_FORCE_CLOSE(newfd3); return ret; -- 2.21.0

2 1

[libvirt] [PATCH] qemu: Do not override config XML in case of snapshot revert
by Maxiwell S. Garcia 15 May '19

15 May '19

Snapshot create operation saves the live XML and uses it to replace the domain definition in case of revert. But the VM config XML is not saved and the revert operation does not address this issue. This commit prevents the config XML from being overridden by snapshot definition. An active domain stores both current and new definitions. The current definition (vm->def) stores the live XML and the new definition (vm->newDef) stores the config XML. In an inactive domain, only the config XML is persistent, and it's saved in vm->def. The revert operation uses the virDomainObjAssignDef() to set the snapshot definition in vm->newDef, if domain is active, or in vm->def otherwise. But before that, it saves the old value to return to caller. This return is used here to restore the config XML after all snapshot startup process finish. Signed-off-by: Maxiwell S. Garcia <maxiwell(a)linux.ibm.com> --- src/qemu/qemu_driver.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b2ac737d1f..a73122454a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16251,6 +16251,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuDomainObjPrivatePtr priv; int rc; virDomainDefPtr config = NULL; + virDomainDefPtr inactiveConfig = NULL; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; bool was_stopped = false; @@ -16465,7 +16466,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto endjob; } if (config) { - virDomainObjAssignDef(vm, config, false, NULL); + virDomainObjAssignDef(vm, config, false, &inactiveConfig); virCPUDefFree(priv->origCPU); VIR_STEAL_PTR(priv->origCPU, origCPU); } @@ -16474,7 +16475,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, load: was_stopped = true; if (config) - virDomainObjAssignDef(vm, config, false, NULL); + virDomainObjAssignDef(vm, config, false, &inactiveConfig); /* No cookie means libvirt which saved the domain was too old to * mess up the CPU definitions. @@ -16533,6 +16534,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, detail); } } + if (inactiveConfig) + VIR_STEAL_PTR(vm->newDef, inactiveConfig); + break; case VIR_DOMAIN_SNAPSHOT_SHUTDOWN: @@ -16560,8 +16564,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuProcessEndJob(driver, vm); goto cleanup; } - if (config) - virDomainObjAssignDef(vm, config, false, NULL); + if (config) { + virDomainObjAssignDef(vm, config, false, &inactiveConfig); + if (inactiveConfig) + VIR_STEAL_PTR(vm->newDef, inactiveConfig); + } if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) { -- 2.20.1

4 9

[libvirt] [PATCH 0/2] introduction of version attribute for VFIO live migration
by Yan Zhao 15 May '19

15 May '19

This patchset introduces a version attribute under sysfs of VFIO Mediated devices. This version attribute is used by user space software like libvirt to determine whether two mdev devices are compatible for live migration before starting live migration. Patch 1 defines version attribute as mandatory for VFIO live migration. It means if version attribute is missing or it returns errno, the corresponding mdev device is regarded as not supporting live migration. samples for vfio-mdev are modified to demonstrate it. Patch 2 uses GVT as an example to show how to expose version attribute and check device compatibility in vendor driver. Yan Zhao (2): vfio/mdev: add version field as mandatory attribute for mdev device drm/i915/gvt: export mdev device version to sysfs for Intel vGPU Documentation/vfio-mediated-device.txt | 36 +++++++++ drivers/gpu/drm/i915/gvt/Makefile | 2 +- drivers/gpu/drm/i915/gvt/device_version.c | 94 +++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/gvt.c | 55 +++++++++++++ drivers/gpu/drm/i915/gvt/gvt.h | 6 ++ samples/vfio-mdev/mbochs.c | 17 ++++ samples/vfio-mdev/mdpy.c | 16 ++++ samples/vfio-mdev/mtty.c | 16 ++++ 8 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c -- 2.17.1

8 33

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

15 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 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 00/12] qemu: Refactor image permission/lock setting (blockdev-add saga)
by Peter Krempa 09 May '19

09 May '19

With new blockjob handling we'll need to modify permissions for chains and individual images. The individual image code was universally accessible but the chain setting code reimplemented it mostly only in qemu_hotplug.h. Refactor the handling by moving the code to qemu_domain.c and making it universal. Peter Krempa (12): qemu: Rename qemuDomainDiskChainElement(Revoke|Prepare) qemu: Move and rename qemuHotplugPrepareDiskSourceAccess qemu: Split entry points to qemuDomainStorageSourceChainAccessPrepare qemu: domain: Rename qemuDomainStorageSourceChainAccessPrepare qemu: Convert boolean flags to enum flags in qemuDomainStorageSourceAccessModify qemu: Allow using qemuDomainStorageSourceAccessModify on singe images qemu: Refactor/simplify qemuDomainStorageSourceAccessRevoke qemu: Allow forcing read-only mode in qemuDomainStorageSourceAccessModify qemu: Use bools rather than labels in qemuDomainStorageSourceAccessModify qemu: Allow skipping the revoke step in qemuDomainStorageSourceAccessModify qemu: Mark when modifying access to existing source in qemuDomainStorageSourceAccessModify qemu: Refactor/simplify qemuDomainStorageSourceAccessAllow src/qemu/qemu_domain.c | 212 +++++++++++++++++++++++++++++++--------- src/qemu/qemu_domain.h | 23 +++-- src/qemu/qemu_driver.c | 24 ++--- src/qemu/qemu_hotplug.c | 80 ++------------- 4 files changed, 200 insertions(+), 139 deletions(-) -- 2.20.1

3 27

[libvirt] [PATCH v6] tests: perform cross compiler builds on GitLab CI
by Daniel P. Berrangé 09 May '19

09 May '19

GitLab CI provides some shared build runners that use Docker containers. This resource can usefully run cross-compiled builds since all other CI build testing is currently x86 only, and Travis CI is already very busy testing native builds. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- .gitlab-ci.yml | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 .gitlab-ci.yml Changed in v6: - Run jobs half & half on 9 vs sid diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml new file mode 100644 index 0000000000..a8a8581d9e --- /dev/null +++ b/.gitlab-ci.yml @@ -0,0 +1,46 @@ +.job_template: &job_definition + script: + - mkdir vpath + - cd vpath + - ../autogen.sh $CONFIGURE_OPTS + - make -j $(getconf _NPROCESSORS_ONLN) + +# We could run every arch on both versions, but it is a little +# overkill. Instead we run half the jobs on 9 and half the jobs +# on sid to give reasonable cross-coverage. + +debian-9-cross-armv6l: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-armv6l:master + +debian-9-cross-mipsel: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-mipsel:master + +debian-9-cross-ppc64le: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-ppc64le:master + +debian-9-cross-s390x: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-9-cross-s390x:master + +debian-sid-cross-aarch64: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-aarch64:master + +debian-sid-cross-armv7l: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-armv7l:master + +debian-sid-cross-i686: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-i686:master + +debian-sid-cross-mips64el: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-mips64el:master + +debian-sid-cross-mips: + <<: *job_definition + image: quay.io/libvirt/buildenv-debian-sid-cross-mips:master -- 2.20.1

2 2

[libvirt] [PATCH 00/10] build: drop some old libs and relax GNU strictness
by Ján Tomko 09 May '19

09 May '19

While testing these changes I found that the build is broken with old rbd on (unsupported) Debian 8. Fix the build by dropping support for old rbd. Ján Tomko (10): rbd: depend on diff_iterate2 rbd: drop support for ceph 0.94 build: drop check for SASL1 build: require yajl >= 2.0.3 json: assume WITH_YAJL2 build: remove WITH_YAJL2 virJSONValueToString: bail out early on error configure: split AM_INIT_AUTOMAKE into multiple lines configure.ac: drop -Wno-obsolete from AM_INIT_AUTOMAKE configure.ac: add foreign to AM_INIT_AUTOMAKE config-post.h | 1 - configure.ac | 12 ++++-- m4/virt-driver-qemu.m4 | 2 +- m4/virt-sasl.m4 | 5 +-- m4/virt-storage-rbd.m4 | 2 +- m4/virt-yajl.m4 | 5 +-- src/storage/storage_backend_rbd.c | 39 ------------------- src/util/virjson.c | 63 +++---------------------------- 8 files changed, 19 insertions(+), 110 deletions(-) -- 2.19.2

5 30