Re: [libvirt] [PATCH] qemu: Refresh state before starting the VCPUs

On Mon, Feb 04, 2019 at 02:37 PM +0100, Marc Hartmayer <mhartmay(a)linux.ibm.com&gt; wrote: > On Mon, Feb 04, 2019 at 01:41 PM +0100, Peter Krempa <pkrempa(a)redhat.com&gt; wrote: >> On Mon, Feb 04, 2019 at 13:36:24 +0100, Marc Hartmayer wrote: >>> For normal starts (no incoming migration) the refresh of the QEMU >>> state must be done before the VCPUs getting started since otherwise >>> there might be a race condition between a possible shutdown of the >>> guest OS and the QEMU monitor queries. >>> >>> This fixes "qemu: migration: Refresh device information after >>> transferring state" (93db7eea1b864). >>> >>> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com&gt; >>> --- >>> src/qemu/qemu_process.c | 20 +++++++++++--------- >>> 1 file changed, 11 insertions(+), 9 deletions(-) >>> >>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >>> index dace5aaca102..2a3763f40d49 100644 >>> --- a/src/qemu/qemu_process.c >>> +++ b/src/qemu/qemu_process.c >>> @@ -6929,10 +6929,17 @@ qemuProcessStart(virConnectPtr conn, >>> } >>> relabel = true; >>> >>> - if (incoming && >>> - incoming->deferredURI && >>> - qemuMigrationDstRun(driver, vm, incoming->deferredURI, asyncJob) < 0) >>> - goto stop; >>> + if (incoming) { >>> + if (incoming->deferredURI && >>> + qemuMigrationDstRun(driver, vm, incoming->deferredURI, asyncJob) < 0) >>> + goto stop; >>> + } else { >> >> This logic does not seem right ... > > I’m not familiar with the usage of this function for migration… so > there's a good chance I'm missing something. > >> >>> + /* Refresh state of devices from QEMU. During migration this >>> + * needs to happen after the state information is fully >>> + * transferred. */ >> >> as this comment clearly states that this should happen after >> migration. > > Is qemuProcessFinishStartup ^^^^^^^^^^^^^^^^^^^^^^^^ qemuProcessRefreshState > not called explicitly in qemuMigrationFinish > for migration? >

>> >> Here it would happen only when migration is not done. > > Without this patch qemuProcessRefreshState is called after > qemuProcessFinishStartup if (!incoming). Now it’s called directly before > qemuProcessFinishStartup if (!incoming). Why should this be wrong now? > What happens in qemuProcessFinishStartup? > > Before your change in 93db7eea1b86408e the function call > 'qemuProcessRefreshState' was done in qemuProcessFinishStartup (without > any condition).