January 2022 - Devel - Libvirt List Archives

by Olaf Hering

style issues in docs/ and libvirt.spec.in (abologna) rebased to 4b77b194069f048e6efdaf5d363098ae039dc4f5 Olaf Hering (2): remove sysconfig files NEWS: mention removal of sysconfig NEWS.rst | 10 ++++ docs/daemons.rst | 21 ++++++++ docs/remote.html.in | 6 +-- libvirt.spec.in | 69 +++++++++++++++++++------ src/ch/meson.build | 5 -- src/ch/virtchd.service.in | 1 + src/ch/virtchd.sysconf | 3 -- src/interface/meson.build | 5 -- src/interface/virtinterfaced.service.in | 1 + src/interface/virtinterfaced.sysconf | 3 -- src/libxl/meson.build | 5 -- src/libxl/virtxend.service.in | 1 + src/libxl/virtxend.sysconf | 3 -- src/locking/meson.build | 5 -- src/locking/virtlockd.service.in | 1 + src/locking/virtlockd.sysconf | 3 -- src/logging/meson.build | 5 -- src/logging/virtlogd.sysconf | 3 -- src/lxc/meson.build | 5 -- src/lxc/virtlxcd.service.in | 1 + src/lxc/virtlxcd.sysconf | 3 -- src/meson.build | 16 ------ src/network/meson.build | 5 -- src/network/virtnetworkd.service.in | 1 + src/network/virtnetworkd.sysconf | 3 -- src/node_device/meson.build | 5 -- src/node_device/virtnodedevd.service.in | 1 + src/node_device/virtnodedevd.sysconf | 3 -- src/nwfilter/meson.build | 5 -- src/nwfilter/virtnwfilterd.service.in | 1 + src/nwfilter/virtnwfilterd.sysconf | 3 -- src/qemu/meson.build | 5 -- src/qemu/virtqemud.service.in | 7 +++ src/qemu/virtqemud.sysconf | 12 ----- src/remote/libvirtd.service.in | 7 +++ src/remote/libvirtd.sysconf | 21 -------- src/remote/meson.build | 10 ---- src/remote/virtproxyd.service.in | 1 + src/remote/virtproxyd.sysconf | 3 -- src/secret/meson.build | 5 -- src/secret/virtsecretd.service.in | 1 + src/secret/virtsecretd.sysconf | 3 -- src/storage/meson.build | 5 -- src/storage/virtstoraged.service.in | 1 + src/storage/virtstoraged.sysconf | 3 -- src/vbox/meson.build | 5 -- src/vbox/virtvboxd.service.in | 1 + src/vbox/virtvboxd.sysconf | 3 -- src/vz/meson.build | 5 -- src/vz/virtvzd.service.in | 1 + src/vz/virtvzd.sysconf | 3 -- tools/libvirt-guests.sh.in | 40 ++++++++++++++ tools/libvirt-guests.sysconf | 50 ------------------ tools/meson.build | 6 --- 54 files changed, 155 insertions(+), 245 deletions(-) delete mode 100644 src/ch/virtchd.sysconf delete mode 100644 src/interface/virtinterfaced.sysconf delete mode 100644 src/libxl/virtxend.sysconf delete mode 100644 src/locking/virtlockd.sysconf delete mode 100644 src/logging/virtlogd.sysconf delete mode 100644 src/lxc/virtlxcd.sysconf delete mode 100644 src/network/virtnetworkd.sysconf delete mode 100644 src/node_device/virtnodedevd.sysconf delete mode 100644 src/nwfilter/virtnwfilterd.sysconf delete mode 100644 src/qemu/virtqemud.sysconf delete mode 100644 src/remote/libvirtd.sysconf delete mode 100644 src/remote/virtproxyd.sysconf delete mode 100644 src/secret/virtsecretd.sysconf delete mode 100644 src/storage/virtstoraged.sysconf delete mode 100644 src/vbox/virtvboxd.sysconf delete mode 100644 src/vz/virtvzd.sysconf delete mode 100644 tools/libvirt-guests.sysconf

2 years, 11 months

3
8
0 / 0

[PATCH] do not report generic OPERATION_FAILED error when calling virConnectOpenAuth()

by Ani Sinha

virConnectOpenAuth() calls virConnectOpenInternal(). This later function generates fine grained errors arising from various failure conditions that are more accurate than a "catch all" broader VIR_ERR_OPERATION_FAILED error that the callers of this function generates. Remove the broader error so that more specific errors can be caught and processed. Signed-off-by: Ani Sinha <ani(a)anisinha.ca> --- src/libxl/libxl_migration.c | 3 --- src/qemu/qemu_migration.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 6d0ab4ee28..bc2b5401da 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -1134,9 +1134,6 @@ libxlDomainMigrationSrcPerformP2P(libxlDriverPrivate *driver, virObjectLock(vm); if (dconn == NULL) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Failed to connect to remote libvirt URI %s: %s"), - dconnuri, virGetLastErrorMessage()); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b9d7d582f5..2635ef1162 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5145,9 +5145,6 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, goto cleanup; if (dconn == NULL) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Failed to connect to remote libvirt URI %s: %s"), - dconnuri, virGetLastErrorMessage()); return -1; } -- 2.25.1

2 years, 11 months

2
2
0 / 0

[PATCH] src: Don't check for retval of g_strsplit()

by Michal Privoznik

The g_strsplit() function can return NULL if and only if either the input string is NULL or delimiter is NULL or an empty string. In neither of places we call it any of the conditions is true and thus we don't need to check for the return value. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/libxl/xen_xl.c | 2 -- src/util/vircgroupv2.c | 2 -- src/util/virprocess.c | 2 -- src/util/virresctrl.c | 2 -- 4 files changed, 8 deletions(-) diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 7604e3d534..869083a1d1 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -265,8 +265,6 @@ xenParseXLCPUID(virConf *conf, virDomainDef *def) } cpuid_pairs = g_strsplit(cpuid_str, ",", 0); - if (!cpuid_pairs) - return -1; if (!cpuid_pairs[0]) return 0; diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 4c110940cf..f00a8f154b 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -286,8 +286,6 @@ virCgroupV2ParseControllersFile(virCgroup *group, virTrimSpaces(contStr, NULL); contList = g_strsplit(contStr, " ", 20); - if (!contList) - return -1; tmp = contList; diff --git a/src/util/virprocess.c b/src/util/virprocess.c index b559a4257e..06767dbf51 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1837,8 +1837,6 @@ virProcessGetSchedInfo(unsigned long long *cpuWait, return -1; lines = g_strsplit(data, "\n", 0); - if (!lines) - return -1; for (i = 0; lines[i] != NULL; i++) { const char *line = lines[i]; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index edbf078654..fe45ad3c64 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1646,8 +1646,6 @@ virResctrlAllocParseCacheLine(virResctrlInfo *resctrl, } caches = g_strsplit(tmp, ";", 0); - if (!caches) - return 0; for (next = caches; *next; next++) { if (virResctrlAllocParseProcessCache(resctrl, alloc, level, type, *next) < 0) -- 2.34.1

2 years, 11 months

2
1
0 / 0

[libvirt PATCH 0/2] docs: Replace node.gif with node.png

by Andrea Bolognani

Andrea Bolognani (2): docs: Update node.svg docs: Replace node.gif with node.png docs/goals.html.in | 2 +- docs/images/meson.build | 2 +- docs/images/node.gif | Bin 1397 -> 0 bytes docs/images/node.png | Bin 0 -> 5532 bytes docs/images/node.svg | 174 +++++++++++++++++++++++++++++++++------- 5 files changed, 145 insertions(+), 33 deletions(-) delete mode 100644 docs/images/node.gif create mode 100644 docs/images/node.png -- 2.31.1

2 years, 11 months

2
4
0 / 0

[libvirt PATCH v3 0/4] docs: Unclutter top-level directory

by Andrea Bolognani

