On 07/05/2011 01:45 AM, Laine Stump wrote:
> The network driver needs to assign physical devices for use by modes
> that use macvtap, keeping track of which physical devices are in use
> (and how many instances, when the devices can be shared). Three calls
> are added:
>
> networkAllocateActualDevice - finds a physical device for use by the
> domain, and sets up the virDomainActualNetDef accordingly.
>
> networkNotifyActualDevice - assumes that the domain was already
> running, but libvirtd was restarted, and needs to be notified by each
> already-running domain about what interfaces they are using.
>
> networkReleaseActualDevice - decrements the usage count of the
> allocated physical device, and frees the virDomainActualNetDef to
> avoid later accidentally using the device.
>
> bridge_driver.[hc] - the new APIs
>
> qemu_(command|driver|hotplug|process).c - add calls to the above APIs
> in the appropriate places.
>
> tests/Makefile.am - need to include libvirt_driver_network.la whenever
> libvirt_driver_qemu.la is linked, to avoid unreferenced symbols
> (in functions that are never called by the test programs...)
> ---
> src/network/bridge_driver.c | 398 +++++++++++++++++++++++++++++++++++++++++++
> src/network/bridge_driver.h | 6 +
> src/qemu/qemu_command.c | 11 ++
> src/qemu/qemu_hotplug.c | 41 +++--
> src/qemu/qemu_process.c | 26 +++-
> tests/Makefile.am | 12 +-
> 6 files changed, 476 insertions(+), 18 deletions(-)
> +
> + /* Find the most specific virtportprofile and copy it */
> + if (iface->data.network.virtPortProfile) {
> + virtport = iface->data.network.virtPortProfile;
> + } else {
> + virPortGroupDefPtr portgroup
> + = virPortGroupFindByName(netdef, iface->data.network.portgroup);
> + if (portgroup)
> + virtport = portgroup->virtPortProfile;
Nit: Indentation looks interesting there - the line starting with = has
one less space. Perhaps something to do with how your preferred editor
splits assignments. My editor uses 4 spaces instead of 3 in that instance.
> + if (!netdef->forwardDev) {
> + networkReportError(VIR_ERR_INTERNAL_ERROR,
> + _("network '%s' uses a direct mode, but has no forward dev and no interface pool"),
> + netdef->name);
> + goto cleanup;
> + }
> + iface->data.network.actual->data.direct.linkdev
> + = strdup(netdef->forwardDev);
And here you used a multiple of four spaces.
> +int
> +networkNotifyActualDevice(virDomainNetDefPtr iface)
> +{
> +
> + /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
> + * exclusive access to a device, so current usageCount must be
> + * 0 in those cases.
> + */
> + if ((dev->usageCount > 0) &&
> + ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) ||
> + ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
> + iface->data.network.actual->data.direct.virtPortProfile &&
> + (iface->data.network.actual->data.direct.virtPortProfile->virtPortType
> + == VIR_VIRTUALPORT_8021QBH)))) {
> + networkReportError(VIR_ERR_INTERNAL_ERROR,
> + _("network '%s' claims dev='%s' is already in use by a different domain"),
Do we have a data race here?
Suppose that libvirtd is restarted while one VM is already using device
0. Is there any chance that if I'm fast enough, I can cause the
creation of a second domain, and that second domain will go to pick from
the interface pool before the notification pass of the libvirtd restart
has completed, and mistakenly claim device 0?
That is, I think we need to somehow guarantee (if we don't already) that
no new domains can be created until after all existing domains have been
reloaded on a libvirtd restart.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list