Re: [libvirt] [PATCH v3 20/36] network: convert networkReleaseActualDevice to virNetworkPortDef

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: > Convert the virDomainNetDef object into a virNetworkPortDef object > at the start of networkReleaseActualDevice. This largely decouples > the method impl from the domain object type. > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/network/bridge_driver.c | 137 +++++++++++++++--------------------- > 1 file changed, 56 insertions(+), 81 deletions(-)

> @@ -5041,26 +5014,30 @@ networkReleaseActualDevice(virNetworkPtr net, > _("network '%s' doesn't have " > "PCI device %04x:%02x:%02x.%x in use by domain"), > netdef->name, > - hostdev->source.subsys.u.pci.addr.domain, > - hostdev->source.subsys.u.pci.addr.bus, > - hostdev->source.subsys.u.pci.addr.slot, > - hostdev->source.subsys.u.pci.addr.function); > - goto error; > + port->plug.hostdevpci.addr.domain, > + port->plug.hostdevpci.addr.bus, > + port->plug.hostdevpci.addr.slot, > + port->plug.hostdevpci.addr.function); > + goto cleanup; > } > + break; > + > + case VIR_NETWORK_PORT_PLUG_TYPE_LAST: > + default: > + virReportEnumRangeError(virNetworkPortPlugType, port->plugtype); > + goto cleanup; > } > - success: > virNetworkObjMacMgrDel(obj, driver->dnsmasqStateDir, dom->name, &iface->mac); Don't you want to change this ^^ to "&port->mac"?

> - if (iface->data.network.actual) { > - netdef->connections--; > - if (dev) > - dev->connections--; > - /* finally we can call the 'unplugged' hook script if any */ > - networkRunHook(obj, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, > - VIR_HOOK_SUBOP_BEGIN); > - networkLogAllocation(netdef, dev, &iface->mac, false); > - } > + netdef->connections--; > + if (dev) > + dev->connections--; > + /* finally we can call the 'unplugged' hook script if any */ > + networkRunHook(obj, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, > + VIR_HOOK_SUBOP_BEGIN); > + networkLogAllocation(netdef, dev, &iface->mac, false); Same with this ^^ Reviewed-by: Laine Stump <laine(a)laine.org&gt; anyway, because whether or not you intended to completely eliminate all references to iface during this patch, I know it ends up that way in the end anyway :-)