[libvirt] [PATCH v3 12/28] _virLockManagerLockDaemonPrivate: Move @hasRWDisks into dom union

[Michal Privoznik]

On 08/30/2018 11:14 PM, John Ferlan wrote:
> 
> 
> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>> The fact whether domain has or hasn't RW disks is specific to
> 
> "or doesn't have"
> 
>> VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN and therefore should reside
>> in union specific to it.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/locking/lock_driver_lockd.c | 187 +++++++++++++++++++++-------------------
>>  1 file changed, 100 insertions(+), 87 deletions(-)
>>
> 
> This patch does a bit more than advertised...
> 
>> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
>> index 8ca0cf5426..98953500b7 100644
>> --- a/src/locking/lock_driver_lockd.c
>> +++ b/src/locking/lock_driver_lockd.c
>> @@ -63,6 +63,8 @@ struct _virLockManagerLockDaemonPrivate {
>>              char *name;
>>              int id;
>>              pid_t pid;
>> +
>> +            bool hasRWDisks;
>>          } dom;
>>  
>>          struct {
>> @@ -74,8 +76,6 @@ struct _virLockManagerLockDaemonPrivate {
>>  
>>      size_t nresources;
>>      virLockManagerLockDaemonResourcePtr resources;
>> -
>> -    bool hasRWDisks;
>>  };
> 
> From the aspect of @dom vs @daemon union, moving @hasRWDisks still has
> no bearing other than classifying it as a @dom type resource which is
> fine, don't get me wrong on this - I'm just trying to go one patch at a
> time here.

Yes. Because of the 'namespacing' I described in reply to previous
patch, hasRWDisks has to go into domain related union. IOW, hasRWDisks
has no meaning for DAEMON type of lock.

> 
>>  
>>  
>> @@ -566,107 +566,119 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>      if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY)
>>          return 0;
>>  
>> -    switch (type) {
>> -    case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>> -        if (params || nparams) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                           _("Unexpected parameters for disk resource"));
>> -            goto cleanup;
>> -        }
>> -        if (!driver->autoDiskLease) {
>> -            if (!(flags & (VIR_LOCK_MANAGER_RESOURCE_SHARED |
>> -                           VIR_LOCK_MANAGER_RESOURCE_READONLY)))
>> -                priv->hasRWDisks = true;
>> -            return 0;
>> -        }
>> +    switch (priv->type) {
>> +    case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
> 
> Hmm. Why wasn't this done in the previous patch?

Okay, I'll move it there.

Michal