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@xxxxxxxxxx>
to v2.
[1] https://listman.redhat.com/archives/libvir-list/2021-March/msg00607.html
--
Andrea Bolognani / Red Hat / Virtualization
