On 08/27/2018 04:08 AM, Michal Privoznik wrote:
> After the previous commit we have VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN
> flag. This is not enough because it will keep connection open for only
> one instance of drvAcquire + drvRelease call. And when starting up a
> domain there will be a lot of such calls as there will be a lot of paths
> to relabel and thus lock. Therfore, VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN
> flag was introduced which allows us to keep connection open even after
s/was/will be/ (or is)
s/which allows us to keep/to allow keeping the/
> the drvAcquire + drvRelease pair. In order to close the connection after
> all locking has been done virLockManagerCloseConn is introduced.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/locking/lock_driver.h | 22 ++++++++++++++++++++++
> src/locking/lock_driver_lockd.c | 24 ++++++++++++++++++++++++
> src/locking/lock_driver_nop.c | 8 ++++++++
> src/locking/lock_manager.c | 11 +++++++++++
> src/locking/lock_manager.h | 4 ++++
> 6 files changed, 70 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 42f15f117e..bca5a51ba0 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1294,6 +1294,7 @@ virDomainLockProcessStart;
> virLockManagerAcquire;
> virLockManagerAddResource;
> virLockManagerClearResources;
> +virLockManagerCloseConn;
> virLockManagerFree;
> virLockManagerInquire;
> virLockManagerNew;
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> index 7e3ffc58b5..d81767707b 100644
> --- a/src/locking/lock_driver.h
> +++ b/src/locking/lock_driver.h
> @@ -282,6 +282,27 @@ typedef int (*virLockDriverRelease)(virLockManagerPtr man,
> char **state,
> unsigned int flags);
>
> +/**
> + * virLockDriverCloseConn:
> + * @man: the lock manager context
> + * @flags: optional flags, currently unused
> + *
> + * Close any connection that was saved via
> + * VIR_LOCK_MANAGER_ACQUIRE_KEEP_OPEN or
> + * VIR_LOCK_MANAGER_RELEASE_KEEP_OPEN flags.
Hmm, a client that forgets to provide KEEP_OPEN on one or the other
would wreak havoc with the algorithm. Upon further thought, the
KEEP_OPEN really only matters for Acquire and it should be saved in
@priv. That way the @priv would manage it not whether the flag was
provided on release.
> + * However, if there is still a resource locked, do not actually
> + * close the connection as it would result in killing the
> + * resource owner. This is similar to refcounting when all
> + * threads call virLockDriverCloseConn() but only the last one
> + * actually closes the connection.
> + *
NB: virObject{Ref|Unlock} uses atomic incr/decr for ref counting.
> + * Returns: 0 on success and connection not actually closed,
> + * 1 on success and connection closed,
> + * -1 otherwise
> + */
> +typedef int (*virLockDriverCloseConn)(virLockManagerPtr man,
> + unsigned int flags);
> +
> /**
> * virLockDriverInquire:
> * @manager: the lock manager context
> @@ -328,6 +349,7 @@ struct _virLockDriver {
>
> virLockDriverAcquire drvAcquire;
> virLockDriverRelease drvRelease;
> + virLockDriverCloseConn drvCloseConn;
> virLockDriverInquire drvInquire;
> };
>
> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
> index 14f9eae760..aec768b0df 100644
> --- a/src/locking/lock_driver_lockd.c
> +++ b/src/locking/lock_driver_lockd.c
> @@ -937,6 +937,28 @@ static int virLockManagerLockDaemonRelease(virLockManagerPtr lock,
> }
>
>
> +static int virLockManagerLockDaemonCloseConn(virLockManagerPtr lock,
> + unsigned int flags)
static int
virLock...
> +{
> + virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
> +
> + virCheckFlags(0, -1);
> +
> + if (priv->clientRefs)
> + return 0;
> +
> + virNetClientClose(priv->client);
> + virObjectUnref(priv->client);
> + virObjectUnref(priv->program);
> +
> + priv->client = NULL;
> + priv->program = NULL;
> + priv->counter = 0;
> +
> + return 1;
The helper concept from the previous patch still could apply here. I
would say I'm surprised this did anything in testing since clientRefs
wouldn't be 0 if acquire, then release is called w/ the KEEP_OPEN flag.
It'd be -1, I believe.
The rest seems fine.
John
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
