Re: [libvirt] [PATCH v3] network: Bring netdevs online later

On 12/11/2014 01:35 PM, Laine Stump wrote: > On 09/16/2014 04:50 PM, Matthew Rosato wrote: >> >> #include "cpu/cpu.h" >> #include "datatypes.h" >> @@ -2947,6 +2948,12 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, >> qemuDomainObjPrivatePtr priv = vm->privateData; >> 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); >> + } > Matthew, > > > I've already pushed your patch, but am trying to add to it for another > related purpose and this bit is bothering me. What is the reason for not > calling qemuInterfaceStartDevices() when reason is > VIR_DOMAIN_RUNNING_UNPAUSED or VIR_DOMAIN_RUNNING_SAVE_CANCELED? Is it > just to avoid setting IFF_UP on an interface that is already IFF_UP? > > If that's the only reason, I would prefer to have it *always* called > when the CPUs are started (and a corresponding > qemuInterfaceStopDevices() called when the CPUs are stopped). Otherwise, > it looks to me like it is possible in some situations (migration error > recovery perhaps? search for VIR_DOMAIN_RUNNING_UNPAUSED) to have the > CPUs running, but the macvtap interfaces *not* IFF_UP. > > Do you see (or did you experience?) a problem calling > qemuInterfaceStartDevices() for all reasons? > Hi Laine, I did not experience any problems calling qemuInterfaceStartDevices() unconditionally, that's actually how I originally wrote the code -- I added these conditional statements based on a review comment to avoid unnecessary IFF_UPs. I'd be fine with the call being unconditional again.