Re: [libvirt] [PATCH v3] qemu: avoid rare race when undefining domain

On 11.12.2014 11:48, Martin Kletzander wrote: > When one domain is being undefined and at the same time started, for > example, there is a possibility of a rare problem occuring. > > - Thread 1 does virDomainUndefine(), has the lock, checks that the > domain is active and because it's not, calls > virDomainObjListRemove(). > > - Thread 2 does virDomainCreate() and tries to lock the domain. > > - Thread 1 needs to lock domain list in order to remove the domain from > it, but must unlock domain first (proper order is to lock domain list > first and the domain itself second). > > - Thread 2 grabs the lock, starts the domain and releases the lock. > > - Thread 1 grabs the lock and removes the domain from list. > > With this patch: > > - The undefining domain gets marked as "to undefine" before it is > unlocked. > > - If domain is found in any of the search APIs, it's returned only if > it is not marked as "to undefine". The check is done while the > domain is locked. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150505 > > Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > --- > > Notes: > I marked this as v3, but it is basically a resend of v1 [1] with > s/undefining/removing/ change. I'm sending this even though the second > version of the patch got in again because the v2 that we pushed didn't > help. I got convinced [2] that doing this with a job fixes it, but it > doesn't, it just serializes the Create API after the Undefine API. The > only difference that the commit b629c64e [3] does is that it waits on > obtaining job, but not on locking the domain. That previous commit can > be reverted if anyone wants, I see almost no point in it being there > now. > > [1] http://www.redhat.com/archives/libvir-list/2014-October/msg01055.html > [2] http://www.redhat.com/archives/libvir-list/2014-October/msg01115.html > [3] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b629c64e > > src/conf/domain_conf.c | 22 +++++++++++++++++++--- > src/conf/domain_conf.h | 1 + > 2 files changed, 20 insertions(+), 3 deletions(-) ACK