Changes from [v2]: * drop all changes related to icons. Changes from [v1]: * extend scope to include icons and CSS files. [v2] https://listman.redhat.com/archives/libvir-list/2021-December/msg00828.html [v1] https://listman.redhat.com/archives/libvir-list/2021-December/msg00818.html Andrea Bolognani (4): docs: Drop structures.svg docs: Move all images to a subdirectory docs: Move all CSS files to a subdirectory docs: Move font definitions with other CSS files docs/api.rst | 6 +- docs/{fonts/stylesheet.css => css/fonts.css} | 18 +- docs/{ => css}/generic.css | 0 docs/{ => css}/libvirt.css | 4 +- docs/{ => css}/main.css | 2 +- docs/css/meson.build | 17 ++ docs/{ => css}/mobile.css | 2 +- docs/fonts/meson.build | 1 - docs/goals.html.in | 2 +- docs/{ => images}/event_loop_simple.png | Bin docs/{ => images}/event_loop_simple.svg | 0 docs/{ => images}/event_loop_worker.png | Bin docs/{ => images}/event_loop_worker.svg | 0 docs/{ => images}/libvirt-daemon-arch.png | Bin docs/{ => images}/libvirt-daemon-arch.svg | 0 docs/{ => images}/libvirt-driver-arch.png | Bin docs/{ => images}/libvirt-driver-arch.svg | 0 docs/{ => images}/libvirt-object-model.png | Bin docs/{ => images}/libvirt-object-model.svg | 0 .../libvirt-virConnect-example.png | Bin .../libvirt-virConnect-example.svg | 0 docs/images/meson.build | 24 +++ .../{ => images}/migration-managed-direct.png | Bin .../{ => images}/migration-managed-direct.svg | 0 docs/{ => images}/migration-managed-p2p.png | Bin docs/{ => images}/migration-managed-p2p.svg | 0 docs/{ => images}/migration-native.png | Bin docs/{ => images}/migration-native.svg | 0 docs/{ => images}/migration-tunnel.png | Bin docs/{ => images}/migration-tunnel.svg | 0 .../migration-unmanaged-direct.png | Bin .../migration-unmanaged-direct.svg | 0 docs/{ => images}/node.gif | Bin docs/{ => images}/node.svg | 0 docs/internals.html.in | 2 +- docs/internals/eventloop.html.in | 4 +- docs/meson.build | 18 +- docs/migration.rst | 10 +- docs/page.xsl | 2 +- docs/structures.svg | 187 ------------------ 40 files changed, 69 insertions(+), 230 deletions(-) rename docs/{fonts/stylesheet.css => css/fonts.css} (60%) rename docs/{ => css}/generic.css (100%) rename docs/{ => css}/libvirt.css (98%) rename docs/{ => css}/main.css (68%) create mode 100644 docs/css/meson.build rename docs/{ => css}/mobile.css (95%) rename docs/{ => images}/event_loop_simple.png (100%) rename docs/{ => images}/event_loop_simple.svg (100%) rename docs/{ => images}/event_loop_worker.png (100%) rename docs/{ => images}/event_loop_worker.svg (100%) rename docs/{ => images}/libvirt-daemon-arch.png (100%) rename docs/{ => images}/libvirt-daemon-arch.svg (100%) rename docs/{ => images}/libvirt-driver-arch.png (100%) rename docs/{ => images}/libvirt-driver-arch.svg (100%) rename docs/{ => images}/libvirt-object-model.png (100%) rename docs/{ => images}/libvirt-object-model.svg (100%) rename docs/{ => images}/libvirt-virConnect-example.png (100%) rename docs/{ => images}/libvirt-virConnect-example.svg (100%) create mode 100644 docs/images/meson.build rename docs/{ => images}/migration-managed-direct.png (100%) rename docs/{ => images}/migration-managed-direct.svg (100%) rename docs/{ => images}/migration-managed-p2p.png (100%) rename docs/{ => images}/migration-managed-p2p.svg (100%) rename docs/{ => images}/migration-native.png (100%) rename docs/{ => images}/migration-native.svg (100%) rename docs/{ => images}/migration-tunnel.png (100%) rename docs/{ => images}/migration-tunnel.svg (100%) rename docs/{ => images}/migration-unmanaged-direct.png (100%) rename docs/{ => images}/migration-unmanaged-direct.svg (100%) rename docs/{ => images}/node.gif (100%) rename docs/{ => images}/node.svg (100%) delete mode 100644 docs/structures.svg -- 2.31.1

2 years, 11 months

2
11
0 / 0

[PATCH] virprocess: Provide non-Linux stubs for virProcessGet{Stat, Sched}Info

by Michal Privoznik

