[libvirt] [PATCH] xm_internal.c: fix locking bug: s/Unlock/Lock/
Merge error? I'll commit this in an hour or so.
From e660e3353b468cb219208a1aba9f8eca456467dd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 22 Jan 2009 20:08:55 +0100 Subject: [PATCH] xm_internal.c: fix locking bug: s/Unlock/Lock/
* src/xm_internal.c (xenXMDomainDefineXML): Release lock, (don't try to acquire it) upon failure, just before returning. --- src/xm_internal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index 31f56b7..b420e80 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2357,7 +2357,7 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { if (!(def = virDomainDefParseString(conn, priv->caps, xml, VIR_DOMAIN_XML_INACTIVE))) { - xenUnifiedLock(priv); + xenUnifiedUnlock(priv); return (NULL); } -- 1.6.1.399.g0d272
On Thu, Jan 22, 2009 at 08:09:51PM +0100, Jim Meyering wrote:
Merge error? I'll commit this in an hour or so.
Go for it - how did you notice it ? Just luck ? This is why I want to get my previous CIL locking test program integrated into the 'make check' somehow - or at least a separate 'make lockcheck' rule Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Thu, Jan 22, 2009 at 08:09:51PM +0100, Jim Meyering wrote:
Merge error? I'll commit this in an hour or so.
Go for it - how did you notice it ? Just luck ?
Ha! I wish. I noticed it while resolving conflicts induced as I've merged your initial 25-cset series with what's been committed.
This is why I want to get my previous CIL locking test program integrated into the 'make check' somehow - or at least a separate 'make lockcheck' rule
Yes, definitely needed.
On Thu, Jan 22, 2009 at 07:13:47PM +0000, Daniel P. Berrange wrote:
On Thu, Jan 22, 2009 at 08:09:51PM +0100, Jim Meyering wrote:
Merge error? I'll commit this in an hour or so.
Go for it - how did you notice it ? Just luck ?
This is why I want to get my previous CIL locking test program integrated into the 'make check' somehow - or at least a separate 'make lockcheck' rule
I'm getting a patch ready for this. A few things are blocking it at the moment. As I mentioned on IRC, there is a licensing problem with the OCaml autoconf macros: http://caml.inria.fr/pub/ml-archives/caml-list/2009/01/233b0823ddc93b99900d0... This should be resolved this week. The other thing is I found a few bugs in the code which I'm trying to understand. This CIL stuff is bloody complicated and obscure ... Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Mon, Jan 26, 2009 at 10:50:32AM +0000, Richard W.M. Jones wrote:
On Thu, Jan 22, 2009 at 07:13:47PM +0000, Daniel P. Berrange wrote:
On Thu, Jan 22, 2009 at 08:09:51PM +0100, Jim Meyering wrote:
Merge error? I'll commit this in an hour or so.
Go for it - how did you notice it ? Just luck ?
This is why I want to get my previous CIL locking test program integrated into the 'make check' somehow - or at least a separate 'make lockcheck' rule
I'm getting a patch ready for this.
A few things are blocking it at the moment. As I mentioned on IRC, there is a licensing problem with the OCaml autoconf macros:
http://caml.inria.fr/pub/ml-archives/caml-list/2009/01/233b0823ddc93b99900d0...
This should be resolved this week.
The other thing is I found a few bugs in the code which I'm trying to understand. This CIL stuff is bloody complicated and obscure ...
There are certainly some bugs in there - I'm far from convinced that I got for/while loop handling correctly implemented. It also does not track locks across variable assignments / aliasing. eg if you have virDomainObjPtr dom = NULL; virDomainObjLock(driver->domains[0]); dom = driver->domains[0]; ...do something with 'dom'.... It will complain that 'dom' is used while unlocked, because it does not track that 'driver->domains[0]' is the same as 'dom' in this context. It wouldn't be too much more work to make that work better, but I didn't bother at the time, because this scenario only occurred in one place in the libvirt code I was checking at the time. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones