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