[libvirt] [PATCH] Clean some comments about locking
Martin Kletzander
mkletzan at redhat.com
Tue Feb 12 12:52:08 UTC 2013
Since the BQDL has been broken, this patch tries to cleanup some
information about it in various comments.
---
To be applied after Dan's patch [1] which removes unnecessary
qemuDriverLocks.
[1] https://www.redhat.com/archives/libvir-list/2013-February/msg00641.html
src/qemu/THREADS.txt | 9 ++-------
src/qemu/qemu_conf.c | 8 +++++---
src/qemu/qemu_conf.h | 5 +----
src/qemu/qemu_domain.c | 11 +++++------
src/qemu/qemu_domain.h | 3 +--
src/qemu/qemu_driver.c | 5 +----
src/qemu/qemu_migration.c | 4 ++--
src/qemu/qemu_process.c | 40 ++++++++++++++--------------------------
8 files changed, 31 insertions(+), 54 deletions(-)
diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt
index 785be99..50a0cf9 100644
--- a/src/qemu/THREADS.txt
+++ b/src/qemu/THREADS.txt
@@ -128,7 +128,7 @@ To acquire the normal job condition
To acquire the asynchronous job condition
- qemuDomainObjBeginAsyncJob() (if driver is unlocked)
+ qemuDomainObjBeginAsyncJob()
- Increments ref count on virDomainObjPtr
- Waits until no async job is running
- Waits for job.cond condition 'job.active != 0' using virDomainObjPtr
@@ -155,8 +155,6 @@ To acquire the QEMU monitor lock
- Releases the qemuMonitorObjPtr lock
- Acquires the virDomainObjPtr lock
- NB: caller must take care to drop the driver lock if necessary
-
These functions must not be used by an asynchronous job.
@@ -166,12 +164,10 @@ To acquire the QEMU monitor lock as part of an asynchronous job
- Validates that the right async job is still running
- Acquires the qemuMonitorObjPtr lock
- Releases the virDomainObjPtr lock
- - Releases the driver lock
- Validates that the VM is still active
qemuDomainObjExitMonitor()
- Releases the qemuMonitorObjPtr lock
- - Acquires the driver lock
- Acquires the virDomainObjPtr lock
These functions are for use inside an asynchronous job; the caller
@@ -180,8 +176,7 @@ To acquire the QEMU monitor lock as part of an asynchronous job
used from a sync job (such as when first starting a domain).
-To keep a domain alive while waiting on a remote command, starting
-with the driver lock held
+To keep a domain alive while waiting on a remote command
qemuDomainObjEnterRemote()
- Increments ref count on virDomainObjPtr
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5ac2004..4ff912d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1,7 +1,7 @@
/*
* qemu_conf.c: QEMU configuration management
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -79,11 +79,13 @@ struct _qemuDriverCloseDef {
qemuDriverCloseCallback cb;
};
-void qemuDriverLock(virQEMUDriverPtr driver)
+static void
+qemuDriverLock(virQEMUDriverPtr driver)
{
virMutexLock(&driver->lock);
}
-void qemuDriverUnlock(virQEMUDriverPtr driver)
+static void
+qemuDriverUnlock(virQEMUDriverPtr driver)
{
virMutexUnlock(&driver->lock);
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 09eacce..14f1427 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -1,7 +1,7 @@
/*
* qemu_conf.h: QEMU configuration management
*
- * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -236,9 +236,6 @@ struct _qemuDomainCmdlineDef {
# define QEMUD_MIGRATION_NUM_PORTS 64
-void qemuDriverLock(virQEMUDriverPtr driver);
-void qemuDriverUnlock(virQEMUDriverPtr driver);
-
virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged);
int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ec4c8f8..482f64a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1,7 +1,7 @@
/*
* qemu_domain.h: QEMU domain private state
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -116,7 +116,6 @@ qemuDomainAsyncJobPhaseFromString(enum qemuDomainAsyncJob job,
}
-/* driver must be locked before calling */
void qemuDomainEventQueue(virQEMUDriverPtr driver,
virDomainEventPtr event)
{
@@ -760,7 +759,7 @@ qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, enum qemuDomainJob job)
#define QEMU_JOB_WAIT_TIME (1000ull * 30)
/*
- * obj must be locked before calling;
+ * obj must be locked before calling
*/
static int ATTRIBUTE_NONNULL(1)
qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
@@ -857,7 +856,7 @@ error:
}
/*
- * obj must be locked before calling, driver must NOT be locked
+ * obj must be locked before calling
*
* This must be called by anything that will change the VM state
* in any way, or anything that will use the QEMU monitor.
@@ -883,7 +882,7 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
/*
- * obj must be locked before calling, driver does not matter
+ * obj must be locked before calling
*
* To be called after completing the work associated with the
* earlier qemuDomainBeginJob() call
@@ -1734,7 +1733,7 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
}
/*
- * The caller must hold a lock on both driver and vm, and there must
+ * The caller must hold a lock the vm and there must
* be no remaining references to vm.
*/
void
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cfc952d..905b099 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1,7 +1,7 @@
/*
* qemu_domain.h: QEMU domain private state
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -175,7 +175,6 @@ int qemuDomainAsyncJobPhaseFromString(enum qemuDomainAsyncJob job,
void qemuDomainEventFlush(int timer, void *opaque);
-/* driver must be locked before calling */
void qemuDomainEventQueue(virQEMUDriverPtr driver,
virDomainEventPtr event);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4d756f3..2ad21dc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2746,8 +2746,7 @@ cleanup:
return ret;
}
-/* This internal function expects the driver lock to already be held on
- * entry and the vm must be active + locked. Vm will be unlocked and
+/* The vm must be active + locked. Vm will be unlocked and
* potentially free'd after this returns (eg transient VMs are freed
* shutdown). So 'vm' must not be referenced by the caller after
* this returns (whether returning success or failure).
@@ -10349,8 +10348,6 @@ cleanup:
}
-
-/* this function expects the driver lock to be held by the caller */
static int
qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) {
qemuDomainObjPrivatePtr priv = vm->privateData;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 60cd47a..36e55d2 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2,7 +2,7 @@
/*
* qemu_migration.c: QEMU migration handling
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -3620,7 +3620,7 @@ cleanup:
}
-/* Helper function called while driver lock is held and vm is active. */
+/* Helper function called while vm is active. */
int
qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm,
int fd, off_t offset, const char *path,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 306f686..e5eadc0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1,7 +1,7 @@
/*
* qemu_process.h: QEMU process management
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -599,8 +599,7 @@ endjob:
cleanup:
if (vm) {
if (ret == -1) {
- ignore_value(qemuProcessKill(vm,
- VIR_QEMU_PROCESS_KILL_FORCE));
+ ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE));
}
if (virObjectUnref(vm))
virObjectUnlock(vm);
@@ -625,13 +624,11 @@ qemuProcessShutdownOrReboot(virQEMUDriverPtr driver,
qemuProcessFakeReboot,
vm) < 0) {
VIR_ERROR(_("Failed to create reboot thread, killing domain"));
- ignore_value(qemuProcessKill(vm,
- VIR_QEMU_PROCESS_KILL_NOWAIT));
+ ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT));
virObjectUnref(vm);
}
} else {
- ignore_value(qemuProcessKill(vm,
- VIR_QEMU_PROCESS_KILL_NOWAIT));
+ ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT));
}
}
@@ -2792,9 +2789,8 @@ qemuProcessPrepareMonitorChr(virQEMUDriverConfigPtr cfg,
/*
- * Precondition: Both driver and vm must be locked,
- * and a job must be active. This method will call
- * {Enter,Exit}Monitor
+ * Precondition: vm must be locked, and a job must be active.
+ * This method will call {Enter,Exit}Monitor
*/
int
qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm,
@@ -3374,23 +3370,15 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
memcpy(data, src, sizeof(*data));
data->payload = obj;
- /* This iterator is called with driver being locked.
+ /*
* We create a separate thread to run qemuProcessReconnect in it.
* However, qemuProcessReconnect needs to:
- * 1. lock driver
- * 2. just before monitor reconnect do lightweight MonitorEnter
+ * 1. just before monitor reconnect do lightweight MonitorEnter
* (increase VM refcount, unlock VM & driver)
- * 3. reconnect to monitor
- * 4. do lightweight MonitorExit (lock driver & VM)
- * 5. continue reconnect process
- * 6. EndJob
- * 7. unlock driver
- *
- * It is necessary to NOT hold driver lock for the entire run
- * of reconnect, otherwise we will get blocked if there is
- * unresponsive qemu.
- * However, iterating over hash table MUST be done on locked
- * driver.
+ * 2. reconnect to monitor
+ * 3. do lightweight MonitorExit (lock VM)
+ * 4. continue reconnect process
+ * 5. EndJob
*
* NB, we can't do normal MonitorEnter & MonitorExit because
* these two lock the monitor lock, which does not exists in
@@ -4184,8 +4172,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
}
/*
- * We may unlock the driver and vm in qemuProcessKill(), and another thread
- * can lock driver and vm, and then call qemuProcessStop(). So we should
+ * We may unlock the vm in qemuProcessKill(), and another thread
+ * can lock the vm, and then call qemuProcessStop(). So we should
* set vm->def->id to -1 here to avoid qemuProcessStop() to be called twice.
*/
vm->def->id = -1;
--
1.8.1.2
More information about the libvir-list
mailing list