Re: [libvirt] [PATCH v3 16/28] lock_driver: Introduce KEEP_OPEN flags

This flag causes connection to be opened when needed (e.g. when calling virLockManagerLockDaemonAcquire for the first time) and instead of closing it at the end of such API store it in privateData so that it can be reused by later calls. This is needed because if a resource is acquired and connection is closed then virtlockd kills the registered PID (that's what virtlockd is designed to do). Therefore we will need the connection to open at drvAcquire and close not any sooner than drvRelease. However, as we will be locking files step-by-step we want to avoid opening new connection for every drvAcquire + drvRelease pair, so the connection is going to be shared even more than that. But more on that in next commit. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/locking/lock_driver.h | 7 +++++ src/locking/lock_driver_lockd.c | 68 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index 59c4c3aac7..7e3ffc58b5 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -67,8 +67,15 @@ typedef enum { VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0), /* Prevent further lock/unlock calls from this process */ VIR_LOCK_MANAGER_ACQUIRE_RESTRICT = (1 << 1), + /* Causes driver to keep connection open and reuse it for further use. */ + VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN = (1 << 2), } virLockManagerAcquireFlags; +typedef enum { + /* Reuse previously saved connection. */ + VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN = (1 << 0), +} virLockManagerReleaseFlags; + typedef enum { /* virLockManagerNew called for a freshly started domain */ VIR_LOCK_MANAGER_NEW_STARTED = (1 << 0), diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 4883e89ac6..14f9eae760 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -76,6 +76,11 @@ struct _virLockManagerLockDaemonPrivate { size_t nresources; virLockManagerLockDaemonResourcePtr resources; + + int clientRefs; + virNetClientPtr client; + virNetClientProgramPtr program; + int counter; }; @@ -440,6 +445,13 @@ virLockManagerLockDaemonPrivateFree(virLockManagerLockDaemonPrivatePtr priv) default: break; } + + if (priv->client) { + virNetClientClose(priv->client); + virObjectUnref(priv->client); + virObjectUnref(priv->program); + } +

VIR_FREE(priv); } @@ -770,7 +782,8 @@ static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock, virLockManagerLockDaemonPrivatePtr priv = lock->privateData; virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY | - VIR_LOCK_MANAGER_ACQUIRE_RESTRICT, -1); + VIR_LOCK_MANAGER_ACQUIRE_RESTRICT | + VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN, -1); if (priv->type == VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN && priv->nresources == 0 && @@ -781,7 +794,14 @@ static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock, return -1; } - if (!(client = virLockManagerLockDaemonConnect(lock, &program, &counter))) + if (flags & VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN) { + client = priv->client; + program = priv->program; + counter = priv->counter; + } + + if (!client && + !(client = virLockManagerLockDaemonConnect(lock, &program, &counter)))

goto cleanup; if (fd && @@ -814,11 +834,25 @@ static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock, virLockManagerLockDaemonConnectionRestrict(lock, client, program, &counter) < 0) goto cleanup; + if (flags & VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN) { + VIR_STEAL_PTR(priv->client, client); + VIR_STEAL_PTR(priv->program, program); + priv->counter = counter; + } +

rv = 0; cleanup: - if (rv != 0 && fd) - VIR_FORCE_CLOSE(*fd); + if (rv < 0) { + if (fd) + VIR_FORCE_CLOSE(*fd); + + priv->client = NULL; + priv->program = NULL; + priv->counter = 0; + priv->clientRefs = 0;

+ } + virNetClientClose(client); virObjectUnref(client); virObjectUnref(program);

@@ -837,12 +871,20 @@ static int virLockManagerLockDaemonRelease(virLockManagerPtr lock, size_t i; virLockManagerLockDaemonPrivatePtr priv = lock->privateData; - virCheckFlags(0, -1); + virCheckFlags(VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN, -1); if (state) *state = NULL; - if (!(client = virLockManagerLockDaemonConnect(lock, &program, &counter))) + if (flags & VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN) { + client = priv->client; + program = priv->program; + counter = priv->counter; + priv->clientRefs--;

+ } + + if (!client && + !(client = virLockManagerLockDaemonConnect(lock, &program, &counter))) goto cleanup; for (i = 0; i < priv->nresources; i++) { @@ -870,9 +912,23 @@ static int virLockManagerLockDaemonRelease(virLockManagerPtr lock, goto cleanup; } + if (flags & VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN) { + /* Avoid freeing in cleanup. */ + client = NULL; + program = NULL; + counter = 0; + } + rv = 0; cleanup: + if (rv < 0) { + priv->client = NULL; + priv->program = NULL; + priv->counter = 0; + priv->clientRefs = 0; + } +

virNetClientClose(client); virObjectUnref(client); virObjectUnref(program);