[PATCH] virnwfilterobj: Unlock virNWFilterObj before triggering rebuild
by Michal Privoznik
When updating am NWFilter, rebuilding of all NWFilters is
triggered. This happens in virNWFilterObjListAssignDef() by
calling virNWFilterTriggerRebuild(). Now consider another thread,
that's currently executing virNWFilterBindingCreateXML() over the
same NWFilter.
What happens is that this second thread gets all the way into
virNWFilterInstantiateFilterInternal(), acquires @updateMutex and
transitively calls virNWFilterObjListFindByName() which iterates
over list of NWFilters, locking one by one, comparing names
trying to find the desired one. Sooner or later it will try to
lock the object that the other, original thread is redefining and
which it holds locked. Now that thread can't continue either
because it's waiting for the @updateMutex lock.
So we end up in a typical deadlock situation, one thread holding
lock A trying to acquire lock B, the other thread holding B and
trying to acquire A.
The solution is to unlock the virNWFilterObj in the first thread,
just before triggering rebuild.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2044379
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
I'm not sure this is 100% correct, but hey - it make that particular bug
go away.
src/conf/virnwfilterobj.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 6bbdf6e6fa..d6c2e59ce8 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -352,12 +352,17 @@ virNWFilterObjListAssignDef(virNWFilterObjList *nwfilters,
}
obj->newDef = def;
- /* trigger the update on VMs referencing the filter */
+
+ /* Trigger the update on VMs referencing the filter, but
+ * unlock the filter because rebuild will lock it again. */
+ virNWFilterObjUnlock(obj);
if (virNWFilterTriggerRebuild() < 0) {
+ virNWFilterObjLock(obj);
obj->newDef = NULL;
virNWFilterObjUnlock(obj);
return NULL;
}
+ virNWFilterObjLock(obj);
virNWFilterDefFree(objdef);
obj->def = def;
--
2.34.1
2 years, 3 months
[PATCH] systemd: Use correct man page name in modular daemon service files
by Peter Krempa
The service files were copied out of the service file for libvirtd and
the name of the corresponding manpage was not fixed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2045959
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/ch/virtchd.service.in | 2 +-
src/interface/virtinterfaced.service.in | 2 +-
src/libxl/virtxend.service.in | 2 +-
src/lxc/virtlxcd.service.in | 2 +-
src/network/virtnetworkd.service.in | 2 +-
src/node_device/virtnodedevd.service.in | 2 +-
src/nwfilter/virtnwfilterd.service.in | 2 +-
src/qemu/virtqemud.service.in | 2 +-
src/remote/virtproxyd.service.in | 2 +-
src/secret/virtsecretd.service.in | 2 +-
src/storage/virtstoraged.service.in | 2 +-
src/vbox/virtvboxd.service.in | 2 +-
src/vz/virtvzd.service.in | 2 +-
13 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in
index f08339f211..f53a12ea05 100644
--- a/src/ch/virtchd.service.in
+++ b/src/ch/virtchd.service.in
@@ -13,7 +13,7 @@ After=local-fs.target
After=remote-fs.target
After=systemd-logind.service
After=systemd-machined.service
-Documentation=man:libvirtd(8)
+Documentation=man:virtchd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/interface/virtinterfaced.service.in b/src/interface/virtinterfaced.service.in
index 3d944e17a9..cb860ff1c4 100644
--- a/src/interface/virtinterfaced.service.in
+++ b/src/interface/virtinterfaced.service.in
@@ -8,7 +8,7 @@ After=network.target
After=dbus.service
After=apparmor.service
After=local-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtinterfaced(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/libxl/virtxend.service.in b/src/libxl/virtxend.service.in
index 2b5163e179..6b083c414f 100644
--- a/src/libxl/virtxend.service.in
+++ b/src/libxl/virtxend.service.in
@@ -12,7 +12,7 @@ After=local-fs.target
After=remote-fs.target
After=xencommons.service
Conflicts=xendomains.service
-Documentation=man:libvirtd(8)
+Documentation=man:virtxend(8)
Documentation=https://libvirt.org
ConditionPathExists=/proc/xen/capabilities
diff --git a/src/lxc/virtlxcd.service.in b/src/lxc/virtlxcd.service.in
index d58bde9f5d..334c34db44 100644
--- a/src/lxc/virtlxcd.service.in
+++ b/src/lxc/virtlxcd.service.in
@@ -13,7 +13,7 @@ After=local-fs.target
After=remote-fs.target
After=systemd-logind.service
After=systemd-machined.service
-Documentation=man:libvirtd(8)
+Documentation=man:virtlxcd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/network/virtnetworkd.service.in b/src/network/virtnetworkd.service.in
index 3decfbbf1d..05ce672b73 100644
--- a/src/network/virtnetworkd.service.in
+++ b/src/network/virtnetworkd.service.in
@@ -11,7 +11,7 @@ After=ip6tables.service
After=dbus.service
After=apparmor.service
After=local-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtnetworkd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/node_device/virtnodedevd.service.in b/src/node_device/virtnodedevd.service.in
index 688cf89822..cd9de362fd 100644
--- a/src/node_device/virtnodedevd.service.in
+++ b/src/node_device/virtnodedevd.service.in
@@ -8,7 +8,7 @@ After=network.target
After=dbus.service
After=apparmor.service
After=local-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtnodedevd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/nwfilter/virtnwfilterd.service.in b/src/nwfilter/virtnwfilterd.service.in
index 36d00b58f0..ab65419e0c 100644
--- a/src/nwfilter/virtnwfilterd.service.in
+++ b/src/nwfilter/virtnwfilterd.service.in
@@ -8,7 +8,7 @@ After=network.target
After=dbus.service
After=apparmor.service
After=local-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtnwfilterd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in
index 551eb4d405..5ad968ace9 100644
--- a/src/qemu/virtqemud.service.in
+++ b/src/qemu/virtqemud.service.in
@@ -15,7 +15,7 @@ After=local-fs.target
After=remote-fs.target
After=systemd-logind.service
After=systemd-machined.service
-Documentation=man:libvirtd(8)
+Documentation=man:virtqemud(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/remote/virtproxyd.service.in b/src/remote/virtproxyd.service.in
index 10e8cf7263..f9bb6b84a9 100644
--- a/src/remote/virtproxyd.service.in
+++ b/src/remote/virtproxyd.service.in
@@ -8,7 +8,7 @@ After=network.target
After=dbus.service
After=apparmor.service
After=local-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtproxyd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/secret/virtsecretd.service.in b/src/secret/virtsecretd.service.in
index cbd63fe0b2..6d298c5334 100644
--- a/src/secret/virtsecretd.service.in
+++ b/src/secret/virtsecretd.service.in
@@ -8,7 +8,7 @@ After=network.target
After=dbus.service
After=apparmor.service
After=local-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtsecretd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/storage/virtstoraged.service.in b/src/storage/virtstoraged.service.in
index f72f8426fd..eda4d86d37 100644
--- a/src/storage/virtstoraged.service.in
+++ b/src/storage/virtstoraged.service.in
@@ -10,7 +10,7 @@ After=iscsid.service
After=apparmor.service
After=local-fs.target
After=remote-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtstoraged(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/vbox/virtvboxd.service.in b/src/vbox/virtvboxd.service.in
index cfdafc39d2..6f447276e9 100644
--- a/src/vbox/virtvboxd.service.in
+++ b/src/vbox/virtvboxd.service.in
@@ -9,7 +9,7 @@ After=dbus.service
After=apparmor.service
After=local-fs.target
After=remote-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtvboxd(8)
Documentation=https://libvirt.org
[Service]
diff --git a/src/vz/virtvzd.service.in b/src/vz/virtvzd.service.in
index 7636bf2b9e..2b1165c92b 100644
--- a/src/vz/virtvzd.service.in
+++ b/src/vz/virtvzd.service.in
@@ -9,7 +9,7 @@ After=dbus.service
After=apparmor.service
After=local-fs.target
After=remote-fs.target
-Documentation=man:libvirtd(8)
+Documentation=man:virtvzd(8)
Documentation=https://libvirt.org
[Service]
--
2.34.1
2 years, 3 months
[PATCH] virnwfilterbindingobj: Fix virNWFilterBindingObjNew()
by Michal Privoznik
The idea behind virNWFilterBindingObjNew() is to create and
return an object of virNWFilterBindingObjClass class. The class
is virObjectLockable (and the corresponding
_virNWFilterBindingObj structure has virObjectLockable parent).
But for some reason plain virObjectNew() is called. This is wrong
because the mutex in the parent is left uninitialized.
Next, the returned object is not locked. This is wrong because in
some cases the returned object is added onto a list of bindings
and then passed to virNWFilterBindingObjEndAPI() which unlocks it
right away. This is potentially dangerous because we might just
have unlocked the object for another thread.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/virnwfilterbindingobj.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c
index acea240b5d..d387af68c0 100644
--- a/src/conf/virnwfilterbindingobj.c
+++ b/src/conf/virnwfilterbindingobj.c
@@ -57,10 +57,15 @@ VIR_ONCE_GLOBAL_INIT(virNWFilterBindingObj);
virNWFilterBindingObj *
virNWFilterBindingObjNew(void)
{
+ virNWFilterBindingObj *ret;
if (virNWFilterBindingObjInitialize() < 0)
return NULL;
- return virObjectNew(virNWFilterBindingObjClass);
+ if (!(ret = virObjectLockableNew(virNWFilterBindingObjClass)))
+ return NULL;
+
+ virObjectLock(ret);
+ return ret;
}
--
2.34.1
2 years, 3 months
Re: [PATCH v3 1/2] hw/i386: Attach CPUs to machine
by Daniel P. Berrangé
Cc libvir-list since this will (intentionally) break compatibility
with current libvirt code that looks for "/machine/unattached/device[0]"
in the assumption it is the first CPU.
On Tue, Feb 01, 2022 at 12:35:06AM +0100, Philippe Mathieu-Daudé wrote:
> Previously CPUs were exposed in the QOM tree at a path
>
> /machine/unattached/device[nn]
>
> where the 'nn' of the first CPU is usually zero, but can
> vary depending on what devices were already created.
>
> With this change the CPUs are now at
>
> /machine/cpu[nn]
>
> where the 'nn' of the first CPU is always zero.
>
> Suggested-by: Daniel P. Berrangé <berrange(a)redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug(a)amsat.org>
> ---
> hw/i386/x86.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index b84840a1bb9..50bf249c700 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -108,6 +108,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> {
> Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
>
> + object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu));
> if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) {
> goto out;
> }
> --
> 2.34.1
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2 years, 3 months