On Thu, Aug 09, 2018 at 03:34:39PM +0200, Michal Privoznik wrote:
> This flag is going to be used to alter default behaviour of the
> lock. Firstly, it means we will lock different offset in the file
> (offset 1 instead of 0). Secondly, it means the lock acquiring
> will actually wait for the lock to be set (compared to default
> behaviour which is set or error out).
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/util/virlockspace.c | 40 +++++++++++++++++++++++++++++++++-------
> src/util/virlockspace.h | 1 +
> 2 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
> index 3364c843aa..a7ec0c05bf 100644
> --- a/src/util/virlockspace.c
> +++ b/src/util/virlockspace.c
> @@ -111,6 +111,8 @@ static void virLockSpaceResourceFree(virLockSpaceResourcePtr res)
> VIR_FREE(res);
> }
>
> +#define DEFAULT_OFFSET 0
> +#define METADATA_OFFSET 1
>
> static virLockSpaceResourcePtr
> virLockSpaceResourceNew(virLockSpacePtr lockspace,
> @@ -120,6 +122,18 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
> {
> virLockSpaceResourcePtr res;
> bool shared = !!(flags & VIR_LOCK_SPACE_ACQUIRE_SHARED);
> + bool metadata = !!(flags & VIR_LOCK_SPACE_ACQUIRE_METADATA);
> + off_t start = DEFAULT_OFFSET;
> + bool waitForLock = false;
> +
> + if (metadata) {
> + /* We want the metadata lock to act like pthread mutex.
> + * This means waiting for the lock to be acquired. */
> + waitForLock = true;
> +
> + /* Also, we are locking different offset. */
> + start = METADATA_OFFSET;
> + }
>
> if (VIR_ALLOC(res) < 0)
> return NULL;
> @@ -157,7 +171,7 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
> goto error;
> }
>
> - if (virFileLock(res->fd, shared, 0, 1, false) < 0) {
> + if (virFileLock(res->fd, shared, start, 1, waitForLock) < 0) {
> if (errno == EACCES || errno == EAGAIN) {
> virReportError(VIR_ERR_RESOURCE_BUSY,
> _("Lockspace resource '%s' is locked"),
> @@ -204,7 +218,7 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
> goto error;
> }
>
> - if (virFileLock(res->fd, shared, 0, 1, false) < 0) {
> + if (virFileLock(res->fd, shared, start, 1, waitForLock) < 0) {
> if (errno == EACCES || errno == EAGAIN) {
> virReportError(VIR_ERR_RESOURCE_BUSY,
> _("Lockspace resource '%s' is locked"),
> @@ -621,7 +635,15 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
> lockspace, resname, flags, (unsigned long long)owner);
>
> virCheckFlags(VIR_LOCK_SPACE_ACQUIRE_SHARED |
> - VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, -1);
> + VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE |
> + VIR_LOCK_SPACE_ACQUIRE_METADATA, -1);
> +
> + if (flags & VIR_LOCK_SPACE_ACQUIRE_METADATA &&
> + flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) {
> + virReportInvalidArg(flags, "%s",
> + _("metadata and shared are mutually exclusive"));
> + return -1;
> + }
>
> virMutexLock(&lockspace->lock);
>
> @@ -635,10 +657,14 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
>
> goto done;
> }
> - virReportError(VIR_ERR_RESOURCE_BUSY,
> - _("Lockspace resource '%s' is locked"),
> - resname);
> - goto cleanup;
> +
> + if (!(res->flags & VIR_LOCK_SPACE_ACQUIRE_METADATA) ||
> + !(flags & VIR_LOCK_SPACE_ACQUIRE_METADATA)) {
> + virReportError(VIR_ERR_RESOURCE_BUSY,
> + _("Lockspace resource '%s' is locked"),
> + resname);
> + goto cleanup;
> + }
> }
>
> if (!(res = virLockSpaceResourceNew(lockspace, resname, flags, owner)))
> diff --git a/src/util/virlockspace.h b/src/util/virlockspace.h
> index 041cf20396..a9a3b2e73f 100644
> --- a/src/util/virlockspace.h
> +++ b/src/util/virlockspace.h
> @@ -45,6 +45,7 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace,
> typedef enum {
> VIR_LOCK_SPACE_ACQUIRE_SHARED = (1 << 0),
> VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE = (1 << 1),
> + VIR_LOCK_SPACE_ACQUIRE_METADATA = (1 << 2),
> } virLockSpaceAcquireFlags;
>
> int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
The virLockSpace code is intended to be a bit more generic than the
locking protocol.
So I'd have a preference for having a VIR_LOCK_SPACE_ACQUIRE_WAIT
flag, and passing in the offset + length to the APIs as parameters.
Just have the "METADATA" concept in the lock driver only, not this
general purpose code.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
