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