May 2022 - Devel - libvirt List Archives

[PATCH] lib: Be consistent about vm->pid
by Michal Privoznik 31 May '22

31 May '22

The virDomainObj struct has @pid member where the domain's hypervisor PID is stored (e.g. QEMU/bhyve/libvirt_lxc/... PID). However, we are not consistent when it comes to shutoff state. Initially, because virDomainObjNew() uses g_new0() the @pid is initialized to 0. But when domain is shut off, some functions set it to -1 (virBhyveProcessStop, virCHProcessStop, qemuProcessStop, ..). In other places, the @pid is tested to be 0, on some other places it's tested for being negative and in the rest for being positive. To solve this inconsistency we can stick with either value, -1 or 0. I've chosen the latter as it's safer IMO. For instance if by mistake we'd kill(vm->pid, SIGTERM) we would kill ourselves instead of init's process group. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/bhyve/bhyve_process.c | 8 ++++---- src/ch/ch_domain.c | 2 +- src/ch/ch_process.c | 2 +- src/conf/domain_conf.h | 2 +- src/hypervisor/domain_cgroup.c | 2 +- src/lxc/lxc_process.c | 6 +++--- src/openvz/openvz_conf.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_process.c | 4 ++-- tests/qemusecuritytest.c | 1 - 10 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 18002b559b..d46786d393 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -293,7 +293,7 @@ virBhyveProcessStop(struct _bhyveConn *driver, return 0; } - if (vm->pid <= 0) { + if (vm->pid == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid PID %d for VM"), (int)vm->pid); @@ -329,7 +329,7 @@ virBhyveProcessStop(struct _bhyveConn *driver, bhyveProcessAutoDestroy); virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); - vm->pid = -1; + vm->pid = 0; vm->def->id = -1; bhyveProcessStopHook(vm, VIR_HOOK_BHYVE_OP_RELEASE); @@ -344,7 +344,7 @@ virBhyveProcessStop(struct _bhyveConn *driver, int virBhyveProcessShutdown(virDomainObj *vm) { - if (vm->pid <= 0) { + if (vm->pid == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid PID %d for VM"), (int)vm->pid); @@ -433,7 +433,7 @@ virBhyveProcessReconnect(virDomainObj *vm, if (!virDomainObjIsActive(vm)) return 0; - if (!vm->pid) + if (vm->pid == 0) return 0; virObjectLock(vm); diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index bb489a74e3..5ab526ba1b 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -405,7 +405,7 @@ virCHDomainGetMachineName(virDomainObj *vm) virCHDriver *driver = priv->driver; char *ret = NULL; - if (vm->pid > 0) { + if (vm->pid != 0) { ret = virSystemdGetMachineNameByPID(vm->pid); if (!ret) virResetLastError(); diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 977082d585..4247902bcf 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -574,7 +574,7 @@ virCHProcessStop(virCHDriver *driver G_GNUC_UNUSED, vm->def->name); } - vm->pid = -1; + vm->pid = 0; vm->def->id = -1; g_clear_pointer(&priv->machineName, g_free); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6906db4af5..e7e0f24443 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3059,7 +3059,7 @@ struct _virDomainObj { virObjectLockable parent; virCond cond; - pid_t pid; + pid_t pid; /* 0 for no PID, avoid negative values like -1 */ virDomainStateReason state; unsigned int autostart : 1; diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c index 8072465615..7265325406 100644 --- a/src/hypervisor/domain_cgroup.c +++ b/src/hypervisor/domain_cgroup.c @@ -517,7 +517,7 @@ virDomainCgroupSetupCgroup(const char *prefix, bool privileged, char *machineName) { - if (!vm->pid) { + if (vm->pid == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot setup cgroups until process is started")); return -1; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 9722d8e1de..d162812a74 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -217,7 +217,7 @@ static void virLXCProcessCleanup(virLXCDriver *driver, lxcProcessRemoveDomainStatus(cfg, vm); virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); - vm->pid = -1; + vm->pid = 0; vm->def->id = -1; if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback) @@ -892,7 +892,7 @@ int virLXCProcessStop(virLXCDriver *driver, _("Some processes refused to die")); return -1; } - } else if (vm->pid > 0) { + } else if (vm->pid != 0) { /* If cgroup doesn't exist, just try cleaning up the * libvirt_lxc process */ if (virProcessKillPainfully(vm->pid, true) < 0) { @@ -1033,7 +1033,7 @@ virLXCProcessReadLogOutputData(virDomainObj *vm, bool isdead = false; char *eol; - if (vm->pid <= 0 || + if (vm->pid == 0 || (kill(vm->pid, 0) == -1 && errno == ESRCH)) isdead = true; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 191c79e1e2..79ab786355 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -528,7 +528,7 @@ int openvzLoadDomains(struct openvz_driver *driver) if (STREQ(status, "stopped")) { virDomainObjSetState(dom, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); - dom->pid = -1; + dom->pid = 0; } else { virDomainObjSetState(dom, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 31e60c7359..8ebf152d95 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10741,7 +10741,7 @@ qemuDomainGetMachineName(virDomainObj *vm) virQEMUDriver *driver = priv->driver; char *ret = NULL; - if (vm->pid > 0) { + if (vm->pid != 0) { ret = virSystemdGetMachineNameByPID(vm->pid); if (!ret) virResetLastError(); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 07acb1c427..cbcc480089 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8239,7 +8239,7 @@ void qemuProcessStop(virQEMUDriver *driver, g_clear_pointer(&vm->deprecations, g_free); vm->ndeprecations = 0; vm->taint = 0; - vm->pid = -1; + vm->pid = 0; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); for (i = 0; i < vm->def->niothreadids; i++) vm->def->iothreadids[i]->thread_id = 0; @@ -8969,7 +8969,7 @@ qemuProcessReconnectHelper(virDomainObj *obj, g_autofree char *name = NULL; /* If the VM was inactive, we don't need to reconnect */ - if (!obj->pid) + if (obj->pid == 0) return 0; data = g_new0(struct qemuProcessReconnectData, 1); diff --git a/tests/qemusecuritytest.c b/tests/qemusecuritytest.c index 924c625a4c..4e2343b7d7 100644 --- a/tests/qemusecuritytest.c +++ b/tests/qemusecuritytest.c @@ -54,7 +54,6 @@ prepareObjects(virQEMUDriver *driver, if (!(vm = virDomainObjNew(driver->xmlopt))) return -1; - vm->pid = -1; priv = vm->privateData; priv->chardevStdioLogd = false; priv->rememberOwner = true; -- 2.35.1

2 1

[PATCH v2 0/5] network: firewalld: fix routed network
by Eric Garver 31 May '22

31 May '22

This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function. Firewalld policies are added. Policies have been supported since firewalld 0.9.0. If the running firewall does not support policies, then it will fallback to the current zone only behavior. v2: - keep existing libvirt zone as is - remove "<forward />" in libvirt-routed zone because this feature requires firewalld >= 0.9.0. Has no impact since the added policies allow forwarding libvirt-routed <--> ANY zone (including itself). - add probe for policies: virFirewallDGetPolicies(), virFirewallDPolicyExists() [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2055706 [2]: https://github.com/firewalld/firewalld/issues/177 Eric Garver (5): util: add virFirewallDGetPolicies() util: add virFirewallDPolicyExists() network: firewalld: add zone for routed networks network: firewalld: add policies for routed networks network: firewalld: add support for routed networks src/libvirt_private.syms | 2 + src/network/bridge_driver_linux.c | 11 +++- src/network/libvirt-routed-in.policy | 11 ++++ src/network/libvirt-routed-out.policy | 12 +++++ src/network/libvirt-routed.zone | 10 ++++ src/network/libvirt-to-host.policy | 20 ++++++++ src/network/meson.build | 20 ++++++++ src/util/virfirewalld.c | 72 +++++++++++++++++++++++++++ src/util/virfirewalld.h | 2 + 9 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/network/libvirt-routed-in.policy create mode 100644 src/network/libvirt-routed-out.policy create mode 100644 src/network/libvirt-routed.zone create mode 100644 src/network/libvirt-to-host.policy -- 2.35.3

1 5

[libvirt PATCH 0/4] ci: reduce number of jobs in the pipeline
by Daniel P. Berrangé 31 May '22

31 May '22

Come June 1st (aka very very soon) GitLab will reduce the CI minutes quota available from 2000 to 400 for public projects. The wallclock minutes for CI are the quota divided by a cost factor currently 0.008. IOW, after June 1st our allowance reduces from 250,000 wallclock minutes to 50,000. While I had intended that we join the OSS program, I'm unhappy with the financial liability the T&Cs require us to agree to for tha Usage stats show we're currently consuming 70-80,000 minutes a month, so we need to cut our usage by at least 30%. The main libvirt project is responsible for the vast majority of usage, so is the biggest quick win, but we need to economise across all our repos. With this series, we increase the number of jobs in the pipeline from 82 to 100, but we mark a great many of them as manual jobs, so they never run unless a user explicitly triggers them. So we have only 54 jobs executing instead of the original 82. The remaining 46 jobs are optional. This is a decent win in usage, but we probably need to cut a little more later to give us breathing space Daniel P. Berrangé (4): ci: refresh with lcitool manifest ci: disable native builds on certain distros ci: move Ubuntu GCC santizers build to 20.04 ci: eliminate many cross arch CI builds ci/gitlab/build-templates.yml | 17 ++- ci/gitlab/builds.yml | 246 ++++++++++++++++++++++++++++-- ci/gitlab/container-templates.yml | 3 + ci/gitlab/containers.yml | 50 +++--- ci/manifest.yml | 59 ++++--- 5 files changed, 329 insertions(+), 46 deletions(-) -- 2.36.1

2 6

[PATCH v2 0/2] 2 release notes for v8.4.0
by Han Han 31 May '22

31 May '22

v1: https://listman.redhat.com/archives/libvir-list/2022-May/231901.html diff from v1: rebase to latest and resolve conflicts Han Han (2): news: Add qemu-vdagent news: Add APIs virDomainSaveParams virDomainRestoreParams NEWS.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) -- 2.36.1

2 3

[PATCH RFC 00/10] qemu: Enable SCHED_CORE for domains and helper processes
by Michal Privoznik 31 May '22

31 May '22

The Linux kernel offers a way to mitigate side channel attacks on Hyper Threads (e.g. MDS and L1TF). Long story short, userspace can define groups of processes (aka trusted groups) and only processes within one group can run on sibling Hyper Threads. The group membership is automatically preserved on fork() and exec(). Now, there is one scenario which I don't cover in my series and I'd like to hear proposal: if there are two guests with odd number of vCPUs they can no longer run on sibling Hyper Threads because my patches create separate group for each QEMU. This is a performance penalty. Ideally, we would have a knob inside domain XML that would place two or more domains into the same trusted group. But since there's pre-existing example (of sharing a piece of information between two domains) I've failed to come up with something usable. Also, it's worth noting, that on kernel level, group membership is expressed by so called 'cookie' which is effectively an unique UL number, but there's no API that would "set this number on given process", so we may have to go with some abstraction layer. Michal Prívozník (10): qemu_tpm: Make APIs work over a single virDomainTPMDef qemu_dbus: Separate PID read code into qemuDBusGetPID qemu_vhost_user_gpu: Export qemuVhostUserGPUGetPid() qemu_tpm: Expose qemuTPMEmulatorGetPid() qemu_virtiofs: Separate PID read code into qemuVirtioFSGetPid virprocess: Core Scheduling support virCommand: Introduce APIs for core scheduling qemu_conf: Introduce a knob to turn off SCHED_CORE qemu: Enable SCHED_CORE for domains and helper processes qemu: Place helper processes into the same trusted group src/libvirt_private.syms | 6 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf.in | 5 + src/qemu/qemu_conf.c | 24 ++++ src/qemu/qemu_conf.h | 2 + src/qemu/qemu_dbus.c | 42 ++++--- src/qemu/qemu_dbus.h | 4 + src/qemu/qemu_extdevice.c | 171 ++++++++++++++++++++++++++--- src/qemu/qemu_extdevice.h | 3 + src/qemu/qemu_process.c | 9 ++ src/qemu/qemu_security.c | 4 + src/qemu/qemu_tpm.c | 91 +++++---------- src/qemu/qemu_tpm.h | 18 ++- src/qemu/qemu_vhost_user_gpu.c | 2 +- src/qemu/qemu_vhost_user_gpu.h | 8 ++ src/qemu/qemu_virtiofs.c | 41 ++++--- src/qemu/qemu_virtiofs.h | 5 + src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/vircommand.c | 74 +++++++++++++ src/util/vircommand.h | 5 + src/util/virprocess.c | 124 +++++++++++++++++++++ src/util/virprocess.h | 8 ++ 22 files changed, 538 insertions(+), 110 deletions(-) -- 2.35.1

4 32

[PATCH] qemu_capspriv: Drop needless declarations
by Michal Privoznik 31 May '22

31 May '22

There are two functions declared in qemu_capspriv.h: 1) virQEMUCapsInitHostCPUModel() which is not used anywhere but qemu_capabilities.c, 2) virQEMUCapsSetSEVCapabilities() which is my personal favorite but despite that it's never implemented nor called. Drop them. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capspriv.h | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce3e7d275a..d0c8217825 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3725,7 +3725,7 @@ virQEMUCapsNewHostCPUModel(void) } -void +static void virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, virArch hostArch, virDomainVirtType type) diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 26bf2d3571..fe24d01342 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -56,11 +56,6 @@ void virQEMUCapsSetArch(virQEMUCaps *qemuCaps, virArch arch); -void -virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, - virArch hostArch, - virDomainVirtType type); - void virQEMUCapsUpdateHostCPUModel(virQEMUCaps *qemuCaps, virArch hostArch, @@ -97,10 +92,6 @@ virQEMUCapsSetGICCapabilities(virQEMUCaps *qemuCaps, virGICCapability *capabilities, size_t ncapabilities); -void -virQEMUCapsSetSEVCapabilities(virQEMUCaps *qemuCaps, - virSEVCapability *capabilities); - int virQEMUCapsProbeCPUDefinitionsTest(virQEMUCaps *qemuCaps, qemuMonitor *mon); -- 2.35.1

2 1

[PATCH] hw/nvme: deprecate the use-intel-id compatibility parameter
by Klaus Jensen 30 May '22

30 May '22

From: Klaus Jensen <k.jensen(a)samsung.com> Since version 5.2 commit 6eb7a071292a ("hw/block/nvme: change controller pci id"), the emulated NVMe controller has defaulted to a non-Intel PCI identifier. Deprecate the compatibility parameter so we can get rid of it once and for all. Signed-off-by: Klaus Jensen <k.jensen(a)samsung.com> --- docs/about/deprecated.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 896e5a97abbd..450f945ac25f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -356,6 +356,14 @@ contains native support for this feature and thus use of the option ROM approach is obsolete. The native SeaBIOS support can be activated by using ``-machine graphics=off``. +``-device nvme,use-intel-id=on|off`` (since 7.1) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``nvme`` device originally used a PCI Vendor/Device Identifier combination +from Intel that was not properly allocated. Since version 5.2, the controller +has used a properly allocated identifier. Deprecate the ``use-intel-id`` +machine compatibility parameter. + Block device options '''''''''''''''''''' -- 2.35.1

2 2

[PATCH 0/2] virDomainInputDefValidate: Validate model
by Michal Privoznik 30 May '22

30 May '22

*** BLURB HERE *** Michal Prívozník (2): virDomainInputDefValidate: Reformat virDomainInputDefValidate: Validate model src/conf/domain_validate.c | 84 +++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 34 deletions(-) -- 2.35.1

2 3

[PATCH] glibcompat: Provide proper override for 'g_hash_table_steal_extended'
by Peter Krempa 30 May '22

30 May '22

We've emulated the function in virHashSteal, with a note pointing to use the proper version. Move the code to glibcomapt.c and make it such that builds using newer glib already use the new function. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/util/glibcompat.c | 20 ++++++++++++++++++++ src/util/glibcompat.h | 8 ++++++++ src/util/virhash.c | 11 +++-------- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c index eb6dcc0111..fdc32af5e2 100644 --- a/src/util/glibcompat.c +++ b/src/util/glibcompat.c @@ -64,6 +64,7 @@ #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #undef g_canonicalize_filename +#undef g_hash_table_steal_extended #undef g_fsync #undef g_strdup_printf #undef g_strdup_vprintf @@ -173,6 +174,25 @@ vir_g_canonicalize_filename(const gchar *filename, } +gboolean +vir_g_hash_table_steal_extended(GHashTable *hash_table, + gconstpointer lookup_key, + gpointer *stolen_key, + gpointer *stolen_value) +{ +#if GLIB_CHECK_VERSION(2, 58, 0) + return g_hash_table_steal_extended(hash_table, lookup_key, stolen_key, stolen_value); +#else /* ! GLIB_CHECK_VERSION(2, 58, 0) */ + if (!(g_hash_table_lookup_extended(hash_table, lookup_key, stolen_key, stolen_value))) + return FALSE; + + g_hash_table_steal(hash_table, lookup_key); + + return TRUE; +#endif /* ! GLIB_CHECK_VERSION(2, 58, 0) */ +} + + /* Drop when min glib >= 2.63.0 */ gint vir_g_fsync(gint fd) diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h index 697687b967..1f3a6f728f 100644 --- a/src/util/glibcompat.h +++ b/src/util/glibcompat.h @@ -70,6 +70,14 @@ gchar * vir_g_canonicalize_filename(const gchar *filename, const gchar *relative_to); + +gboolean +vir_g_hash_table_steal_extended(GHashTable *hash_table, + gconstpointer lookup_key, + gpointer *stolen_key, + gpointer *stolen_value); +#define g_hash_table_steal_extended vir_g_hash_table_steal_extended + gint vir_g_fsync(gint fd); char *vir_g_strdup_printf(const char *msg, ...) G_GNUC_PRINTF(1, 2); diff --git a/src/util/virhash.c b/src/util/virhash.c index a89b2d662e..c72c248186 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -242,24 +242,19 @@ virHashHasEntry(GHashTable *table, * Find the userdata specified by @name * and remove it from the hash without freeing it. * - * Deprecated: consider using g_hash_table_steal_extended once we upgrade to - * glib 2.58 + * Deprecated: consider using g_hash_table_steal_extended instead * * Returns a pointer to the userdata */ void *virHashSteal(GHashTable *table, const char *name) { - g_autofree void *orig_name = NULL; + g_autofree void *orig_name = NULL; /* the original key needs to be freed */ void *val = NULL; if (!table || !name) return NULL; - /* we can replace this by g_hash_table_steal_extended with glib 2.58 */ - if (!(g_hash_table_lookup_extended(table, name, &orig_name, &val))) - return NULL; - - g_hash_table_steal(table, name); + g_hash_table_steal_extended(table, name, &orig_name, &val); return val; } -- 2.35.3

2 1

[libvirt PATCH] virt-host-validate: Improve failure message when no HW virt is found
by Martin Kletzander 30 May '22

30 May '22

Make sure that more users understand that without HW virtualization you cannot have KVM working. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2086677 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- tools/virt-host-validate-qemu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c index 46ff1d249488..fcf9876ac916 100644 --- a/tools/virt-host-validate-qemu.c +++ b/tools/virt-host-validate-qemu.c @@ -71,7 +71,8 @@ int virHostValidateQEMU(void) virHostMsgPass(); } else { virHostMsgFail(VIR_HOST_VALIDATE_FAIL, - _("Only emulated CPUs are available, performance will be significantly limited")); + _("Host not compatible with KVM, HW virtualization CPU features not found. " + "Only emulated CPUs are available, performance will be significantly limited")); ret = -1; } } -- 2.35.1

2 1