[libvirt] [PATCH v3 13/28] lock_driver: Introduce VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA

Michal Privoznik posted 28 patches 7 years, 5 months ago
There is a newer version of this series
[libvirt] [PATCH v3 13/28] lock_driver: Introduce VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA
Posted by Michal Privoznik 7 years, 5 months ago
This is a new type of object that lock drivers can handle.
Currently, it is supported by lockd driver only.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/locking/lock_driver.h         |  2 ++
 src/locking/lock_driver_lockd.c   | 43 +++++++++++++++++++++++++++++++--------
 src/locking/lock_driver_sanlock.c |  3 ++-
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
index a9d2041c30..9be0abcfba 100644
--- a/src/locking/lock_driver.h
+++ b/src/locking/lock_driver.h
@@ -51,6 +51,8 @@ typedef enum {
     VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0,
     /* A lease against an arbitrary resource */
     VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE = 1,
+    /* The resource to be locked is a metadata */
+    VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA = 2,
 } virLockManagerResourceType;
 
 typedef enum {
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 98953500b7..d7cb183d7a 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -557,6 +557,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
     virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
     char *newName = NULL;
     char *newLockspace = NULL;
+    int newFlags = 0;
     bool autoCreate = false;
     int ret = -1;
 
@@ -569,7 +570,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
     switch (priv->type) {
     case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
 
-        switch (type) {
+        switch ((virLockManagerResourceType) type) {
         case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
             if (params || nparams) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -670,6 +671,8 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
                 goto cleanup;
 
         }   break;
+
+        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
         default:
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Unknown lock manager object type %d for domain lock object"),
@@ -679,6 +682,29 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
         break;
 
     case VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON:
+        switch ((virLockManagerResourceType) type) {
+        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
+            if (params || nparams) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("Unexpected parameters for metadata resource"));
+                goto cleanup;
+            }
+            if (VIR_STRDUP(newLockspace, "") < 0 ||
+                VIR_STRDUP(newName, name) < 0)
+                goto cleanup;
+            newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA;
+            break;
+
+        case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
+        case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE:
+        default:
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unknown lock manager object type %d for daemon lock object"),
+                           type);
+            goto cleanup;
+        }
+        break;
+
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unknown lock manager object type %d"),
@@ -686,19 +712,18 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
         goto cleanup;
     }
 
+    if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
+        newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
+
+    if (autoCreate)
+        newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE;
+
     if (VIR_EXPAND_N(priv->resources, priv->nresources, 1) < 0)
         goto cleanup;
 
     VIR_STEAL_PTR(priv->resources[priv->nresources-1].lockspace, newLockspace);
     VIR_STEAL_PTR(priv->resources[priv->nresources-1].name, newName);
-
-    if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
-        priv->resources[priv->nresources-1].flags |=
-            VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
-
-    if (autoCreate)
-        priv->resources[priv->nresources-1].flags |=
-            VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE;
+    priv->resources[priv->nresources-1].flags = newFlags;
 
     ret = 0;
  cleanup:
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index fe422d3be6..9393e7d9a2 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -815,7 +815,7 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
     if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY)
         return 0;
 
-    switch (type) {
+    switch ((virLockManagerResourceType) type) {
     case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
         if (driver->autoDiskLease) {
             if (virLockManagerSanlockAddDisk(driver, lock, name, nparams, params,
@@ -839,6 +839,7 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
             return -1;
         break;
 
+    case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
     default:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unknown lock manager object type %d for domain lock object"),
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 13/28] lock_driver: Introduce VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA
Posted by John Ferlan 7 years, 5 months ago

On 08/27/2018 04:08 AM, Michal Privoznik wrote:
> This is a new type of object that lock drivers can handle.
> Currently, it is supported by lockd driver only.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/locking/lock_driver.h         |  2 ++
>  src/locking/lock_driver_lockd.c   | 43 +++++++++++++++++++++++++++++++--------
>  src/locking/lock_driver_sanlock.c |  3 ++-
>  3 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> index a9d2041c30..9be0abcfba 100644
> --- a/src/locking/lock_driver.h
> +++ b/src/locking/lock_driver.h
> @@ -51,6 +51,8 @@ typedef enum {
>      VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0,
>      /* A lease against an arbitrary resource */
>      VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE = 1,
> +    /* The resource to be locked is a metadata */
> +    VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA = 2,
>  } virLockManagerResourceType;
>  
>  typedef enum {
> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
> index 98953500b7..d7cb183d7a 100644
> --- a/src/locking/lock_driver_lockd.c
> +++ b/src/locking/lock_driver_lockd.c
> @@ -557,6 +557,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>      virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
>      char *newName = NULL;
>      char *newLockspace = NULL;
> +    int newFlags = 0;
>      bool autoCreate = false;
>      int ret = -1;
>  
> @@ -569,7 +570,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>      switch (priv->type) {
>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
>  
> -        switch (type) {
> +        switch ((virLockManagerResourceType) type) {
>          case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>              if (params || nparams) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -670,6 +671,8 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>                  goto cleanup;
>  
>          }   break;
> +
> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:

I'm still conflicted with Unknown and Unsupported.

>          default:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("Unknown lock manager object type %d for domain lock object"),
> @@ -679,6 +682,29 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>          break;
>  
>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON:
> +        switch ((virLockManagerResourceType) type) {
> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
> +            if (params || nparams) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("Unexpected parameters for metadata resource"));
> +                goto cleanup;
> +            }
> +            if (VIR_STRDUP(newLockspace, "") < 0 ||
> +                VIR_STRDUP(newName, name) < 0)
> +                goto cleanup;
> +            newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA;
> +            break;
> +
> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE:

Again Unknown and Unsupported...

> +        default:
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unknown lock manager object type %d for daemon lock object"),
> +                           type);
> +            goto cleanup;
> +        }
> +        break;
> +
>      default:
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Unknown lock manager object type %d"),
> @@ -686,19 +712,18 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>          goto cleanup;
>      }
>  
> +    if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
> +        newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
> +
> +    if (autoCreate)

Interstingly enough, @newFlags is adjusted in the new case and we could
do the same in the existing case instead of setting @autoCreate, just
set the newFlags. Of course I'm quite aware that this could have been
done in a separate patch too. IOW: I could easily support removing
@autoCreate...

> +        newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE;
> +
>      if (VIR_EXPAND_N(priv->resources, priv->nresources, 1) < 0)
>          goto cleanup;
>  
>      VIR_STEAL_PTR(priv->resources[priv->nresources-1].lockspace, newLockspace);
>      VIR_STEAL_PTR(priv->resources[priv->nresources-1].name, newName);
> -
> -    if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
> -        priv->resources[priv->nresources-1].flags |=
> -            VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
> -
> -    if (autoCreate)
> -        priv->resources[priv->nresources-1].flags |=
> -            VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE;
> +    priv->resources[priv->nresources-1].flags = newFlags;
>  
>      ret = 0;
>   cleanup:
> diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
> index fe422d3be6..9393e7d9a2 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -815,7 +815,7 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
>      if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY)
>          return 0;
>  
> -    switch (type) {
> +    switch ((virLockManagerResourceType) type) {
>      case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>          if (driver->autoDiskLease) {
>              if (virLockManagerSanlockAddDisk(driver, lock, name, nparams, params,
> @@ -839,6 +839,7 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
>              return -1;
>          break;
>  
> +    case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:

Conflict continues.

>      default:
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Unknown lock manager object type %d for domain lock object"),
> 

So after all that - I guess I can accept that Unknown will be used and a
separate Unsupported isn't necessary.

However, I do think removing @autoCreate is worthwhile especially since
it's specific to TYPE_DOMAIN and TYPE_DISK. So with that, for the logic
at least regardless if the exact location changes as a result of motion
that may happen prior to this...

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

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 13/28] lock_driver: Introduce VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA
Posted by Michal Privoznik 7 years, 5 months ago
On 08/30/2018 11:34 PM, John Ferlan wrote:
> 
> 
> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>> This is a new type of object that lock drivers can handle.
>> Currently, it is supported by lockd driver only.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/locking/lock_driver.h         |  2 ++
>>  src/locking/lock_driver_lockd.c   | 43 +++++++++++++++++++++++++++++++--------
>>  src/locking/lock_driver_sanlock.c |  3 ++-
>>  3 files changed, 38 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
>> index a9d2041c30..9be0abcfba 100644
>> --- a/src/locking/lock_driver.h
>> +++ b/src/locking/lock_driver.h
>> @@ -51,6 +51,8 @@ typedef enum {
>>      VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0,
>>      /* A lease against an arbitrary resource */
>>      VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE = 1,
>> +    /* The resource to be locked is a metadata */
>> +    VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA = 2,
>>  } virLockManagerResourceType;
>>  
>>  typedef enum {
>> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
>> index 98953500b7..d7cb183d7a 100644
>> --- a/src/locking/lock_driver_lockd.c
>> +++ b/src/locking/lock_driver_lockd.c
>> @@ -557,6 +557,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>      virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
>>      char *newName = NULL;
>>      char *newLockspace = NULL;
>> +    int newFlags = 0;
>>      bool autoCreate = false;
>>      int ret = -1;
>>  
>> @@ -569,7 +570,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>      switch (priv->type) {
>>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
>>  
>> -        switch (type) {
>> +        switch ((virLockManagerResourceType) type) {
>>          case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>>              if (params || nparams) {
>>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> @@ -670,6 +671,8 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>                  goto cleanup;
>>  
>>          }   break;
>> +
>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
> 
> I'm still conflicted with Unknown and Unsupported.
> 
>>          default:

As I explain in one of my previous replies, users are not really
expected to see this message. Is merely for us to avoid broken code
pattern. Even if it so happens that broken code slips through review,
what difference does it make for users to see "Unsupported lock manager
object type" vs "Unknown lock manager object type"? They'll file a bug
and we will notice immediately what is the problem when looking into the
code (we will notice it because the error message logs type number).

>>              virReportError(VIR_ERR_INTERNAL_ERROR,
>>                             _("Unknown lock manager object type %d for domain lock object"),
>> @@ -679,6 +682,29 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>          break;
>>  
>>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON:
>> +        switch ((virLockManagerResourceType) type) {
>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
>> +            if (params || nparams) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("Unexpected parameters for metadata resource"));
>> +                goto cleanup;
>> +            }
>> +            if (VIR_STRDUP(newLockspace, "") < 0 ||
>> +                VIR_STRDUP(newName, name) < 0)
>> +                goto cleanup;
>> +            newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA;
>> +            break;
>> +
>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE:
> 
> Again Unknown and Unsupported...
> 
>> +        default:
>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                           _("Unknown lock manager object type %d for daemon lock object"),
>> +                           type);
>> +            goto cleanup;
>> +        }
>> +        break;
>> +
>>      default:
>>          virReportError(VIR_ERR_INTERNAL_ERROR,
>>                         _("Unknown lock manager object type %d"),
>> @@ -686,19 +712,18 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>          goto cleanup;
>>      }
>>  
>> +    if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
>> +        newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
>> +
>> +    if (autoCreate)
> 
> Interstingly enough, @newFlags is adjusted in the new case and we could
> do the same in the existing case instead of setting @autoCreate, just
> set the newFlags. Of course I'm quite aware that this could have been
> done in a separate patch too. IOW: I could easily support removing
> @autoCreate...

Okay.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 13/28] lock_driver: Introduce VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA
Posted by John Ferlan 7 years, 5 months ago

On 09/03/2018 11:13 AM, Michal Privoznik wrote:
> On 08/30/2018 11:34 PM, John Ferlan wrote:
>>
>>
>> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>>> This is a new type of object that lock drivers can handle.
>>> Currently, it is supported by lockd driver only.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>  src/locking/lock_driver.h         |  2 ++
>>>  src/locking/lock_driver_lockd.c   | 43 +++++++++++++++++++++++++++++++--------
>>>  src/locking/lock_driver_sanlock.c |  3 ++-
>>>  3 files changed, 38 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
>>> index a9d2041c30..9be0abcfba 100644
>>> --- a/src/locking/lock_driver.h
>>> +++ b/src/locking/lock_driver.h
>>> @@ -51,6 +51,8 @@ typedef enum {
>>>      VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0,
>>>      /* A lease against an arbitrary resource */
>>>      VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE = 1,
>>> +    /* The resource to be locked is a metadata */
>>> +    VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA = 2,
>>>  } virLockManagerResourceType;
>>>  
>>>  typedef enum {
>>> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
>>> index 98953500b7..d7cb183d7a 100644
>>> --- a/src/locking/lock_driver_lockd.c
>>> +++ b/src/locking/lock_driver_lockd.c
>>> @@ -557,6 +557,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>>      virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
>>>      char *newName = NULL;
>>>      char *newLockspace = NULL;
>>> +    int newFlags = 0;
>>>      bool autoCreate = false;
>>>      int ret = -1;
>>>  
>>> @@ -569,7 +570,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>>      switch (priv->type) {
>>>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
>>>  
>>> -        switch (type) {
>>> +        switch ((virLockManagerResourceType) type) {
>>>          case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>>>              if (params || nparams) {
>>>                  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> @@ -670,6 +671,8 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>>                  goto cleanup;
>>>  
>>>          }   break;
>>> +
>>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
>>
>> I'm still conflicted with Unknown and Unsupported.
>>
>>>          default:
> 
> As I explain in one of my previous replies, users are not really
> expected to see this message. Is merely for us to avoid broken code
> pattern. Even if it so happens that broken code slips through review,
> what difference does it make for users to see "Unsupported lock manager
> object type" vs "Unknown lock manager object type"? They'll file a bug
> and we will notice immediately what is the problem when looking into the
> code (we will notice it because the error message logs type number).
> 

Consider my your early warning bz then ;-).  It doesn't really matter
that much... Maybe it's more of an "Unsupported" message regardless of
whether it's META or 'default'. At first I considered noting the Enum
range error, but it's not the same here.

I'll leave it as a design decision and move on.

John

>>>              virReportError(VIR_ERR_INTERNAL_ERROR,
>>>                             _("Unknown lock manager object type %d for domain lock object"),
>>> @@ -679,6 +682,29 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>>          break;
>>>  
>>>      case VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON:
>>> +        switch ((virLockManagerResourceType) type) {
>>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
>>> +            if (params || nparams) {
>>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +                               _("Unexpected parameters for metadata resource"));
>>> +                goto cleanup;
>>> +            }
>>> +            if (VIR_STRDUP(newLockspace, "") < 0 ||
>>> +                VIR_STRDUP(newName, name) < 0)
>>> +                goto cleanup;
>>> +            newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA;
>>> +            break;
>>> +
>>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>>> +        case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE:
>>
>> Again Unknown and Unsupported...
>>
>>> +        default:
>>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>>> +                           _("Unknown lock manager object type %d for daemon lock object"),
>>> +                           type);
>>> +            goto cleanup;
>>> +        }
>>> +        break;
>>> +
>>>      default:
>>>          virReportError(VIR_ERR_INTERNAL_ERROR,
>>>                         _("Unknown lock manager object type %d"),
>>> @@ -686,19 +712,18 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>>          goto cleanup;
>>>      }
>>>  
>>> +    if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
>>> +        newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED;
>>> +
>>> +    if (autoCreate)
>>
>> Interstingly enough, @newFlags is adjusted in the new case and we could
>> do the same in the existing case instead of setting @autoCreate, just
>> set the newFlags. Of course I'm quite aware that this could have been
>> done in a separate patch too. IOW: I could easily support removing
>> @autoCreate...
> 
> Okay.
> 
> Michal
> 

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