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

[Michal Privoznik]

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 at redhat.com>
> ---
>
> 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

Michal