Both virProcessGetStatInfo() and virProcessGetSchedInfo() are Linux centric. Provide stubs for non-Linux platforms. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virprocess.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index c74bd16fe6..5788faea9c 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1766,6 +1766,7 @@ virProcessGetStat(pid_t pid, } +#ifdef __linux__ int virProcessGetStatInfo(unsigned long long *cpuTime, int *lastCpu, @@ -1873,3 +1874,26 @@ virProcessGetSchedInfo(unsigned long long *cpuWait, return 0; } + +#else +int +virProcessGetStatInfo(unsigned long long *cpuTime G_GNUC_UNUSED, + int *lastCpu G_GNUC_UNUSED, + long *vm_rss G_GNUC_UNUSED, + pid_t pid G_GNUC_UNUSED, + pid_t tid G_GNUC_UNUSED) +{ + errno = ENOSYS; + return -1; +} + +int +virProcessGetSchedInfo(unsigned long long *cpuWait G_GNUC_UNUSED, + pid_t pid G_GNUC_UNUSED, + pid_t tid G_GNUC_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("scheduler information is not supported on this platform")); + return -1; +} +#endif /* __linux__ */ -- 2.34.1

2 years, 11 months

6
16
0 / 0

[PATCH] report error when virProcessGetStatInfo() is unable to parse data

by Ani Sinha

Currently virProcessGetStatInfo() always returns success and only logs error when it is unable to parse the data. Make this function actually report the error and return a negative value in this error scenario. Signed-off-by: Ani Sinha <ani(a)anisinha.ca> --- src/util/virprocess.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index c74bd16fe6..b9f498d5d8 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1783,7 +1783,11 @@ virProcessGetStatInfo(unsigned long long *cpuTime, virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 0 || virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 || virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) { - VIR_WARN("cannot parse process status data"); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse process status data for pid '%d/%d'"), + (int) pid, (int) tid); + + return -1; } /* We got jiffies -- 2.25.1

2 years, 11 months

2
3
0 / 0

[PATCH v2] report error when virProcessGetStatInfo() is unable to parse data

by Ani Sinha

Currently virProcessGetStatInfo() always returns success and only logs error when it is unable to parse the data. Make this function actually report the error and return a negative value in this error scenario. Fix the callers so that they do not override the error generated. Signed-off-by: Ani Sinha <ani(a)anisinha.ca> --- src/ch/ch_driver.c | 2 -- src/qemu/qemu_driver.c | 7 +------ src/util/virprocess.c | 6 +++++- 3 files changed, 6 insertions(+), 9 deletions(-) changelog: v2: fixed the callers diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 53e0872207..3cbc668489 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -1073,8 +1073,6 @@ chDomainHelperGetVcpus(virDomainObj *vm, if (virProcessGetStatInfo(&vcpuinfo->cpuTime, &vcpuinfo->cpu, NULL, vm->pid, vcpupid) < 0) { - virReportSystemError(errno, "%s", - _("cannot get vCPU placement & pCPU time")); return -1; } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4974450333..015ffb2ce7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1359,8 +1359,6 @@ qemuDomainHelperGetVcpus(virDomainObj *vm, if (virProcessGetStatInfo(&vcpuinfo->cpuTime, &vcpuinfo->cpu, NULL, vm->pid, vcpupid) < 0) { - virReportSystemError(errno, "%s", - _("cannot get vCPU placement & pCPU time")); return -1; } } @@ -2521,8 +2519,6 @@ qemuDomainGetInfo(virDomainPtr dom, if (virDomainObjIsActive(vm)) { if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("cannot read cputime for domain")); goto cleanup; } } @@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver, } if (virProcessGetStatInfo(NULL, NULL, &rss, vm->pid, 0) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("cannot get RSS for domain")); + return -1; } else { stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; stats[ret].val = rss; diff --git a/src/util/virprocess.c b/src/util/virprocess.c index c74bd16fe6..b9f498d5d8 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1783,7 +1783,11 @@ virProcessGetStatInfo(unsigned long long *cpuTime, virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 0 || virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 || virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) { - VIR_WARN("cannot parse process status data"); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse process status data for pid '%d/%d'"), + (int) pid, (int) tid); + + return -1; } /* We got jiffies -- 2.25.1

2 years, 11 months

1
0
0 / 0

[libvirt PATCH v4 00/19] qemu: Introduce hvf domain type for Hypervisor.framework

by Andrea Bolognani

In order to hopefully address [libvirt#147] at long last, I've picked up Roman's patches from 2018 and attempted to forward-port them. More specifically, I've used the [roolebo/hvf-domain] branch as a starting point, since it seems to contain a few improvements over [v2] and was just easier to pick up. The code is mostly his own, so I've retained the existing authorship information, but I've dropped Reviewed-by tags for commits that have been modified in non-trivial ways. I've applied very minimal style tweaks along the way, but overall I've tried to modify the existing patches as little as possible. I've added a few changes of my own, which I've marked as "fixup!" when I felt that they should be squashed into the previous patch rather than existing as separate commits. The new test cases, such as they are, pass, and no regressions to KVM support appear to have been introduced in the process. I don't currently have access to a machine running macOS, so I can't verify that it's actually possible to start a hardware-accelerated VM. Changes from [v3]: * reintroduced the patch that was missing in the initial version of the forward-port; * converted the documentation to reStructuredText and trimmed it significantly; * reworked virQEMUCapsAccelStr() based on Dan's suggestions; * reworked macOS support in the test suite based on Dan's suggestions; * fixed a few minor issues found while doing the above. Changes from [v2]: * rebased on top of master; * added a couple of simple test cases. Useful links: * GitLab: [abologna/hvf] * CI: [pipeline] [libvirt#147] https://gitlab.com/libvirt/libvirt/-/issues/147 [roolebo/hvf-domain] https://github.com/roolebo/libvirt/tree/hvf-domain [abologna/hvf] https://gitlab.com/abologna/libvirt/-/commits/hvf [pipeline] https://gitlab.com/abologna/libvirt/-/pipelines/443320533 [v3] https://listman.redhat.com/archives/libvir-list/2022-January/msg00131.html [v2] https://listman.redhat.com/archives/libvir-list/2018-November/msg00802.html Andrea Bolognani (6): qemu: Only probe KVM on Linux tests: Introduce testQemuHostOS tests: Add macOS support to testutilsqemu tests: Add macOS support to qemuxml2*test tests: Add HVF test cases fixup! NEWS: Mention Apple Silicon support for HVF Roman Bolshakov (13): qemu: Add KVM CPUs into cache only if KVM is present conf: Add hvf domain type qemu: Define hvf capability qemu: Query hvf capability on macOS qemu: Expose hvf domain type if hvf is supported qemu: Introduce virQEMUCapsAccelStr qemu: Introduce virQEMUCapsTypeIsAccelerated qemu: Introduce virQEMUCapsHaveAccel qemu: Correct CPU capabilities probing for hvf docs: Add hvf on QEMU driver page docs: Note hvf support for domain elements docs: Add support page for libvirt on macOS news: Mention hvf domain type NEWS.rst | 6 + docs/docs.html.in | 3 + docs/drvqemu.rst | 48 +++++- docs/formatdomain.rst | 22 +-- docs/index.html.in | 4 +- docs/macos.rst | 44 ++++++ docs/meson.build | 1 + docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 135 ++++++++++++++-- src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 4 + src/qemu/qemu_process.c | 10 +- .../hvf-aarch64-virt-headless.args | 48 ++++++ .../hvf-aarch64-virt-headless.xml | 45 ++++++ .../hvf-x86_64-q35-headless.args | 47 ++++++ .../hvf-x86_64-q35-headless.x86_64-latest.err | 1 + .../hvf-x86_64-q35-headless.xml | 44 ++++++ tests/qemuxml2argvtest.c | 43 ++++- .../hvf-aarch64-virt-headless.xml | 94 +++++++++++ .../hvf-x86_64-q35-headless.xml | 97 ++++++++++++ tests/qemuxml2xmltest.c | 43 ++++- tests/testutilsqemu.c | 147 ++++++++++++++---- tests/testutilsqemu.h | 10 ++ 25 files changed, 837 insertions(+), 65 deletions(-) create mode 100644 docs/macos.rst create mode 100644 tests/qemuxml2argvdata/hvf-aarch64-virt-headless.args create mode 100644 tests/qemuxml2argvdata/hvf-aarch64-virt-headless.xml create mode 100644 tests/qemuxml2argvdata/hvf-x86_64-q35-headless.args create mode 100644 tests/qemuxml2argvdata/hvf-x86_64-q35-headless.x86_64-latest.err create mode 100644 tests/qemuxml2argvdata/hvf-x86_64-q35-headless.xml create mode 100644 tests/qemuxml2xmloutdata/hvf-aarch64-virt-headless.xml create mode 100644 tests/qemuxml2xmloutdata/hvf-x86_64-q35-headless.xml -- 2.31.1

2 years, 11 months

1
19
0 / 0

.conf file setting(s) for packet filtering backend(s)

by Laine Stump

(this probably will make no sense to anyone who hasn't spent time looking at daemonConfig*, in which case you can go ahead and hit Delete now. At any rate I'm just tossing this out into the void to see if anyone has any ideas/opinions, so in *any* case feel free to hit delete!) Happy New Year! and time for another bit of confused ramblings trying to figure out how to do something that ends up being non-confused and straightforward. I'm currently working on switching the backend of the network driver from using iptables to using nftables. Due to some functionality that is not available with nftables (the rule that fixes up the checksum of DHCP packets which, btw, is only relevant for *very* old guests, e.g. RHEL5), this needs to be opt-in via a config file setting. In the meantime, in order to make this doable in a reasonable amount of time, I am *not* converting the nwfilter driver right away, and when I do it will need its own config file setting for opt-in. I've never before looked at the code for the .conf file settings at all. I had assumed there would be some sort of "pull" API, where code in the drivers could call, e.g. virConfGetString("filter_backend") and it would return the config setting to the caller. But when I look at it, I see that all daemons use the same daemonConfigLoadFile() called from remote_daemon.c:main() (which is shared by all the daemons) and the daemonConfig object that is created to hold the config settings that are read is only visible within main() - the only way that a config setting is used is by main() "pushing" it out to a static variable somewhere else where it is later retrieved by the interested party, e.g. the way that main() calls daemonSetupNetDevOpenvswitch(config), which then sets the static virNetDevOpenvswitchTimeout in util/virnetdevopenvswitch.c. (NB: util/virnetdevopenvswitch.c is linked into every deamon, so even for the daemons that don't use it, calls to virnetdevopenvswitch.c functions still compile properly (and calling them is harmless), so virNetDevOpenvswitchTimeout is set even for daemons that never call openvswitch APIs). If I could count on all builds using split daemons (i.e. separate virtnetworkd and virtnwfilterd) then I could add a similar API in virfirewall.c that remote_daemon.c:main() could use to set "filter_backend" into a static in virfirewall.c (which is used by both drivers) and everything would just happily work: virtnetworkd.conf: filter_backend = nftables virtnwfilterd.conf filter_backend = iptables However, I need to also deal with the possibility that the nwfilter and network drivers are in the same unified libvirtd binary, and in that case both drivers would have the same virfirewall.c:filter_backend setting, thus making it impossible to use the iptables backend for the nwfilter driver and nftables backend for the network driver. For that case I would need separate settings in the config for each driver, e.g. libvirtd.conf: network_filter_backend = nftables nwfilter_backend = iptables and then those would need to be stored off somewhere different for each driver, then they would use it to set the backend for each virFirewall object as it is created. Organizationally, it would make the most sense for these settings (and the API to set them) to be located in the drivers that use them (so, for example, network_filter_backend could live in network/bridge_driver_linux.c and nwfilter_backend could live in nwfilter/nwfilter_driver.c). But that would mean that remote_daemon.c:main() would need to directly call functions in those files, which is a no-no (because, in the case of split daemons, you either have one or the other at build time, but never both). So should I perhaps declare the nftables backend for nwfilter to be a lost cause until everyone moves to split daemons, add a "filter_backend" setting that is directly set in virfirewall.c (by remote_daemon.c:main()), and then provide some sort of override in virFirewallNew so calls from the nwfilter driver can say "ignore the filter_backend setting and use iptables"? Or should we make the virConf APIs beefier, and add facilities to save off the entire daemonConfig object and make its contents available via something like virConfGetString("network_filter_backend")? But if I did that, it would mean two differently-named config entries, and it would certainly be nice if I didn't have to introduce daemon-specific names like this that would need to be carried over from libvirtd.conf into virtnwfilterd.conf and virtnetworkd.conf (where differing names would no longer be required). I suppose I could go "full MS" and introduce the concept of sections to the conf file, so libvirtd.conf could have something like this: [network] filter_backend = nftables [nwfilter] filter_backend = iptables but that seems like a lot of work for something that will be obsolete in a year or so anyway (when the unified daemon is a thing of the past). Am I thinking about details too much and missing an obvious simple solution? (I think until/unless I or someone else comes up with a better idea, I'm going to go with a single filter_backend setting set in virfirewall.c that is somehow overridden when the virFirewall calls come from the nwfilter driver. Then if everyone moves to split daemons before I get around to making the nwfilter driver support nftables, the problem will end up just solving itself; otherwise I can think of some other piece of ugliness to make it work).

2 years, 11 months

2
3
0 / 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Devel January 2022