<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div style="margin: 0;"><br></div><div style="margin: 0;">Thanks, This is my first fatch to libvirt project and there are some rules </div><div style="margin: 0;">I am not familar to.</div><div style="margin: 0;">I feel starnge that the comment, becasue the live-migration do the refresh</div><div style="margin: 0;">in <span style="font-family: arial; white-space: pre-wrap;">qemuMigrationDstFinish, but restore method is not called this function.</span></div><div style="margin: 0;"><span style="font-family: arial; white-space: pre-wrap;">And the live-migration in </span><font face="arial"><span style="white-space: pre-wrap;">qemuMigrationDstPrepareAny</span></font><span style="font-family: arial; white-space: pre-wrap;"> is not the</span></div><div style="margin: 0;"><span style="font-family: arial; white-space: pre-wrap;"> same function flow with the restore in </span><font face="arial"><span style="white-space: pre-wrap;">qemuDomainObjStart. I was wonder</span></font></div><div style="margin: 0;"><font face="arial"><span style="white-space: pre-wrap;">there some forget to the fcuntion </span></font><span style="font-family: arial; white-space: pre-wrap;">qemuProcessRefreshState.</span></div><p style="margin: 0;"><br></p><p style="margin: 0;"><br></p><p style="margin: 0;"><br></p><div style="position:relative;zoom:1"></div><div id="divNeteaseMailCard"></div><p style="margin: 0;"><br></p><pre><br>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
>> 
</pre></div><br><br><span title="neteasefooter"><p> </p></span>