Thanks, This is my first fatch to libvirt project and there are some rules
I am not familar to.
I feel starnge that the comment, becasue the live-migration do the refresh
in qemuMigrationDstFinish, but restore method is not called this function.
And the live-migration in qemuMigrationDstPrepareAny is not the
same function flow with the restore in qemuDomainObjStart. I was wonder
there some forget to the fcuntion qemuProcessRefreshState.
At 2021-12-10 19:37:24, "Peter Krempa" <pkrempa@redhat.com> wrote:
>On Fri, Dec 10, 2021 at 11:26:18 +0800, JorhsonDeng wrote:
>> To resolve the bug: #253.
>>
>> The restore method should call the qemuProcessRefreshState method
>> to refreash the state of the devices.
>
>Please also mention that this happens when restoring a VM from a save
>image, as it's not clear from the commit message itself.
>
>In general the commit message must be a standalone source of information
>describing what's happening. This also means that the summary line
>should be more descriptive.
>
>
>Note that the libvirt project requires that contributors declare
>conformance with the Developer certificate of origin:
>
>https://www.libvirt.org/hacking.html#developer-certificate-of-origin
>
>>
>> ---
>> src/qemu/qemu_process.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 6b83a571b9..ebd60a7b84 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -7703,14 +7703,11 @@ qemuProcessStart(virConnectPtr conn,
>> if (incoming->deferredURI &&
>> qemuMigrationDstRun(driver, vm, incoming->deferredURI, asyncJob) < 0)
>> goto stop;
>> - } else {
>> - /* Refresh state of devices from QEMU. During migration this happens
>> - * in qemuMigrationDstFinish to ensure that state information is fully
>> - * transferred. */
>> - if (qemuProcessRefreshState(driver, vm, asyncJob) < 0)
>> - goto stop;
>
>
>So you are removing a comment which says that for migration this is
>happening elsewhere without any justification or fix to the code.
>
>The refresh in case of migration is happening in a different place for a
>very good reason so we must keep it that way. This means you'll have to
>introduce some form of logic which refreshes the state only when
>restoring a save image.
>
>> }
>>
>> + if (qemuProcessRefreshState(driver, vm, asyncJob) < 0)
>> + goto stop;
>> +
>> if (qemuProcessFinishStartup(driver, vm, asyncJob,
>> !(flags & VIR_QEMU_PROCESS_START_PAUSED),
>> incoming ?
>> --
>> 2.27.0
>>