Re: [libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

On 28.02.2012 21:14, Laine Stump wrote: > qemuDomainAttachNetDevice > > - re-ordered some things at start of function because > networkAllocateActualDevice should always be run and a slot > in def->nets always allocated, but host_net_add isn't needed > if the actual type is hostdev. > > - if actual type is hostdev, defer to > qemuDomainAttachHostDevice (which will reach up to the NetDef > for things like MAC address when necessary). After return > from qemuDomainAttachHostDevice, slip directly to cleanup, > since the rest of the function is specific to emulated net > devices. > > - put assignment of new NetDef into expanded def->nets down > below cleanup: (but only on success) since it is also needed > for emulated and hostdev net devices. > > qemuDomainDetachHostDevice > > - after locating the exact device to detach, check if it's a > network device and, if so, use toplevel > qemuDomainDetachNetDevice instead so that the def->nets list > is properly updated, and 'actual device' properly returned to > network pool if appropriate. Otherwise, for normal hostdevs, > call the lower level qemuDomainDetachThisDevice. > > qemuDomainDetachNetDevice > > - This is where it gets a bit tricky. After locating the device > on the def->nets list, if the network device type == hostdev, > call the *lower level* qemuDomainDetachThisDevice (which will > reach back up to the parent net device for MAC address / > virtualport when appropriate, then clear the device out of > def->hostdevs) before skipping past all the emulated > net-device-specific code to cleanup:, where the network > device is removed from def->nets, and the network device > object is freed. > > In short, any time a hostdev-type network device is detached, we must > go through the toplevel virDomaineDetachNetDevice function first and > last, to make sure 1) the def->nnets list is properly managed, and 2) > any device allocated with networkAllocateActualDevice is properly > freed. At the same time, in the middle we need to go through the > lower-level virDomainDetach*This*HostDevice to be sure that 1) the > def->hostdevs list is properly managed, 2) the PCI device is properly > detached from the guest and reattached to the host (if appropriate), > and 3) any higher level setup/teardown is called at the appropriate > time, by reaching back up to the NetDef config (part (3) will be > covered in a separate patch). > > --- > src/qemu/qemu_hotplug.c | 61 +++++++++++++++++++++++++++++++++------------- > 1 files changed, 44 insertions(+), 17 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 6119108..50563c5 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -661,9 +661,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > bool iface_connected = false; > int actualType; > > - if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_HOST_NET_ADD)) { > - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("installed qemu version does not support host_net_add")); > + /* preallocate new slot for device */ > + if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) { > + virReportOOMError(); > return -1; > } > > @@ -672,9 +672,27 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > * to the one defined in the network definition. > */ > if (networkAllocateActualDevice(net) < 0) > - goto cleanup; > + return -1; Okay, vm->def->nets won't leak, but will be one item bigger; Do we want to realloc it back as we do in all detach functions, e.g. virDomainNetRemove() ? The vm->def->nnets counter isn't changed yet so I guess this is alright.

Anyway, looking good so ACK.