[libvirt] [PATCH 1/3] qemu: always call qemuInterfaceStartDevices() when starting CPUs

[Matthew Rosato]

On 12/12/2014 11:34 AM, Laine Stump wrote:
> The patch that added qemuInterfaceStartDevices() (upstream commit
> 82977058f5b1d143a355079900029e9cbfee2fe4) had an extra conditional to
> prevent calling it if the reason for starting the CPUs was
> VIR_DOMAIN_RUNNING_UNPAUSED or VIR_DOMAIN_RUNNING_SAVE_CANCELED.  This
> was put in by the author as the result of a reviewer asking if it was
> necessary to ifup the interfaces in *all* occasions (because these
> were the two cases where the CPU would have already been started (and
> stopped) once, so the interface would already be ifup'ed).
> 
> It turns out that, as long as there is no corresponding
> qemuInterfaceStopDevices() to ifdown the interfaces anytime the CPUs
> are stopped, neglecting to ifup when reason is RUNNING_UNPAUSED or
> RUNNING_SAVE_CANCELED doesn't cause any problems (because it just
> happens that the interface will have already been ifup'ed by a prior
> call when the CPU was previously started for some other reason).
> 
> However, it also doesn't *help*, and there will soon be a
> qemuInterfaceStopDevices() function which *will* ifdown these
> interfaces when the guest CPUs are stopped, and once that is done, the
> interfaces will be left down in some cases when they should be up (for
> example, if a domain is paused and then unpaused).
> 
> So, this patch is removing the condition in favor of always calling
> qemuInterfaeStartDevices() when the guest CPUs are started.
> 
> This patch (and the aforementioned patch) resolve:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1081461
> ---
>  src/qemu/qemu_process.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index ab4df9b..0028283 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3126,9 +3126,7 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm,
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> 
>      /* Bring up netdevs before starting CPUs */
> -    if (reason != VIR_DOMAIN_RUNNING_UNPAUSED &&
> -        reason != VIR_DOMAIN_RUNNING_SAVE_CANCELED &&
> -        qemuInterfaceStartDevices(vm->def) < 0)
> +    if (qemuInterfaceStartDevices(vm->def) < 0)
>         goto cleanup;
> 
>      VIR_DEBUG("Using lock state '%s'", NULLSTR(priv->lockState));
> 

I agreed to this in a separate thread & code looks good so:

Reviewed by: Matthew Rosato <mjrosato at linux.vnet.ibm.com>