On 11/12/2014 04:51 AM, Pavel Hrdina wrote:
> Commit 6e5c79a1 tried to fix deadlock between nwfilter{Define,Undefine}
> and starting of guest, but this same deadlock is also for
s/is also/exists/
> updating/attaching network device to domain.
>
> The deadlock was introduced by removing global QEMU driver lock because
> nwfilter was counting on this lock and ensure that all driver locks are
> locked inside of nwfilter{Define,Undefine}.
>
> This patch extends usage of virNWFilterReadLockFilterUpdates to prevent
> the deadlock for all possible paths in QEMU driver. LXC and UML drivers
> still have global lock.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143780
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 12 ++++++++++++
> src/qemu/qemu_migration.c | 3 +++
> src/qemu/qemu_process.c | 4 ++++
> 3 files changed, 19 insertions(+)
>
I thought I had built these yesterday when reviewing, but apparently not
as doing a build just now failed because the symbols are not defined in
qemu_migration.c and qemu_process.c (I have gcc version 4.8.3 20140911
(Red Hat 4.8.3-7) (GCC) on my f20 box)
I squashed the following and it builds...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 533bb35..9106800 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -50,6 +50,7 @@
#include "viruuid.h"
#include "virtime.h"
#include "locking/domain_lock.h"
+#include "nwfilter_conf.h"
#include "rpc/virnetsocket.h"
#include "virstoragefile.h"
#include "viruri.h"
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0078a70..1c1476c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -57,6 +57,7 @@
#include "domain_nwfilter.h"
#include "locking/domain_lock.h"
#include "network/bridge_driver.h"
+#include "nwfilter_conf.h"
#include "viruuid.h"
#include "virprocess.h"
#include "virtime.h"
ACK with the squashed in #includes
John
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 56e8430..716e9a4 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5902,6 +5902,8 @@ qemuDomainRestoreFlags(virConnectPtr conn,
> VIR_DOMAIN_SAVE_PAUSED, -1);
>
>
> + virNWFilterReadLockFilterUpdates();
> +
> fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml,
> (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
> &wrapperFd, false, false);
> @@ -5979,6 +5981,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
> virFileWrapperFdFree(wrapperFd);
> if (vm)
> virObjectUnlock(vm);
> + virNWFilterUnlockFilterUpdates();
> return ret;
> }
>
> @@ -7500,6 +7503,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> VIR_DOMAIN_AFFECT_CONFIG, -1);
>
> + virNWFilterReadLockFilterUpdates();
> +
> cfg = virQEMUDriverGetConfig(driver);
>
> affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
> @@ -7616,6 +7621,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
> virObjectUnlock(vm);
> virObjectUnref(caps);
> virObjectUnref(cfg);
> + virNWFilterUnlockFilterUpdates();
> return ret;
> }
>
> @@ -7646,6 +7652,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
> VIR_DOMAIN_AFFECT_CONFIG |
> VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
>
> + virNWFilterReadLockFilterUpdates();
> +
> cfg = virQEMUDriverGetConfig(driver);
>
> affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
> @@ -7762,6 +7770,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
> virObjectUnlock(vm);
> virObjectUnref(caps);
> virObjectUnref(cfg);
> + virNWFilterUnlockFilterUpdates();
> return ret;
> }
>
> @@ -14503,6 +14512,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> * and use of FORCE can cause multiple transitions.
> */
>
> + virNWFilterReadLockFilterUpdates();
> +
> if (!(vm = qemuDomObjFromSnapshot(snapshot)))
> return -1;
>
> @@ -14824,6 +14835,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> virObjectUnlock(vm);
> virObjectUnref(caps);
> virObjectUnref(cfg);
> + virNWFilterUnlockFilterUpdates();
>
> return ret;
> }
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index c797206..533bb35 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2525,6 +2525,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> if (virTimeMillisNow(&now) < 0)
> return -1;
>
> + virNWFilterReadLockFilterUpdates();
> +
> if (flags & VIR_MIGRATE_OFFLINE) {
> if (flags & (VIR_MIGRATE_NON_SHARED_DISK |
> VIR_MIGRATE_NON_SHARED_INC)) {
> @@ -2826,6 +2828,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> qemuDomainEventQueue(driver, event);
> qemuMigrationCookieFree(mig);
> virObjectUnref(caps);
> + virNWFilterUnlockFilterUpdates();
> return ret;
>
> stop:
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 24e5f0f..0078a70 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3438,6 +3438,8 @@ qemuProcessReconnect(void *opaque)
>
> VIR_FREE(data);
>
> + virNWFilterReadLockFilterUpdates();
> +
> virObjectLock(obj);
>
> cfg = virQEMUDriverGetConfig(driver);
> @@ -3589,6 +3591,7 @@ qemuProcessReconnect(void *opaque)
>
> virObjectUnref(conn);
> virObjectUnref(cfg);
> + virNWFilterUnlockFilterUpdates();
>
> return;
>
> @@ -3624,6 +3627,7 @@ qemuProcessReconnect(void *opaque)
> }
> virObjectUnref(conn);
> virObjectUnref(cfg);
> + virNWFilterUnlockFilterUpdates();
> }
>
> static int
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list