[libvirt] [PATCH] qemu: allocate network connections sooner during domain startup

[Laine Stump]

On 05/07/2013 05:52 AM, Michal Privoznik wrote:
> On 06.05.2013 22:16, Laine Stump wrote:
>>  
> I'd expect a one line comment here at least to give a reason why we are
> skipping VIR_DOMAIN_NET_TYPE_HOSTDEV. Something like:
> /* hostdev interfaces already handled by qemuNetworkPrepareDevices */
>
> It's obvious now, but if we get later to this code it won't be IMO.


Makes sense. The dual nature of these device does tend to be confusing.



>> +            if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV)
>> +                continue;
>> +
>>              /* VLANs are not used with -netdev, so don't record them */
>>              if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
>>                  virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
> I actually posted a patch which moves the whole block into a separate
> function, which is what we should do:
>
> https://www.redhat.com/archives/libvir-list/2013-April/msg01550.html


Ah. I missed that whole series while I was busy trying to get VFIO
working before the 1.0.5 freeze. I should go back now and review it.



>> +    /* network devices must be "prepared" before hostdevs, because
>> +     * setting up a network device might create a new hostdev that
>> +     * will need to be setup.
>> +     */
>> +    VIR_DEBUG("Preparing network devices");
> Is it worth mentioning s/network/network host/?


My intent is the qemuNetworkPrepareDevices() will eventually handle the
setup required for *all* guest network devices, not just those that are
passed-through host devices. The only reason I didn't do it that was
right away is because the fd's for tap and vhost-net devices are
required in qemuBuildCommandLine(), so we need to decide on the best
place to stow those away where qemuBuildCommandLine() can retrieve them.


>> +    if (qemuNetworkPrepareDevices(vm->def) < 0)
>> +       goto cleanup;
>> +
>>      /* Must be run before security labelling */
>>      VIR_DEBUG("Preparing host devices");
>>      if (qemuPrepareHostDevices(driver, vm->def, !migrateFrom) < 0)
>>
> ACK if you add the comment. The debug message fix is optional.


Okay. I'm going to add the comment. I think the debug message is proper
as-is, for the reason I give above.

Thanks for reviewing!