On Mon, Feb 04, 2019 at 02:37 PM +0100, Marc Hartmayer <mhartmay@xxxxxxxxxxxxx> wrote:
> On Mon, Feb 04, 2019 at 01:41 PM +0100, Peter Krempa <pkrempa@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
>>> ---
>>> 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).
>
> Thanks.
>
>>
>>
>>> + if (qemuProcessRefreshState(driver, vm, asyncJob) < 0)
>>> + goto stop;
>>> + }
>>>
>>> if (qemuProcessFinishStartup(driver, vm, asyncJob,
>>> !(flags & VIR_QEMU_PROCESS_START_PAUSED),
>>> @@ -6945,11 +6952,6 @@ qemuProcessStart(virConnectPtr conn,
>>> /* Keep watching qemu log for errors during incoming migration, otherwise
>>> * unset reporting errors from qemu log. */
>>> qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);
>>> -
>>> - /* Refresh state of devices from qemu. During migration this needs to
>>> - * happen after the state information is fully transferred. */
>>> - if (qemuProcessRefreshState(driver, vm, asyncJob) < 0)
>>> - goto stop;
>>> }
>>>
>>> ret = 0;
>>> --
>>> 2.17.0
>>>
> --
> Kind regards / Beste Grüße
> Marc Hartmayer
>
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Matthias Hartmann
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
