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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list