- Devel - libvirt List Archives

[PATCH] virt-host-validate: Rework calling of driver validation
by Michal Privoznik 11 Jul '24

11 Jul '24

It all started with me looking at the --help output which also printed "bhyve" as supported hypervisor type. Well, it's not on my Linux machine. To resolve this, I'm just creating a static array of { "$driver", callback() } pairs and iterating over it. The array is then initialized at compile time with supported drivers. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate.c | 80 +++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/tools/virt-host-validate.c b/tools/virt-host-validate.c index 426648a5d3..365b8acd92 100644 --- a/tools/virt-host-validate.c +++ b/tools/virt-host-validate.c @@ -29,6 +29,7 @@ #include "internal.h" #include "virerror.h" #include "virgettext.h" +#include "virglibutil.h" #include "virt-host-validate-common.h" #if WITH_QEMU @@ -44,26 +45,58 @@ # include "virt-host-validate-ch.h" #endif +typedef struct _virValidateCallbacks virValidateCallbacks; +struct _virValidateCallbacks { + const char *name; + int (*callback)(void); +}; + +static virValidateCallbacks validateCallbacks[] = { +#if WITH_QEMU + { "qemu", virHostValidateQEMU }, +#endif +#if WITH_LXC + { "lxc", virHostValidateLXC }, +#endif +#if WITH_BHYVE + { "bhyve", virHostValidateBhyve }, +#endif +#if WITH_CH + { "ch", virHostValidateCh }, +#endif +}; + static void show_help(FILE *out, const char *argv0) { + g_autofree char *hvs = NULL; + char *hvs_list[G_N_ELEMENTS(validateCallbacks) + 1] = { }; + size_t i; + + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + hvs_list[i] = g_strdup_printf(" - %1$s", validateCallbacks[i].name); + } + + hvs = g_strjoinv("\n", hvs_list); + + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + g_free(hvs_list[i]); + } + fprintf(out, _("\n" "syntax: %1$s [OPTIONS] [HVTYPE]\n" "\n" " Hypervisor types:\n" "\n" - " - qemu\n" - " - lxc\n" - " - bhyve\n" - " - ch\n" + "%2$s\n" "\n" " Options:\n" " -h, --help Display command line help\n" " -v, --version Display command version\n" " -q, --quiet Don't display progress information\n" "\n"), - argv0); + argv0, hvs); } static void @@ -87,6 +120,7 @@ main(int argc, char **argv) int ret = EXIT_SUCCESS; bool quiet = false; bool usedHvname = false; + size_t i; if (virGettextInitialize() < 0 || virErrorInitialize() < 0) { @@ -126,37 +160,13 @@ main(int argc, char **argv) virValidateSetQuiet(quiet); -#if WITH_QEMU - if (!hvname || STREQ(hvname, "qemu")) { - usedHvname = true; - if (virHostValidateQEMU() < 0) - ret = EXIT_FAILURE; + for (i = 0; i < G_N_ELEMENTS(validateCallbacks); i++) { + if (!hvname || STREQ(hvname, validateCallbacks[i].name)) { + usedHvname = true; + if (validateCallbacks[i].callback() < 0) + ret = EXIT_FAILURE; + } } -#endif - -#if WITH_LXC - if (!hvname || STREQ(hvname, "lxc")) { - usedHvname = true; - if (virHostValidateLXC() < 0) - ret = EXIT_FAILURE; - } -#endif - -#if WITH_BHYVE - if (!hvname || STREQ(hvname, "bhyve")) { - usedHvname = true; - if (virHostValidateBhyve() < 0) - ret = EXIT_FAILURE; - } -#endif - -#if WITH_CH - if (!hvname || STREQ(hvname, "ch")) { - usedHvname = true; - if (virHostValidateCh() < 0) - ret = EXIT_FAILURE; - } -#endif if (hvname && !usedHvname) { fprintf(stderr, _("%1$s: unsupported hypervisor name %2$s\n"), -- 2.44.2

2 1

[PATCH] virt-host-validate: Drop extra "PASS"
by Michal Privoznik 11 Jul '24

11 Jul '24

If virt-host-validate is ran on a SEV-SNP capable machine, an extra "PASS" is printed out. This is because virHostValidateAMDSev() prints "PASS" and then returns 1 (indicating success) which in turn makes the caller (virHostValidateSecureGuests()) print "PASS" again. Just drop the extra printing in the caller and let virHostValidateAMDSev() do all the printing. Fixes: 1a8f646f291775d2423ce4e4df62ad69f06ab827 Resolves: https://issues.redhat.com/browse/RHEL-46868 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate-common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index a29a5b6d5f..86db4815c3 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -488,11 +488,7 @@ int virHostValidateSecureGuests(const char *hvname, return VIR_VALIDATE_FAILURE(level); } } else if (hasAMDSev) { - int rc = virHostValidateAMDSev(hvname, level); - - if (rc > 0) - virValidatePass(); - return rc; + return virHostValidateAMDSev(hvname, level); } virValidateFail(level, -- 2.44.2

2 1

[PATCH v1 0/1] Add support for Vfio Stats in Libvirt
by Kshitij Jha 10 Jul '24

10 Jul '24

Hi, This is a solution to add support for data transferred by Vfio devices during the Live Migration in Libvirt. Currently libvirt only supports a few essential stats like memory transferred, dirty rate and number of iteration etc. returned as part of virDomainGetJobStats for each iteration. However it does not return stats for Vfio devices like data transferred by the device during the migration With this change virDomainGetJobStats will be able to support Vfio stats. Regards Kshitij Jha Kshitij Jha (1): Include support for Vfio stats during Migration include/libvirt/libvirt-domain.h | 9 +++++++++ src/qemu/qemu_domainjob.c | 6 ++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 12 ++++++++++++ 4 files changed, 28 insertions(+) -- 2.22.3

2 2

[PATCH] network: allow "modify" option for DNS-Txt records
by Adam Julis 10 Jul '24

10 Jul '24

The "modify" command allows to replace an existing record (its text value). The primary key is the name of the record. If duplicity or missing record detected, throw error. Tests in networkxml2xmlupdatetest.c contain replacements of an existing DNS-text record and failure due to non-existing record. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/639 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/network_conf.c | 18 ++++++++----- .../dns-txt-record-modify-fail.xml | 1 + .../dns-txt-record-modify-success.xml | 1 + .../nat-network-dns-txt-modify-ok.xml | 26 +++++++++++++++++++ tests/networkxml2xmlupdatetest.c | 9 +++++++ 5 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index fc387f9566..dd362b6ab2 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3351,12 +3351,6 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def, bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); - if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("DNS TXT records cannot be modified, only added or deleted")); - goto cleanup; - } - if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0) goto cleanup; @@ -3395,6 +3389,18 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def, virNetworkDNSTxtDefClear(&dns->txts[foundIdx]); VIR_DELETE_ELEMENT(dns->txts, foundIdx, dns->ntxts); + } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + + if (foundIdx == dns->ntxts) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS TXT record in network %1$s"), + def->name); + goto cleanup; + } + + VIR_FREE(dns->txts[foundIdx].value); + dns->txts[foundIdx].value = g_strdup(txt.value); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; diff --git a/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml b/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml new file mode 100644 index 0000000000..75ed475fe1 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml @@ -0,0 +1 @@ +<txt name='notexisted' value='modified example'/> diff --git a/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml b/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml new file mode 100644 index 0000000000..e16c352253 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml @@ -0,0 +1 @@ +<txt name='example' value='modified example'/> diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml b/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml new file mode 100644 index 0000000000..4b4dda094a --- /dev/null +++ b/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml @@ -0,0 +1,26 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <dns> + <txt name='example' value='modified example'/> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/> + <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/> + </dhcp> + </ip> + <ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'> + </ip> + <ip family='ipv4' address='10.24.10.1'> + </ip> +</network> diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index 59e6ce98e5..875cede035 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -306,6 +306,15 @@ mymain(void) "dns-txt-record-snowman", "nat-network-dns-txt-record", VIR_NETWORK_UPDATE_COMMAND_DELETE); + DO_TEST("modify-dns-txt-record", + "dns-txt-record-modify-success", + "nat-network-dns-txt-record", + "nat-network-dns-txt-modify-ok", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); + DO_TEST_FAIL("modify-missing-dns-txt-record", + "dns-txt-record-modify-fail", + "nat-network-dns-txt-record", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); section = VIR_NETWORK_SECTION_DNS_SRV; -- 2.45.2

2 1

[PATCH] network: allow "modify" option for DNS hostname
by Adam Julis 10 Jul '24

10 Jul '24

The "modify" command allows you to replace an existing record (its hostname, sub-elements). IP address acts as the primary key. If it is not found, the attempt ends with an error message. If the XML contains a duplicate address, it will select the last one. Tests in networkxml2xmlupdatetest.c contain replacements of an existing DNS-Host record and failure due to non-existing record. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/639 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/network_conf.c | 28 ++++++++++++++----- .../dns-host-modify-not-existing.xml | 4 +++ .../dns-host-modify.xml | 5 ++++ .../nat-network-dns-hosts-modified.xml | 28 +++++++++++++++++++ tests/networkxml2xmlupdatetest.c | 9 ++++++ 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml create mode 100644 tests/networkxml2xmlupdatein/dns-host-modify.xml create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f5ccf4bd12..2a541cd5b0 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3138,19 +3138,13 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, unsigned int fflags G_GNUC_UNUSED) { size_t i, j, k; - int foundIdx = -1, ret = -1; + int foundIdx = -1, ret = -1, foundIdxModify = -1; virNetworkDNSDef *dns = &def->dns; virNetworkDNSHostDef host = { 0 }; bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); int foundCt = 0; - if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("DNS HOST records cannot be modified, only added or deleted")); - goto cleanup; - } - if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0) goto cleanup; @@ -3163,6 +3157,12 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, if (virSocketAddrEqual(&host.ip, &dns->hosts[i].ip)) foundThisTime = true; + /* modify option required index of matching ip-address, the loop under + * this comment could affect results of found index foundThisTime, + * so the foundIdxModify is there used instead */ + if (foundThisTime) + foundIdxModify = i; + /* when adding we want to only check duplicates of address since having * multiple addresses with the same hostname is a legitimate configuration */ if (!isAdd) { @@ -3213,6 +3213,20 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, virNetworkDNSHostDefClear(&dns->hosts[foundIdx]); VIR_DELETE_ELEMENT(dns->hosts, foundIdx, dns->nhosts); + } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + + if (foundCt == 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS HOST record in network %1$s"), + def->name); + goto cleanup; + } + + virNetworkDNSHostDefClear(&dns->hosts[foundIdxModify]); + + memcpy(&dns->hosts[foundIdxModify], &host, sizeof(virNetworkDNSHostDef)); + memset(&host, 0, sizeof(virNetworkDNSHostDef)); + } else { virNetworkDefUpdateUnknownCommand(command); goto cleanup; diff --git a/tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml b/tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml new file mode 100644 index 0000000000..357fccd110 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-host-modify-not-existing.xml @@ -0,0 +1,4 @@ +<host ip='192.168.122.333'> + <hostname>shared</hostname> + <hostname>names</hostname> +</host> diff --git a/tests/networkxml2xmlupdatein/dns-host-modify.xml b/tests/networkxml2xmlupdatein/dns-host-modify.xml new file mode 100644 index 0000000000..78b9fd88a6 --- /dev/null +++ b/tests/networkxml2xmlupdatein/dns-host-modify.xml @@ -0,0 +1,5 @@ +<host ip='192.168.122.2'> + <hostname>Another</hostname> + <hostname>decent</hostname> + <hostname>names</hostname> +</host> diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml b/tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml new file mode 100644 index 0000000000..8fcaad15d1 --- /dev/null +++ b/tests/networkxml2xmlupdateout/nat-network-dns-hosts-modified.xml @@ -0,0 +1,28 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'> + <interface dev='eth0'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <dns forwardPlainNames='no'> + <host ip='192.168.122.122'> + <hostname>pudding</hostname> + </host> + <host ip='192.168.122.1'> + <hostname>host</hostname> + <hostname>gateway</hostname> + </host> + <host ip='192.168.122.2'> + <hostname>Another</hostname> + <hostname>decent</hostname> + <hostname>names</hostname> + </host> + <host ip='fd8f:1391:3a82:150::c0a8:9603'> + <hostname>shared</hostname> + <hostname>names</hostname> + </host> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index afe2b1f574..383cbf85ce 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -276,6 +276,15 @@ mymain(void) "nat-network-dns-hosts", "nat-network-no-hosts", VIR_NETWORK_UPDATE_COMMAND_DELETE); + DO_TEST("modify-dns-host", + "dns-host-modify", + "nat-network-dns-hosts", + "nat-network-dns-hosts-modified", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); + DO_TEST_FAIL("modify-dns-host-not-existing", + "dns-host-modify-not-existing", + "nat-network-dns-hosts", + VIR_NETWORK_UPDATE_COMMAND_MODIFY); section = VIR_NETWORK_SECTION_DNS_TXT; -- 2.45.2

2 1

[PATCH] domain_conf: comment not match the code below
by Adam Julis 10 Jul '24

10 Jul '24

The outdated comment refers to a non-existent member in the virDomainObj structure. Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7033b4e9fe..115862bbde 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4449,7 +4449,7 @@ virDomainObjGetDefs(virDomainObj *vm, * @live: set to true if live config was returned (may be omitted) * * Helper function to resolve @flags and return the correct domain pointer - * object. This function returns one of @vm->def or @vm->persistentDef + * object. This function returns one of @vm->def or @vm->newDef * according to @flags. @live is set to true if the live vm config will be * returned. This helper should be used only in APIs that guarantee * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE or @@ -4489,7 +4489,7 @@ virDomainObjGetOneDefState(virDomainObj *vm, * @flags: for virDomainModificationImpact * * Helper function to resolve @flags and return the correct domain pointer - * object. This function returns one of @vm->def or @vm->persistentDef + * object. This function returns one of @vm->def or @vm->newDef * according to @flags. This helper should be used only in APIs that guarantee * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE or * VIR_DOMAIN_AFFECT_CONFIG and not both. -- 2.45.2

2 1

[PATCH] virt-aa-helper: Drop needless comments
by Michal Privoznik 09 Jul '24

09 Jul '24

When generating paths for a domain specific AppArmor profile each path undergoes a validation where it's matched against an array of well known prefixes (among other things). Now, for OVMF/AAVMF/... images we have a list and some entries have comments to which type of image the entry belongs to. For instance: "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/AAVMF/", /* for AAVMF images */ But these comments are pretty useless. The path itself already gives away the image type. Drop them. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/virt-aa-helper.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index a3f85d26b0..c1e89dc6cf 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -475,15 +475,15 @@ valid_path(const char *path, const bool readonly) "/initrd", "/initrd.img", "/usr/share/edk2/", - "/usr/share/edk2-ovmf/", /* for OVMF images */ - "/usr/share/OVMF/", /* for OVMF images */ - "/usr/share/ovmf/", /* for OVMF images */ - "/usr/share/AAVMF/", /* for AAVMF images */ + "/usr/share/edk2-ovmf/", + "/usr/share/OVMF/", + "/usr/share/ovmf/", + "/usr/share/AAVMF/", "/usr/share/qemu-efi/", /* for AAVMF images */ - "/usr/share/qemu-efi-aarch64/", /* for AAVMF images */ + "/usr/share/qemu-efi-aarch64/", "/usr/share/qemu/", /* SUSE path for OVMF and AAVMF images */ - "/usr/lib/u-boot/", /* u-boot loaders for qemu */ - "/usr/lib/riscv64-linux-gnu/opensbi" /* RISC-V SBI implementation */ + "/usr/lib/u-boot/", + "/usr/lib/riscv64-linux-gnu/opensbi", }; /* override the above with these */ const char * const override[] = { -- 2.44.2

2 1

[libvirt PATCHv2 0/5] qemu: do not use deprecated options for new virtiofsd (virtiofs epopee)
by Ján Tomko 09 Jul '24

09 Jul '24

Also, allow migration if we think we can. Ján Tomko (5): tests: vhostuser: add virtiofsd json descriptor tests: qemuxmlconf: adjust test case to new virtiofsd qemu: fill capabilities for virtiofsd qemu: do not use deprecated options for new virtiofsd qemu: migration: allow migration for virtiofs src/conf/domain_conf.h | 1 + src/qemu/qemu_migration.c | 9 +- src/qemu/qemu_vhost_user.c | 60 ++++++++++++ src/qemu/qemu_vhost_user.h | 12 +++ src/qemu/qemu_virtiofs.c | 92 +++++++++++++------ .../libexec/qemu/vhost-user/test-virtiofsd | 10 ++ .../qemu/vhost-user/50-qemu-virtiofsd.json | 5 + tests/qemuvhostusertest.c | 1 + .../vhost-user-fs-fd-memory.xml | 3 +- 9 files changed, 161 insertions(+), 32 deletions(-) create mode 100755 tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-virtiofsd create mode 100644 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-qemu-virtiofsd.json -- 2.45.2

2 7

[PATCH v3 0/2] security_manager: Fix security manager deadlock after libvirtd fork()
by hongmianquan 09 Jul '24

09 Jul '24

We have the stack security driver, which internally manages other security drivers, just call them "top" and "nested". We call virSecurityStackPreFork() to lock the top one, and it also locks and then unlocks the nested drivers prior to fork. Then in qemuSecurityPostFork(), it unlocks the top one, but not the nested ones. Thus, if one of the nested drivers ("dac" or "selinux") is still locked, it will cause a deadlock. If we always surround nested locks with top lock, it is always secure. Because we have got top lock before fork child libvirtd. However, it is not always the case in the current code, We discovered this case: the nested list obtained through the qemuSecurityGetNested() will be locked directly for subsequent use, such as in virQEMUDriverCreateCapabilities(), where the nested list is locked using qemuSecurityGetDOI, but the top one is not locked beforehand. The problem stack is as follows: libvirtd thread1 libvirtd thread2 child libvirtd | | | | | | virsh capabilities qemuProcessLanuch | | | | | lock top | | | | lock nested | | | | | | fork------------------->|(nested lock held by thread1) | | | | | | unlock nested unlock top unlock top | | qemuSecuritySetSocketLabel | | lock nested (deadlock) v3 changes: Made modifications based on Michal's comments - ensured matching qemuSecurityStackLock() and qemuSecurityStackUnlock() - modify the correct order in libvirt_private.syms - split the code streamlining part into a separate patch hongmianquan (2): security_manager: Ensure top lock is acquired before nested locks security_manager: Remove redundant qemuSecurityGetNested() call src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 13 +++++++++++-- src/qemu/qemu_driver.c | 21 +++++++++++++-------- src/qemu/qemu_security.h | 2 ++ src/security/security_manager.c | 22 ++++++++++++++++++++++ src/security/security_manager.h | 2 ++ 6 files changed, 52 insertions(+), 10 deletions(-) -- 2.20.1

3 5

[PATCH 0/1] qemuDomainChangeNet: check virtio options for non-virtio models
by Miroslav Los 09 Jul '24

09 Jul '24

Hi, in our project, we create all of our domain interfaces regardless of model with a <driver><host csum='off'/></driver>. The documentation is not clear as to which models support offloading or not, and the domain validates. In libvirtd, since <driver> is present, this causes each network device to have its virDomainVirtioOptions *virtio to be non-NULL, with all members ABSENT, which is also deemed valid. However, clients receive the interface elements without the driver. When the modified value is sent back via updateDeviceFlags, the candidate's virtio attribute is thus NULL. The validation in qemuDomainChangeNet requires both structs to be equal or both NULL, which is violated by such a request. This can be seen also with how virsh domif-setlink operates: virsh # domiflist 5 Interface Type Source Model MAC -------------------------------------------------------- - udp - e1000 52:54:00:1c:10:42 - udp - e1000 52:54:00:09:29:9c - udp - e1000 52:54:00:1d:a6:8d - udp - e1000 52:54:00:07:4a:83 52:54:00:1c:10:42 up virsh # domif-setlink 5 52:54:00:1c:10:42 down error: Failed to update interface link state error: Operation not supported: cannot modify virtio network device driver options A workaround is to add an empty <driver> in the update request, which works until libvirt is restarted. The driver element on a non-virtio interface is not present when the domain is reloaded, thus the pointer is NULL in olddev, causing the inverse issue and same error. Our current workaround is to retry with the empty driver element only if the first call fails, but we think libvirt should handle this struct more consistently. This patch expands the check to the non-virtio case, where the olddev is assumed to be valid, and newdev's virtio options are validated to be NULL or all-ABSENT. Since the driver element is normally preserved for virtio models, the stricter equality condition is left unchanged in that case. I did not investigate further if the discrepancy between NULL and all-ABSENT values for virDomainVirtioOptions can cause a problem elsewhere, or in other device kinds. Though I do wonder if the options handling should be improved. One possibility is that non-virtio devices keep the value NULL, e.g. after validating that the driver element does not contain any of the options, whereas virtio devices always create the struct even with no driver element present on input. Another option is that NULL pointer is treated the same as all-ABSENT struct, e.g. in virDomainCheckVirtioOptionsAreAbsent and virDomainVirtioOptionsCheckABIStability; the former does that while the latter does not. These functions would also need to become callable from the qemu driver. Miroslav Los (1): qemuDomainChangeNet: check virtio options for non-virtio models src/qemu/qemu_hotplug.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) -- 2.25.1

2 2