[PATCH 1/4] qemu_shim: Don't hang if failed to start domain

[Andrea Bolognani]

On Fri, 2021-03-12 at 15:24 +0100, Michal Privoznik wrote:
> On 3/12/21 11:42 AM, Andrea Bolognani wrote:
> > On Mon, 2021-03-01 at 12:49 +0100, Michal Privoznik wrote:
> > > +++ b/src/qemu/qemu_shim.c
> > > @@ -45,9 +45,12 @@ qemuShimEventLoop(void *opaque G_GNUC_UNUSED)
> > >       while (!quit) {
> > >           g_mutex_lock(&eventLock);
> > >           if (eventQuitFlag && !eventPreventQuitFlag) {
> > > +            quit = true;
> > >               if (dom) {
> > >                   virDomainDestroy(dom);
> > > -                quit = true;
> > > +            } else {
> > > +                g_mutex_unlock(&eventLock);
> > > +                break;
> > >               }
> > >           }
> > >           g_mutex_unlock(&eventLock);
> > 
> > I'm probably missing something obvious, but I thought this could be
> > simply
> > 
> >    while (!quit) {
> >        g_mutex_lock(&eventLock);
> >        if (eventQuitFlag && !eventPreventQuitFlag) {
> >            quit = true;
> >            if (dom) {
> >                virDomainDestroy(dom);
> >            }
> >        }
> >        g_mutex_unlock(&eventLock);
> >        virEventRunDefaultImpl();
> >    }
> > 
> > Do we specifically want to avoid call virEventRunDefaultImpl() one
> > last time if the domain has failed to start?
> 
> Huh, riddle me this - how come now this is sufficient, but back then 
> when I was writing this it wasn't?
> 
> diff --git i/src/qemu/qemu_shim.c w/src/qemu/qemu_shim.c
> index c10598df4b..5a9ffdd416 100644
> --- i/src/qemu/qemu_shim.c
> +++ w/src/qemu/qemu_shim.c
> @@ -45,9 +45,9 @@ qemuShimEventLoop(void *opaque G_GNUC_UNUSED)
>       while (!quit) {
>           g_mutex_lock(&eventLock);
>           if (eventQuitFlag && !eventPreventQuitFlag) {
> +            quit = true;
>               if (dom) {
>                   virDomainDestroy(dom);
> -                quit = true;
>               }
>           }
>           g_mutex_unlock(&eventLock);
> 
> Will post it as v2, soon.

I see v2 it in the archives[1], but it doesn't seem to have any
intention of reaching my inbox (is libvir-list broken again?) so I'll
just comment here.

Assuming you have verified that the rewritten code still addresses
the bug,

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

to v2.


[1] https://listman.redhat.com/archives/libvir-list/2021-March/msg00607.html
-- 
Andrea Bolognani / Red Hat / Virtualization