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

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(a)redhat.com&gt; > --- > 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.

> > > @@ -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?