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@xxxxxxxxxx> 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 >>
