On 05/12/2014 09:37 AM, Martin Kletzander wrote: > When a domain was started without registration in sanlock, but libvirt > was restarted after that, most of the operations failed due to > contacting sanlock about that process. E.g. migration could not be > performed because the locks couldn't be released (or inquired before a > release). > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1088034 > > Signed-off-by: Martin Kletzander <mklet...@redhat.com> > --- > src/locking/lock_driver_sanlock.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/src/locking/lock_driver_sanlock.c > b/src/locking/lock_driver_sanlock.c > index 01441a0..5bc72ba 100644 > --- a/src/locking/lock_driver_sanlock.c > +++ b/src/locking/lock_driver_sanlock.c > @@ -91,6 +91,9 @@ struct _virLockManagerSanlockPrivate { > bool hasRWDisks; > int res_count; > struct sanlk_resource *res_args[SANLK_MAX_RESOURCES]; > + > + /* whether the VM was registered or not */ > + bool registered; > }; > > /* > @@ -450,6 +453,7 @@ static int virLockManagerSanlockNew(virLockManagerPtr > lock, > virLockManagerParamPtr param; > virLockManagerSanlockPrivatePtr priv; > size_t i; > + int resCount = 0; > > virCheckFlags(0, -1); > > @@ -487,6 +491,16 @@ static int virLockManagerSanlockNew(virLockManagerPtr > lock, > } > } > > + /* Sanlock needs process registration, but the only way how to probe > + * whether a process has been registered is ti inquire the lock. If > + * sanlock_inquire() returns -ESRCH, then it is not registered, but > + * if it returns any other error (rv < 0), then we cannot fail due > + * to back-compat. So this whole call is non-fatal, because it's > + * called from all over the place (is will usually fail). It merely > + * updates privateData. */ > + if (sanlock_inquire(-1, priv->vm_pid, 0, &resCount, NULL) >= 0) > + priv->registered = true; > + > lock->privateData = priv; > return 0; > > @@ -915,6 +929,9 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr > lock, > goto error; > } > > + /* Mark the pid as registered */ > + priv->registered = true; > + > if (action != VIR_DOMAIN_LOCK_FAILURE_DEFAULT) { > char uuidstr[VIR_UUID_STRING_BUFLEN]; > virUUIDFormat(priv->vm_uuid, uuidstr); > @@ -922,6 +939,9 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr > lock, > uuidstr, action) < 0) > goto error; > } > + } else if (!priv->registered) { > + VIR_DEBUG("Process not registered, not acquiring lock"); > + return 0;
Coverity found an issue regarding 'opt' not being VIR_FREE()'d John > } > > /* sanlock doesn't use owner_name for anything, so it's safe to take just > @@ -1025,6 +1045,11 @@ static int > virLockManagerSanlockRelease(virLockManagerPtr lock, > > virCheckFlags(0, -1); > > + if (!priv->registered) { > + VIR_DEBUG("Process not registered, skipping release"); > + return 0; > + } > + > if (state) { > if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < > 0) { > if (rv <= -200) > @@ -1070,6 +1095,12 @@ static int > virLockManagerSanlockInquire(virLockManagerPtr lock, > > VIR_DEBUG("pid=%d", priv->vm_pid); > > + if (!priv->registered) { > + VIR_DEBUG("Process not registered, skipping inquiry"); > + VIR_FREE(*state); > + return 0; > + } > + > if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < 0) { > if (rv <= -200) > virReportError(VIR_ERR_INTERNAL_ERROR, > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list