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

[John Ferlan]

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
> @@ -5787,14 +5787,6 @@ qemuProcessLaunch(virConnectPtr conn,
>      if (qemuProcessSetLinkStates(driver, vm, asyncJob) < 0)
>          goto cleanup;
> 
> -    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("Setting initial memory amount");
>      if (qemuProcessSetupBalloon(driver, vm, asyncJob) < 0)
>          goto cleanup;
> @@ -5806,14 +5798,6 @@ qemuProcessLaunch(virConnectPtr conn,
>          qemuProcessRefreshBalloonState(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;
> -
>      if (flags & VIR_QEMU_PROCESS_START_AUTODESTROY &&
>          qemuProcessAutoDestroyAdd(driver, vm, conn) < 0)
>          goto cleanup;
> @@ -5831,6 +5815,46 @@ qemuProcessLaunch(virConnectPtr conn,
>  }
> 
> 
> +/**
> + * 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 at redhat.com>

John

> +}
> +
> +
>  /**
>   * qemuProcessFinishStartup:
>   *
> @@ -5847,6 +5871,9 @@ qemuProcessFinishStartup(virConnectPtr conn,
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      int ret = -1;
> 
> +    if (qemuProcessRefreshState(driver, vm, asyncJob) < 0)
> +        goto cleanup;
> +
>      if (startCPUs) {
>          VIR_DEBUG("Starting domain CPUs");
>          if (qemuProcessStartCPUs(driver, vm, conn,
>