Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/locking/lock_driver.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
index 8b7cccc521..7c8f79520a 100644
--- a/src/locking/lock_driver.h
+++ b/src/locking/lock_driver.h
@@ -56,6 +56,8 @@ typedef enum {
     VIR_LOCK_MANAGER_RESOURCE_READONLY = (1 << 0),
     /* The resource is assigned in shared, writable mode */
     VIR_LOCK_MANAGER_RESOURCE_SHARED   = (1 << 1),
+    /* The resource is locked for metadata change */
+    VIR_LOCK_MANAGER_RESOURCE_METADATA = (1 << 2),
 } virLockManagerResourceFlags;
 
 typedef enum {
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 08/14/2018 07:19 AM, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/locking/lock_driver.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> index 8b7cccc521..7c8f79520a 100644
> --- a/src/locking/lock_driver.h
> +++ b/src/locking/lock_driver.h
> @@ -56,6 +56,8 @@ typedef enum {
>      VIR_LOCK_MANAGER_RESOURCE_READONLY = (1 << 0),
>      /* The resource is assigned in shared, writable mode */
>      VIR_LOCK_MANAGER_RESOURCE_SHARED   = (1 << 1),
> +    /* The resource is locked for metadata change */
> +    VIR_LOCK_MANAGER_RESOURCE_METADATA = (1 << 2),

Does this work or make sense for lease type?

Of course this adds something that would "conceivably" be available such
that we may want to document it on
https://libvirt.org/internals/locking.html, but then again that's so
sparse it would take a bit of spelunking in the code to figure it out.

Anyway, since the name itself seems reasonable to me,

Reviewed-by: John Ferlan <jferlan@redhat.com>

if you add a few more details around it related to disk vs. lease that'd
be nice. If you update the docs eventually with all that you've learned,
then that'd be great too!

John


>  } virLockManagerResourceFlags;
>  
>  typedef enum {
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 08/17/2018 12:24 AM, John Ferlan wrote:
> 
> 
> On 08/14/2018 07:19 AM, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/locking/lock_driver.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
>> index 8b7cccc521..7c8f79520a 100644
>> --- a/src/locking/lock_driver.h
>> +++ b/src/locking/lock_driver.h
>> @@ -56,6 +56,8 @@ typedef enum {
>>      VIR_LOCK_MANAGER_RESOURCE_READONLY = (1 << 0),
>>      /* The resource is assigned in shared, writable mode */
>>      VIR_LOCK_MANAGER_RESOURCE_SHARED   = (1 << 1),
>> +    /* The resource is locked for metadata change */
>> +    VIR_LOCK_MANAGER_RESOURCE_METADATA = (1 << 2),
> 
> Does this work or make sense for lease type?

That's the thing. You're right, it doesn't make sense for lease type.
But on the level of RPC of virtlockd both leases and disks kind of blend
together. I mean, virtlockd is merely told to lock this or that file.

> 
> Of course this adds something that would "conceivably" be available such
> that we may want to document it on
> https://libvirt.org/internals/locking.html, but then again that's so
> sparse it would take a bit of spelunking in the code to figure it out.
> 
> Anyway, since the name itself seems reasonable to me,
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> if you add a few more details around it related to disk vs. lease that'd
> be nice. If you update the docs eventually with all that you've learned,
> then that'd be great too!

I'm gonna save that to a follow up patch.

Thanks!
Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Aug 20, 2018 at 09:25:18AM +0200, Michal Prívozník wrote:
> On 08/17/2018 12:24 AM, John Ferlan wrote:
> > 
> > 
> > On 08/14/2018 07:19 AM, Michal Privoznik wrote:
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  src/locking/lock_driver.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> >> index 8b7cccc521..7c8f79520a 100644
> >> --- a/src/locking/lock_driver.h
> >> +++ b/src/locking/lock_driver.h
> >> @@ -56,6 +56,8 @@ typedef enum {
> >>      VIR_LOCK_MANAGER_RESOURCE_READONLY = (1 << 0),
> >>      /* The resource is assigned in shared, writable mode */
> >>      VIR_LOCK_MANAGER_RESOURCE_SHARED   = (1 << 1),
> >> +    /* The resource is locked for metadata change */
> >> +    VIR_LOCK_MANAGER_RESOURCE_METADATA = (1 << 2),
> > 
> > Does this work or make sense for lease type?
> 
> That's the thing. You're right, it doesn't make sense for lease type.
> But on the level of RPC of virtlockd both leases and disks kind of blend
> together. I mean, virtlockd is merely told to lock this or that file.

That's ok.   The way I look at it the domain XML describes various
resources, disks, leases, etc.  The virtlockd daemon provides a
generic mechanism for acquiring locks.  We just happen to map leases
onto the default lock type. Here we're adding a 2nd lock type and do
not require any mapping from leases.


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