Re: [libvirt] [PATCH] qemu: process: Refresh data from qemu monitor after migration

On 09/25/2017 10:25 AM, Peter Krempa wrote: > Some values we read from the qemu monitor may be changed with the actual > state by the incomming migration. This means that we should refresh s/incomming/incoming > certain things only after the migration has finished. > > This is mostly visible in the cdrom tray state, which is by default > closed but may be opened by the guest OS. This would be refreshed before > qemu transferred the actual state and thus libvirt would think that the > tray is closed. > > Note that this patch moves only a few obvious query commands. Other may > be moved later after individual assesment. > s/Other/Others s/assesment/assessment > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1463168 > --- > src/qemu/qemu_process.c | 59 +++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 43 insertions(+), 16 deletions(-) > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index c104985aa..2be82e958 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c

> > +/** > + * qemuProcessRefreshState: > + * @driver: qemu driver data > + * @vm: domain to refresh > + * @asyncJob: async job type > + * > + * This function gathers calls to refresh qemu state after startup. This > + * function is called after a deferred migration finishes so that we can update > + * state influenced by the migration stream. Would it be useful to place a caveat here of things to "consider" before blindly moving something from Launch to here? Buyer beware... Something that perhaps qemuMigrationRunIncoming may assume is already done... I know it's pretty low level so I cannot think of something right now, but perhaps while it's fresh in your mind...

> + */ > +static int > +qemuProcessRefreshState(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + qemuDomainAsyncJob asyncJob) > +{ > + int ret = -1; > + > + VIR_DEBUG("Fetching list of active devices"); > + if (qemuDomainUpdateDeviceList(driver, vm, asyncJob) < 0) > + goto cleanup; > + > + VIR_DEBUG("Updating info of memory devices"); > + if (qemuDomainUpdateMemoryDeviceInfo(driver, vm, asyncJob) < 0) > + goto cleanup; > + > + VIR_DEBUG("Detecting actual memory size for video device"); > + if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) > + goto cleanup; > + > + VIR_DEBUG("Updating disk data"); > + if (qemuProcessRefreshDisks(driver, vm, asyncJob) < 0) > + goto cleanup; > + > + ret = 0; > + > + cleanup: > + return ret; Nothing to clean up.... all those goto cleanup's should just be return -1; instead and the @ret unnecessary.

Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt;