On Fri, Mar 06, 2015 at 05:03:16PM +0100, Jiri Denemark wrote: > On Fri, Mar 06, 2015 at 16:58:22 +0100, Jiri Denemark wrote: > > Commit v1.2.4-52-gda879e5 fixed issues with domains started before > > sanlock driver was enabled by checking whether a running domain is > > registered with sanlock and if it's not, sanlock driver is basically > > ignored for the domain. > > > > However, it was checking this even for domain which has just been > > started and no sanlock_* API was called for them yet. This results in > > > > cmd 9 target pid 2135544 not found > > > > error messages to appear in sanlock.log whenever we start a new domain. > > > > This patch avoids this useless check for freshly started domains. > > > > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx> > > --- > > src/locking/domain_lock.c | 22 ++++++++++++---------- > > src/locking/lock_driver.h | 12 +++++++++++- > > src/locking/lock_driver_lockd.c | 2 +- > > src/locking/lock_driver_sanlock.c | 9 ++++++--- > > src/locking/lock_manager.c | 2 +- > > 5 files changed, 31 insertions(+), 16 deletions(-) > ... > > diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c > > index 8d184fe..72a4a0c 100644 > > --- a/src/locking/lock_driver_lockd.c > > +++ b/src/locking/lock_driver_lockd.c > > @@ -439,7 +439,7 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, > > virLockManagerLockDaemonPrivatePtr priv; > > size_t i; > > > > - virCheckFlags(VIR_LOCK_MANAGER_USES_STATE, -1); > > + virCheckFlags(VIR_LOCK_MANAGER_NEW_STARTED, -1); > > > > if (VIR_ALLOC(priv) < 0) > > return -1; > > Confused with this weird change? So am I :-) The > VIR_LOCK_MANAGER_USES_STATE should really have been 0 (as in the sanlock > driver) since virLockManagerNew is never called with such a flag. > > However, should VIR_LOCK_MANAGER_USES_STATE be set to > virLockDriverImpl.flags in src/locking/lock_driver_lockd.c, which should > be the only usage of this flag in any lock driver? Sanlock sets the flag > but should lock set it too or not? The "state" here is information about the locks held on the source host which may need to be transferred to the target host during migration. Sanlock uses this facility to transfer lease information that needs to be maintained for its disk paxos algorithm. The lockd impl has no state information that needs to be transferred, so it should never references the VIR_LOCK_MANAGER_USES_STATE flag at all. IOW your change here is fine - if I wanted to nitpick though I'd say you should remove VIR_LOCK_MANAGER_USES_STATE from this method in a separate patch to this, just to avoid confusing future reviewers. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list