Okay so I needed to do some studying to understand what's going on in the first part of this series. Just gonna type some notes here: virDomainActualNetDef tracks all the data we need to convert a virNetworkPtr content to a virDomainNetDef <source>. It's only ever filled in for <interface type='network'> when a domain is running. hypervisor drivers fill the data in at VM startup by calling virDomainNetAllocateActualDevice, which hands the DomainNetDefPtr off to bridge_drive.c callbacks, which serialize the net config into virDomainActualNetDef. One desired end goal of this series is making sure that bridge_driver is not messing with any Domain*Def stuff. The first 10-15 patches here are unwinding some pieces of that before it gets into the real new stuff. On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: For this and the following patch I don't really follow why we can't put the iface->type == TYPE_NETWORK and the netconn = virGetConnectNetwork() directly into the domain_conf.c bridge_driver callback wrappers like virDomainNetAllocateActualDevice. After the final patch in this series, the open coded netconn lookup and TYPE_NETWORK checks are still there, and the function signature hasn't changed. Am I missing something? By dropping the validate label, netdef is always set at this point in the code, so this makes sense. This diff is ugly but it's just unindenting an if (netdef) block So by no longer calling this function on != TYPE_NETWORK, we no longer get the bandwidth and vlan option validation for those interfaces. That seems like a minor regression. IMO not enough to hold up applying this but we should remember it for later. I'm pretty sure it can solved with some similar checks added to DomainValidate callbacks Provided there's a reason for not centralizing the TYPE_NETWORK checks: Reviewed-by: Cole Robinson <crobinso(a)redhat.com&gt; And this could go in now IMO Thanks, Cole

The APIs for allocating/notifying/removing network ports just take an internal domain interface struct right now. As a step towards turning these into public facing APIs, add a virNetworkPtr argument to all of them. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 40 ++++++++++++++++++++---- src/conf/domain_conf.h | 18 +++++++---- src/libxl/libxl_domain.c | 30 +++++++++++++----- src/libxl/libxl_driver.c | 26 +++++++++++----- src/lxc/lxc_driver.c | 24 +++++++++++--- src/lxc/lxc_process.c | 24 +++++++++----- src/network/bridge_driver.c | 54 ++++++++++++++++++-------------- src/qemu/qemu_hotplug.c | 62 +++++++++++++++++++++++++++---------- src/qemu/qemu_process.c | 30 +++++++++++++----- 9 files changed, 223 insertions(+), 85 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b7bb94b90b..22c6fe1e38 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30184,37 +30184,65 @@ virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, } int -virDomainNetAllocateActualDevice(virDomainDefPtr dom, +virDomainNetAllocateActualDevice(virConnectPtr conn, + virDomainDefPtr dom, virDomainNetDefPtr iface) { + virNetworkPtr net = NULL; + int ret = -1; + if (!netAllocate) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("Virtual networking driver is not available")); return -1; } - return netAllocate(dom, iface); + if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) + return -1; + + ret = netAllocate(net, dom, iface); + + virObjectUnref(net); + return ret; } void -virDomainNetNotifyActualDevice(virDomainDefPtr dom, +virDomainNetNotifyActualDevice(virConnectPtr conn, + virDomainDefPtr dom, virDomainNetDefPtr iface) { + virNetworkPtr net = NULL; + if (!netNotify) return; - netNotify(dom, iface); + if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) + return; + + netNotify(net, dom, iface); + + virObjectUnref(net); } int -virDomainNetReleaseActualDevice(virDomainDefPtr dom, +virDomainNetReleaseActualDevice(virConnectPtr conn, + virDomainDefPtr dom, virDomainNetDefPtr iface) { + virNetworkPtr net = NULL; + int ret; + if (!netRelease) return 0; - return netRelease(dom, iface); + if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) + return -1; + + ret = netRelease(net, dom, iface); + + virObjectUnref(net); + return ret; } bool diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 538fb50b9e..e77fdcabf3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3505,15 +3505,18 @@ virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action); typedef int -(*virDomainNetAllocateActualDeviceImpl)(virDomainDefPtr dom, +(*virDomainNetAllocateActualDeviceImpl)(virNetworkPtr net, + virDomainDefPtr dom, virDomainNetDefPtr iface); typedef void -(*virDomainNetNotifyActualDeviceImpl)(virDomainDefPtr dom, +(*virDomainNetNotifyActualDeviceImpl)(virNetworkPtr net, + virDomainDefPtr dom, virDomainNetDefPtr iface); typedef int -(*virDomainNetReleaseActualDeviceImpl)(virDomainDefPtr dom, +(*virDomainNetReleaseActualDeviceImpl)(virNetworkPtr net, + virDomainDefPtr dom, virDomainNetDefPtr iface); typedef bool @@ -3533,17 +3536,20 @@ virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetBandwidthUpdateImpl bandwidthUpdate); int -virDomainNetAllocateActualDevice(virDomainDefPtr dom, +virDomainNetAllocateActualDevice(virConnectPtr conn, + virDomainDefPtr dom, virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void -virDomainNetNotifyActualDevice(virDomainDefPtr dom, +virDomainNetNotifyActualDevice(virConnectPtr conn, + virDomainDefPtr dom, virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int -virDomainNetReleaseActualDevice(virDomainDefPtr dom, +virDomainNetReleaseActualDevice(virConnectPtr conn, + virDomainDefPtr dom, virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index e2f1b54210..91b34feca0 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -35,6 +35,7 @@ #include "virtime.h" #include "locking/domain_lock.h" #include "xen_common.h" +#include "driver.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -844,6 +845,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, char *file; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI; + virConnectPtr conn = NULL; #ifdef LIBXL_HAVE_PVUSB hostdev_flags |= VIR_HOSTDEV_SP_USB; @@ -903,8 +905,12 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, /* cleanup actual device */ virDomainNetRemoveHostdev(vm->def, net); - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (conn || (conn = virGetConnectNetwork())) + virDomainNetReleaseActualDevice(conn, vm->def, net); + else + VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); + } } } @@ -927,6 +933,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virDomainObjRemoveTransientDef(vm); virObjectUnref(cfg); + virObjectUnref(conn); } /* @@ -1052,6 +1059,8 @@ static int libxlNetworkPrepareDevices(virDomainDefPtr def) { size_t i; + virConnectPtr conn = NULL; + int ret = -1; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; @@ -1061,9 +1070,12 @@ libxlNetworkPrepareDevices(virDomainDefPtr def) * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(def, net) < 0) - return -1; + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!conn && !(conn = virGetConnectNetwork())) + goto cleanup; + if (virDomainNetAllocateActualDevice(conn, def, net) < 0) + goto cleanup; + } actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && @@ -1083,10 +1095,14 @@ libxlNetworkPrepareDevices(virDomainDefPtr def) pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; if (virDomainHostdevInsert(def, hostdev) < 0) - return -1; + goto cleanup; } } - return 0; + + ret = 0; + cleanup: + virObjectUnref(conn); + return ret; } static void diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d5cd3fc834..506dcdfbeb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3379,6 +3379,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, libxl_device_nic nic; int ret = -1; char mac[VIR_MAC_STRING_BUFLEN]; + virConnectPtr conn = NULL; libxl_device_nic_init(&nic); @@ -3390,9 +3391,12 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(vm->def, net) < 0) - goto cleanup; + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!(conn = virGetConnectNetwork())) + goto cleanup; + if (virDomainNetAllocateActualDevice(conn, vm->def, net) < 0) + goto cleanup; + } actualType = virDomainNetGetActualType(net); @@ -3441,9 +3445,10 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, vm->def->nets[vm->def->nnets++] = net; } else { virDomainNetRemoveHostdev(vm->def, net); - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && conn) + virDomainNetReleaseActualDevice(conn, vm->def, net); } + virObjectUnref(conn); virObjectUnref(cfg); return ret; } @@ -3865,8 +3870,15 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, cleanup: libxl_device_nic_dispose(&nic); if (!ret) { - if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, detach); + if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virConnectPtr conn = virGetConnectNetwork(); + if (conn) { + virDomainNetReleaseActualDevice(conn, vm->def, detach); + virObjectUnref(conn); + } else { + VIR_WARN("Unable to release network device '%s'", NULLSTR(detach->ifname)); + } + } virDomainNetRemove(vm->def, detachidx); } virObjectUnref(cfg); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 027c4fd990..a184e67e15 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3834,9 +3834,16 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(vm->def, net) < 0) - return -1; + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virConnectPtr netconn = virGetConnectNetwork(); + if (!netconn) + return -1; + if (virDomainNetAllocateActualDevice(netconn, vm->def, net) < 0) { + virObjectUnref(netconn); + return -1; + } + virObjectUnref(netconn); + } actualType = virDomainNetGetActualType(net); @@ -4389,8 +4396,15 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, ret = 0; cleanup: if (!ret) { - if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, detach); + if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virConnectPtr conn = virGetConnectNetwork(); + if (conn) { + virDomainNetReleaseActualDevice(conn, vm->def, detach); + virObjectUnref(conn); + } else { + VIR_WARN("Unable to release network device '%s'", NULLSTR(detach->ifname)); + } + } virDomainNetRemove(vm->def, detachidx); virDomainNetDefFree(detach); } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 7849aaf5b9..d9362c5ff6 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -165,6 +165,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, virLXCDomainObjPrivatePtr priv = vm->privateData; virNetDevVPortProfilePtr vport = NULL; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); + virConnectPtr conn = NULL; VIR_DEBUG("Cleanup VM name=%s pid=%d reason=%d", vm->def->name, (int)vm->pid, (int)reason); @@ -224,8 +225,12 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, iface->ifname)); ignore_value(virNetDevVethDelete(iface->ifname)); } - if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, iface); + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (conn || (conn = virGetConnectNetwork())) + virDomainNetReleaseActualDevice(conn, vm->def, iface); + else + VIR_WARN("Unable to release network device '%s'", NULLSTR(iface->ifname)); + } } virDomainConfVMNWFilterTeardown(vm); @@ -543,6 +548,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, size_t niface = 0; virDomainNetDefPtr net; virDomainNetType type; + virConnectPtr netconn = NULL; if (VIR_ALLOC_N(*veths, def->nnets + 1) < 0) return -1; @@ -559,9 +565,12 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, if (virLXCProcessValidateInterface(net) < 0) goto cleanup; - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(def, net) < 0) - goto cleanup; + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!netconn && !(netconn = virGetConnectNetwork())) + goto cleanup; + if (virDomainNetAllocateActualDevice(netconn, def, net) < 0) + goto cleanup; + } type = virDomainNetGetActualType(net); switch (type) { @@ -639,10 +648,11 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, ignore_value(virNetDevOpenvswitchRemovePort( virDomainNetGetActualBridgeName(iface), iface->ifname)); - if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(def, iface); + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && netconn) + virDomainNetReleaseActualDevice(netconn, def, iface); } } + virObjectUnref(netconn); return ret; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index cf37a16c64..abeed0a806 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4356,7 +4356,8 @@ networkLogAllocation(virNetworkDefPtr netdef, * Returns 0 on success, -1 on failure. */ static int -networkAllocateActualDevice(virDomainDefPtr dom, +networkAllocateActualDevice(virNetworkPtr net, + virDomainDefPtr dom, virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = networkGetDriver(); @@ -4371,6 +4372,14 @@ networkAllocateActualDevice(virDomainDefPtr dom, size_t i; int ret = -1; + obj = virNetworkObjFindByName(driver->networks, net->name); + if (!obj) { + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + net->name); + goto error; + } + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Expected a interface for a virtual network")); @@ -4380,13 +4389,6 @@ networkAllocateActualDevice(virDomainDefPtr dom, virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; - obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - iface->data.network.name); - goto error; - } netdef = virNetworkObjGetDef(obj); if (!virNetworkObjIsActive(obj)) { @@ -4775,7 +4777,8 @@ networkAllocateActualDevice(virDomainDefPtr dom, * No return value (but does log any failures) */ static void -networkNotifyActualDevice(virDomainDefPtr dom, +networkNotifyActualDevice(virNetworkPtr net, + virDomainDefPtr dom, virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = networkGetDriver(); @@ -4786,19 +4789,20 @@ networkNotifyActualDevice(virDomainDefPtr dom, size_t i; char *master = NULL; + obj = virNetworkObjFindByName(driver->networks, net->name); + if (!obj) { + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + net->name); + goto error; + } + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Expected a interface for a virtual network")); goto error; } - obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - iface->data.network.name); - goto error; - } netdef = virNetworkObjGetDef(obj); if (!virNetworkObjIsActive(obj)) { @@ -5011,7 +5015,8 @@ networkNotifyActualDevice(virDomainDefPtr dom, * Returns 0 on success, -1 on failure. */ static int -networkReleaseActualDevice(virDomainDefPtr dom, +networkReleaseActualDevice(virNetworkPtr net, + virDomainDefPtr dom, virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = networkGetDriver(); @@ -5022,19 +5027,20 @@ networkReleaseActualDevice(virDomainDefPtr dom, size_t i; int ret = -1; + obj = virNetworkObjFindByName(driver->networks, net->name); + if (!obj) { + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + net->name); + goto error; + } + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Expected a interface for a virtual network")); goto error; } - obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - iface->data.network.name); - goto error; - } netdef = virNetworkObjGetDef(obj); switch ((virNetworkForwardType) netdef->forward.type) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 97c9de04f0..becded57d9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1374,6 +1374,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, bool charDevPlugged = false; bool netdevPlugged = false; char *netdev_name; + virConnectPtr conn = NULL; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) @@ -1383,9 +1384,12 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(vm->def, net) < 0) - goto cleanup; + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!(conn = virGetConnectNetwork())) + goto cleanup; + if (virDomainNetAllocateActualDevice(conn, vm->def, net) < 0) + goto cleanup; + } actualType = virDomainNetGetActualType(net); @@ -1688,8 +1692,12 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainNetRemoveHostdev(vm->def, net); - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (conn) + virDomainNetReleaseActualDevice(conn, vm->def, net); + else + VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); + } } VIR_FREE(nicstr); @@ -1709,6 +1717,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, VIR_FREE(vhostfd); VIR_FREE(vhostfdName); VIR_FREE(charDevAlias); + virObjectUnref(conn); virObjectUnref(cfg); virDomainCCWAddressSetFree(ccwaddrs); @@ -3719,6 +3728,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, bool needVlanUpdate = false; int ret = -1; int changeidx = -1; + virConnectPtr conn = NULL; if ((changeidx = virDomainNetFindIdx(vm->def, newdev)) < 0) goto cleanup; @@ -3894,9 +3904,11 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, /* allocate new actual device to compare to old - we will need to * free it if we fail for any reason */ - if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(vm->def, newdev) < 0) { - goto cleanup; + if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!(conn = virGetConnectNetwork())) + goto cleanup; + if (virDomainNetAllocateActualDevice(conn, vm->def, newdev) < 0) + goto cleanup; } newType = virDomainNetGetActualType(newdev); @@ -4107,8 +4119,12 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, /* this function doesn't work with HOSTDEV networks yet, thus * no need to change the pointer in the hostdev structure */ - if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, olddev); + if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (conn || (conn = virGetConnectNetwork())) + virDomainNetReleaseActualDevice(conn, vm->def, olddev); + else + VIR_WARN("Unable to release network device '%s'", NULLSTR(olddev->ifname)); + } virDomainNetDefFree(olddev); /* move newdev into the nets list, and NULL it out from the * virDomainDeviceDef that we were given so that the caller @@ -4139,8 +4155,8 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, * that the changes were minor enough that we didn't need to * replace the entire device object. */ - if (newdev && newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, newdev); + if (newdev && newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK && conn) + virDomainNetReleaseActualDevice(conn, vm->def, newdev); return ret; } @@ -4697,8 +4713,15 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefFree(hostdev); if (net) { - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virConnectPtr conn = virGetConnectNetwork(); + if (conn) { + virDomainNetReleaseActualDevice(conn, vm->def, net); + virObjectUnref(conn); + } else { + VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); + } + } virDomainNetDefFree(net); } @@ -4794,8 +4817,15 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, qemuDomainNetDeviceVportRemove(net); - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virConnectPtr conn = virGetConnectNetwork(); + if (conn) { + virDomainNetReleaseActualDevice(conn, vm->def, net); + virObjectUnref(conn); + } else { + VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); + } + } virDomainNetDefFree(net); ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6ec69d3c96..a8b3ed9099 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3266,6 +3266,7 @@ static void qemuProcessNotifyNets(virDomainDefPtr def) { size_t i; + virConnectPtr conn = NULL; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; @@ -3277,9 +3278,14 @@ qemuProcessNotifyNets(virDomainDefPtr def) if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) ignore_value(virNetDevMacVLanReserveName(net->ifname, false)); - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetNotifyActualDevice(def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!conn && !(conn = virGetConnectNetwork())) + continue; + virDomainNetNotifyActualDevice(conn, def, net); + } } + + virObjectUnref(conn); } /* Attempt to instantiate the filters. Ignore failures because it's @@ -5455,6 +5461,7 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def) { int ret = -1; size_t i; + virConnectPtr conn = NULL; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; @@ -5464,9 +5471,12 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def) * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && - virDomainNetAllocateActualDevice(def, net) < 0) - goto cleanup; + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (!conn && !(conn = virGetConnectNetwork())) + goto cleanup; + if (virDomainNetAllocateActualDevice(conn, def, net) < 0) + goto cleanup; + } actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && @@ -5497,6 +5507,7 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def) } ret = 0; cleanup: + virObjectUnref(conn); return ret; } @@ -7111,6 +7122,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, size_t i; char *timestamp; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virConnectPtr conn = NULL; VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%lld, " "reason=%s, asyncJob=%s, flags=0x%x", @@ -7311,8 +7323,12 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* kick the device out of the hostdev list too */ virDomainNetRemoveHostdev(def, net); - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) - virDomainNetReleaseActualDevice(vm->def, net); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (conn || (conn = virGetConnectNetwork())) + virDomainNetReleaseActualDevice(conn, vm->def, net); + else + VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); + } } retry: -- 2.20.1

hostdevs have a link back to the original network device. This is fairly generic accepting any type of device, however, we don't intend to make use of this approach in future. It can thus be specialized to network devices. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 18 ++++++++---------- src/conf/domain_conf.h | 8 +++++++- src/libxl/libxl_driver.c | 4 ++-- src/network/bridge_driver.c | 3 +-- src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_domain_address.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 14 ++++++-------- src/qemu/qemu_hotplug.h | 2 +- src/util/virhostdev.c | 17 ++++++++--------- 10 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 22c6fe1e38..bcd997a705 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2758,7 +2758,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) /* If there is a parent device object, it will handle freeing * def->info. */ - if (def->parent.type == VIR_DOMAIN_DEVICE_NONE) + if (!def->parent) virDomainDeviceInfoFree(def->info); switch (def->mode) { @@ -2829,7 +2829,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def) /* If there is a parent device object, it will handle freeing * the memory. */ - if (def->parent.type == VIR_DOMAIN_DEVICE_NONE) + if (!def->parent) VIR_FREE(def); } @@ -5396,7 +5396,7 @@ virDomainDefCollectBootOrder(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE) { + dev->data.hostdev->parent) { /* This hostdev is a child of a higher level device * (e.g. interface), and thus already being counted on the * list for the other device type. @@ -6246,7 +6246,7 @@ virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def, return 0; if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) { + dev->data.hostdev->parent) { /* This hostdev is a copy of some previous interface. * Aliases are duplicated. */ return 0; @@ -11088,8 +11088,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { virDomainHostdevDefPtr hostdev = &actual->data.hostdev.def; - hostdev->parent.type = VIR_DOMAIN_DEVICE_NET; - hostdev->parent.data.net = parent; + hostdev->parent = parent; hostdev->info = &parent->info; /* The helper function expects type to already be found and * passed in as a string, since it is in a different place in @@ -11741,8 +11740,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, case VIR_DOMAIN_NET_TYPE_HOSTDEV: hostdev = &def->data.hostdev.def; - hostdev->parent.type = VIR_DOMAIN_DEVICE_NET; - hostdev->parent.data.net = def; + hostdev->parent = def; hostdev->info = &def->info; /* The helper function expects type to already be found and * passed in as a string, since it is in a different place in @@ -28398,11 +28396,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, } for (n = 0; n < def->nhostdevs; n++) { - /* If parent.type != NONE, this is just a pointer to the + /* If parent != NONE, this is just a pointer to the * hostdev in a higher-level device (e.g. virDomainNetDef), * and will have already been formatted there. */ - if (def->hostdevs[n]->parent.type == VIR_DOMAIN_DEVICE_NONE && + if (!def->hostdevs[n]->parent && virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) { goto error; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e77fdcabf3..3db319c23d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -330,7 +330,13 @@ struct _virDomainHostdevCaps { /* basic device for direct passthrough */ struct _virDomainHostdevDef { - virDomainDeviceDef parent; /* higher level Def containing this */ + /* If 'parent' is non-NULL it means this host dev was + * not originally present in the XML. It was copied from + * a network interface for convenience when handling + * hostdevs internally. This hostdev should never be + * visible to the user except as part of the interface + */ + virDomainNetDefPtr parent; int mode; /* enum virDomainHostdevMode */ int startupPolicy; /* enum virDomainStartupPolicy */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 506dcdfbeb..3bb93ab5d7 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3915,9 +3915,9 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, /* If this is a network hostdev, we need to use the higher-level * detach function so that mac address / virtualport are reset */ - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) + if (hostdev->parent) ret = libxlDomainDetachNetDevice(driver, vm, - hostdev->parent.data.net); + hostdev->parent); else ret = libxlDomainDetachHostDevice(driver, vm, hostdev); break; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index abeed0a806..1e6d1be32e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4489,8 +4489,7 @@ networkAllocateActualDevice(virNetworkPtr net, netdef->name); goto error; } - iface->data.network.actual->data.hostdev.def.parent.type = VIR_DOMAIN_DEVICE_NET; - iface->data.network.actual->data.hostdev.def.parent.data.net = iface; + iface->data.network.actual->data.hostdev.def.parent = iface; iface->data.network.actual->data.hostdev.def.info = &iface->info; iface->data.network.actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; iface->data.network.actual->data.hostdev.def.managed = netdef->forward.managed ? 1 : 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bc02362495..022bba0280 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5708,8 +5708,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, /* bootNet will be non-0 if boot order was set and no other * net devices were encountered */ - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && - bootIndex == 0) { + if (hostdev->parent && bootIndex == 0) { bootIndex = *bootHostdevNet; *bootHostdevNet = 0; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 3eccf40eb5..7920f11d13 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1520,7 +1520,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, if (!virDeviceInfoPCIAddressIsPresent(info) || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && - (device->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE))) { + device->data.hostdev->parent)) { /* If a hostdev has a parent, its info will be a part of the * parent, and will have its address collected during the scan * of the parent's device type. @@ -1615,7 +1615,7 @@ qemuDomainCollectPCIAddressExtension(virDomainDefPtr def ATTRIBUTE_UNUSED, if (!virDeviceInfoPCIAddressExtensionIsPresent(info) || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && - (device->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE))) { + device->data.hostdev->parent)) { /* If a hostdev has a parent, its info will be a part of the * parent, and will have its address collected during the scan * of the parent's device type. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d2ea301981..b81c411007 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8044,7 +8044,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachLease(driver, vm, dev->data.lease); break; case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainDetachNetDevice(driver, vm, dev, async); + ret = qemuDomainDetachNetDevice(driver, vm, dev->data.net, async); break; case VIR_DOMAIN_DEVICE_HOSTDEV: ret = qemuDomainDetachHostDevice(driver, vm, dev, async); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index becded57d9..0e8f082eee 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4656,11 +4656,9 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias); virObjectEventStateQueue(driver->domainEventState, event); - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) { - net = hostdev->parent.data.net; - + if (hostdev->parent) { for (i = 0; i < vm->def->nnets; i++) { - if (vm->def->nets[i] == net) { + if (vm->def->nets[i] == hostdev->parent) { virDomainNetRemove(vm->def, i); break; } @@ -5782,8 +5780,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, /* If this is a network hostdev, we need to use the higher-level detach * function so that mac address / virtualport are reset */ - if (detach->parent.type == VIR_DOMAIN_DEVICE_NET) - return qemuDomainDetachNetDevice(driver, vm, &detach->parent, async); + if (detach->parent) + return qemuDomainDetachNetDevice(driver, vm, detach->parent, async); else return qemuDomainDetachThisHostDevice(driver, vm, detach, async); } @@ -5945,13 +5943,13 @@ qemuDomainDetachRedirdevDevice(virQEMUDriverPtr driver, int qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainNetDefPtr net, bool async) { int detachidx, ret = -1; virDomainNetDefPtr detach = NULL; - if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) + if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0) goto cleanup; detach = vm->def->nets[detachidx]; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 7ac03b7810..d3fa588aac 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -109,7 +109,7 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, bool async); int qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainNetDefPtr net, bool async); int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 6be395cdda..7f692a5289 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -333,8 +333,7 @@ virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) { return hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && - hostdev->parent.data.net; + hostdev->parent != NULL; } @@ -427,7 +426,7 @@ virHostdevSaveNetConfig(virDomainHostdevDefPtr hostdev, int vf = -1; if (!virHostdevIsPCINetDevice(hostdev) || - virDomainNetGetActualVirtPortProfile(hostdev->parent.data.net)) + virDomainNetGetActualVirtPortProfile(hostdev->parent)) return 0; if (virHostdevIsVirtualFunction(hostdev) != 1) { @@ -474,8 +473,8 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, if (virHostdevNetDevice(hostdev, -1, &linkdev, &vf) < 0) return -1; - vlan = virDomainNetGetActualVlan(hostdev->parent.data.net); - virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parent.data.net); + vlan = virDomainNetGetActualVlan(hostdev->parent); + virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parent); if (virtPort) { if (vlan) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -485,11 +484,11 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, return -1; } if (virHostdevNetConfigVirtPortProfile(linkdev, vf, virtPort, - &hostdev->parent.data.net->mac, + &hostdev->parent->mac, uuid, port_profile_associate) < 0) return -1; } else { - if (virNetDevSetNetConfig(linkdev, vf, &hostdev->parent.data.net->mac, + if (virNetDevSetNetConfig(linkdev, vf, &hostdev->parent->mac, vlan, NULL, true) < 0) return -1; } @@ -535,10 +534,10 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, if (virHostdevNetDevice(hostdev, 0, &linkdev, &vf) < 0) return -1; - virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parent.data.net); + virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parent); if (virtPort) { return virHostdevNetConfigVirtPortProfile(linkdev, vf, virtPort, - &hostdev->parent.data.net->mac, + &hostdev->parent->mac, NULL, port_profile_associate); } else { -- 2.20.1

Ports allocated on virtual networks with type=nat|route|open all get given an actual type of 'network'. Only ports in networks with type=bridge use an actual type of 'bridge'. This distinction makes little sense since the virtualization drivers will treat both actual types in exactly the same way, as they're all just bridge devices a VM needs to be connected to. This doesn't affect user visible XML since the "actual" device XML is internal only, but we need code to convert the data upgrades. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4770dd1e4e..40122612c8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4454,11 +4454,7 @@ networkAllocateActualDevice(virNetworkPtr net, case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_OPEN: - /* for these forward types, the actual net type really *is* - * NETWORK; we just keep the info from the portgroup in - * iface->data.network.actual - */ - iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; + iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; /* we also store the bridge device and macTableManager settings * in iface->data.network.actual->data.bridge for later use @@ -4832,6 +4828,15 @@ networkNotifyActualDevice(virNetworkPtr net, netdef->bridge) < 0)) goto error; + /* Older libvirtd uses actualType==network, but we now + * just use actualType==bridge, as nothing needs to + * distinguish the two cases, and this simplifies virt + * drive code */ + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { + iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + actualType = VIR_DOMAIN_NET_TYPE_BRIDGE; + } + /* see if we're connected to the correct bridge */ if (netdef->bridge) { bool useOVS = false; @@ -5475,9 +5480,8 @@ networkBandwidthGenericChecks(virDomainNetDefPtr iface, virNetDevBandwidthPtr ifaceBand; unsigned long long old_floor, new_floor; - if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_NETWORK && - (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_BRIDGE || - iface->data.network.actual->data.bridge.brname == NULL)) { + if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_BRIDGE || + iface->data.network.actual->data.bridge.brname == NULL) { /* This is not an interface that's plugged into a network. * We don't care. Thus from our POV bandwidth change is allowed. */ return false; -- 2.20.1

On 3/21/19 9:07 PM, Cole Robinson wrote: I remember there was *some* reason for it, but it's very possible it's not valid. So lacking any solid memory from that distant time, one thing to do is audit all occurrences of VIR_DOMAIN_NET_TYPE_NETWORK where the actualType is being compared (rather than the NetDef's type), and behavior is different from VIR_DOMAIN_NET_TYPE_BRIDGE (which is apparently what Cole did, since he's already found most of the "interesting" places I found :-)). He mentions one that he found below, and he also pointed out the class_id thing during parsing in the previous patch, which will now be broken even if we don't support bandwidth on unmanaged bridge networks (because class_id is only parsed when actualType == NETWORK). I'll see if I find any others with a quick search... I *think* that's correct, but it's been too long since I looked at the code. virDomainNetResolveActualType() also needs to be adjusted. Also libxl handles the two types differently in libxlMakeNic, although I don't know enough about Xen networking to know which is way is better. Those are the only additional places I could find.

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virnetdevtap.c | 69 ++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevtap.h | 12 +++++++ 3 files changed, 82 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9494a04bb..6f5a734fdb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2461,6 +2461,7 @@ virNetDevTapDelete; virNetDevTapGetName; virNetDevTapGetRealDeviceName; virNetDevTapInterfaceStats; +virNetDevTapReattachBridge; # util/virnetdevveth.h diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 972f3405aa..0484c7c5a4 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -553,6 +553,75 @@ virNetDevTapAttachBridge(const char *tapname, } +/** + * virNetDevTapReattachBridge: + * @tapname: the tap interface name (or name template) + * @brname: the bridge name + * @macaddr: desired MAC address + * @virtPortProfile: bridge/port specific configuration + * @virtVlan: vlan tag info + * @mtu: requested MTU for port (or 0 for "default") + * @actualMTU: MTU actually set for port (after accounting for bridge's MTU) + * + * Ensures that the tap device (@tapname) is connected to the bridge + * (@brname), potentially removing it from any existing bridge that + * does not match. + * + * Returns 0 in case of success or -1 on failure + */ +int +virNetDevTapReattachBridge(const char *tapname, + const char *brname, + const virMacAddr *macaddr, + const unsigned char *vmuuid, + virNetDevVPortProfilePtr virtPortProfile, + virNetDevVlanPtr virtVlan, + unsigned int mtu, + unsigned int *actualMTU) +{ + bool useOVS = false; + char *master = NULL; + + if (virNetDevGetMaster(tapname, &master) < 0) + return -1; + + /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ + if (STREQ_NULLABLE(master, "ovs-system")) { + useOVS = true; + VIR_FREE(master); + if (virNetDevOpenvswitchInterfaceGetMaster(tapname, &master) < 0) + return -1; + } + + /* Nothing more todo if we're on the right bridge already */ + if (STREQ_NULLABLE(brname, master)) + return 0; + + /* disconnect from current (incorrect) bridge, if any */ + if (master) { + int ret; + VIR_INFO("Removing %s from %s", tapname, master); + if (useOVS) + ret = virNetDevOpenvswitchRemovePort(master, tapname); + else + ret = virNetDevBridgeRemovePort(master, tapname); + VIR_FREE(master); + if (ret < 0) + return -1; + } + + VIR_INFO("Attaching %s to %s", tapname, brname); + if (virNetDevTapAttachBridge(tapname, brname, + macaddr, vmuuid, + virtPortProfile, + virtVlan, + mtu, actualMTU) < 0) + return -1; + + return 0; +} + + /** * virNetDevTapCreateInBridgePort: * @brname: the bridge name diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index 226122aa2c..2b3893cd37 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -71,6 +71,18 @@ virNetDevTapAttachBridge(const char *tapname, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +int +virNetDevTapReattachBridge(const char *tapname, + const char *brname, + const virMacAddr *macaddr, + const unsigned char *vmuuid, + virNetDevVPortProfilePtr virtPortProfile, + virNetDevVlanPtr virtVlan, + unsigned int mtu, + unsigned int *actualMTU) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const virMacAddr *macaddr, -- 2.20.1

On 3/21/19 9:14 PM, Cole Robinson wrote: I don't remember why I made those non-fatal in the original code. Probably wanted to give as high a chance as possible of muddling through even in the face of errors from ovs. But looking at the code again, I agree that it's just as well to fail hard instead of ignoring the errors.

Switch over to use the new API for re-attaching the bridge device Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 47 ++++++++----------------------------- 1 file changed, 10 insertions(+), 37 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 40122612c8..7df0c0f67a 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4792,7 +4792,6 @@ networkNotifyActualDevice(virNetworkPtr net, virNetworkDefPtr netdef; virNetworkForwardIfDefPtr dev = NULL; size_t i; - char *master = NULL; obj = virNetworkObjFindByName(driver->networks, net->name); if (!obj) { @@ -4839,42 +4838,17 @@ networkNotifyActualDevice(virNetworkPtr net, /* see if we're connected to the correct bridge */ if (netdef->bridge) { - bool useOVS = false; - - if (virNetDevGetMaster(iface->ifname, &master) < 0) + /* + * NB: we can't notify the guest of any MTU change anyway, + * so there is no point in trying to learn the actualMTU + * (final arg to virNetDevTapReattachBridge()) + */ + if (virNetDevTapReattachBridge(iface->ifname, netdef->bridge, + &iface->mac, dom->uuid, + virDomainNetGetActualVirtPortProfile(iface), + virDomainNetGetActualVlan(iface), + iface->mtu, NULL) < 0) { goto error; - - /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ - if (STREQ_NULLABLE(master, "ovs-system")) { - useOVS = true; - VIR_FREE(master); - if (virNetDevOpenvswitchInterfaceGetMaster(iface->ifname, &master) < 0) - goto error; - } - - if (STRNEQ_NULLABLE(netdef->bridge, master)) { - /* disconnect from current (incorrect) bridge */ - if (master) { - VIR_INFO("Removing %s from %s", iface->ifname, master); - if (useOVS) - ignore_value(virNetDevOpenvswitchRemovePort(master, iface->ifname)); - else - ignore_value(virNetDevBridgeRemovePort(master, iface->ifname)); - } - - /* attach/reattach to correct bridge. - * NB: we can't notify the guest of any MTU change anyway, - * so there is no point in trying to learn the actualMTU - * (final arg to virNetDevTapAttachBridge()) - */ - VIR_INFO("Attaching %s to %s", iface->ifname, netdef->bridge); - if (virNetDevTapAttachBridge(iface->ifname, netdef->bridge, - &iface->mac, dom->uuid, - virDomainNetGetActualVirtPortProfile(iface), - virDomainNetGetActualVlan(iface), - iface->mtu, NULL) < 0) { - goto error; - } } } @@ -5009,7 +4983,6 @@ networkNotifyActualDevice(virNetworkPtr net, cleanup: virNetworkObjEndAPI(&obj); - VIR_FREE(master); return; error: -- 2.20.1

The call to resolve the actual network type will turn any NICs with type=network into one of the other types. Thus there should be no need to handle type=network in later switch() statements jumping off the actual type. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libxl/libxl_conf.c | 21 +++------------------ src/lxc/lxc_driver.c | 15 +++++++++++---- src/qemu/qemu_command.c | 8 ++++++-- src/qemu/qemu_hotplug.c | 13 +++++++++++-- src/qemu/qemu_interface.c | 12 ++++++++++-- src/qemu/qemu_process.c | 5 ++++- 6 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index e55a9fe22e..54adfd7379 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1352,25 +1352,10 @@ libxlMakeNic(virDomainDefPtr def, } break; case VIR_DOMAIN_NET_TYPE_NETWORK: - { - if (!(conn = virConnectOpen("xen:///system"))) - goto cleanup; - - if (!(network = - virNetworkLookupByName(conn, l_nic->data.network.name))) { - goto cleanup; - } - - if (l_nic->guestIP.nips > 0) { - x_nic->ip = xenMakeIPList(&l_nic->guestIP); - if (!x_nic->ip) - goto cleanup; - } + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; - if (!(x_nic->bridge = virNetworkGetBridgeName(network))) - goto cleanup; - break; - } case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_SERVER: diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a184e67e15..37002dbf23 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3848,8 +3848,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, actualType = virDomainNetGetActualType(net); switch (actualType) { - case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: { + case VIR_DOMAIN_NET_TYPE_BRIDGE: { const char *brname = virDomainNetGetActualBridgeName(net); if (!brname) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -3867,6 +3866,10 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, if (!(veth = virLXCProcessSetupInterfaceDirect(conn, vm->def, net))) goto cleanup; } break; + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: @@ -3912,7 +3915,6 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, } else if (veth) { switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_ETHERNET: ignore_value(virNetDevVethDelete(veth)); break; @@ -3921,6 +3923,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, ignore_value(virNetDevMacVLanDelete(veth)); break; + case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: @@ -4352,7 +4355,6 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_ETHERNET: if (virNetDevVethDelete(detach->ifname) < 0) { virDomainAuditNet(vm, detach, NULL, "detach", false); @@ -4360,6 +4362,11 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, } break; + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; + /* It'd be nice to support this, but with macvlan * once assigned to a container nothing exists on * the host side. Further the container can change diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 022bba0280..64e47a79f0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8768,7 +8768,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, } switch (actualType) { - case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: tapfdSize = net->driver.virtio.queues; if (!tapfdSize) @@ -8846,6 +8845,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, break; + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: @@ -8862,7 +8866,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, */ switch ((virDomainNetType)actualType) { case VIR_DOMAIN_NET_TYPE_ETHERNET: - case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_DIRECT: { @@ -8880,6 +8883,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, break; } + case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0e8f082eee..7b16530ab9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1442,7 +1442,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: tapfdSize = vhostfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = vhostfdSize = 1; @@ -1534,6 +1533,11 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, /* No preparation needed. */ break; + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; + case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -3635,8 +3639,13 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm, switch (virDomainNetGetActualType(newdev)) { case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: break; + + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + return -1; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index c8effa68f4..9bb0d70986 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -58,7 +58,6 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net) switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: if (virDomainNetGetActualBridgeMACTableManager(net) == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) { /* libvirt is managing the FDB of the bridge this device @@ -111,6 +110,11 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net) break; + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: @@ -164,7 +168,6 @@ qemuInterfaceStopDevice(virDomainNetDefPtr net) switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: if (virDomainNetGetActualBridgeMACTableManager(net) == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) { /* remove the FDB entries that were added during @@ -198,6 +201,11 @@ qemuInterfaceStopDevice(virDomainNetDefPtr net) break; } + case VIR_DOMAIN_NET_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpectedly found type=network for actual NIC type")); + goto cleanup; + case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a8b3ed9099..0c53e17dd8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7290,12 +7290,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, } break; case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_NETWORK: #ifdef VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP if (!(vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)) ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap)); #endif break; + case VIR_DOMAIN_NET_TYPE_NETWORK: + VIR_WARN("Unexpectedly found type=network for actual NIC type"); + break; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: -- 2.20.1

During initial NIC setup the hypervisor drivers are responsible for attaching the TAP device to the bridge device. Any fixup after libvirtd restarts should thus also be their responsibility. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 20 +++++++++++++++++++- src/conf/domain_conf.h | 2 +- src/network/bridge_driver.c | 27 ++++++--------------------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bcd997a705..99b75e26f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -54,6 +54,7 @@ #include "virsecret.h" #include "virstring.h" #include "virnetdev.h" +#include "virnetdevtap.h" #include "virnetdevmacvlan.h" #include "virhostdev.h" #include "virmdev.h" @@ -30217,8 +30218,25 @@ virDomainNetNotifyActualDevice(virConnectPtr conn, if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) return; - netNotify(net, dom, iface); + if (netNotify(net, dom, iface) < 0) + goto cleanup; + + if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_BRIDGE) { + /* + * NB: we can't notify the guest of any MTU change anyway, + * so there is no point in trying to learn the actualMTU + * (final arg to virNetDevTapReattachBridge()) + */ + if (virNetDevTapReattachBridge(iface->ifname, + iface->data.network.actual->data.bridge.brname, + &iface->mac, dom->uuid, + virDomainNetGetActualVirtPortProfile(iface), + virDomainNetGetActualVlan(iface), + iface->mtu, NULL) < 0) + goto cleanup; + } + cleanup: virObjectUnref(net); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3db319c23d..30aa985344 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3515,7 +3515,7 @@ typedef int virDomainDefPtr dom, virDomainNetDefPtr iface); -typedef void +typedef int (*virDomainNetNotifyActualDeviceImpl)(virNetworkPtr net, virDomainDefPtr dom, virDomainNetDefPtr iface); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7df0c0f67a..5f3371b150 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4776,12 +4776,11 @@ networkAllocateActualDevice(virNetworkPtr net, * Called to notify the network driver when libvirtd is restarted and * finds an already running domain. If appropriate it will force an * allocation of the actual->direct.linkdev to get everything back in - * order, or re-attach the interface's tap device to the network's - * bridge. + * order. * - * No return value (but does log any failures) + * Returns 0 on success, -1 on failure. */ -static void +static int networkNotifyActualDevice(virNetworkPtr net, virDomainDefPtr dom, virDomainNetDefPtr iface) @@ -4792,6 +4791,7 @@ networkNotifyActualDevice(virNetworkPtr net, virNetworkDefPtr netdef; virNetworkForwardIfDefPtr dev = NULL; size_t i; + int ret = -1; obj = virNetworkObjFindByName(driver->networks, net->name); if (!obj) { @@ -4836,22 +4836,6 @@ networkNotifyActualDevice(virNetworkPtr net, actualType = VIR_DOMAIN_NET_TYPE_BRIDGE; } - /* see if we're connected to the correct bridge */ - if (netdef->bridge) { - /* - * NB: we can't notify the guest of any MTU change anyway, - * so there is no point in trying to learn the actualMTU - * (final arg to virNetDevTapReattachBridge()) - */ - if (virNetDevTapReattachBridge(iface->ifname, netdef->bridge, - &iface->mac, dom->uuid, - virDomainNetGetActualVirtPortProfile(iface), - virDomainNetGetActualVlan(iface), - iface->mtu, NULL) < 0) { - goto error; - } - } - if (!iface->data.network.actual || (actualType != VIR_DOMAIN_NET_TYPE_DIRECT && actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV)) { @@ -4980,10 +4964,11 @@ networkNotifyActualDevice(virNetworkPtr net, goto error; } networkLogAllocation(netdef, actualType, dev, iface, true); + ret = 0; cleanup: virNetworkObjEndAPI(&obj); - return; + return ret; error: goto cleanup; -- 2.20.1

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: The above code was added in libvirt 1.2.11 in December 2014. It seems nearly impossible that someone would be upgrading from a libvirt that was 1.2.10 or older straight to libvirt-5.2.0 *without rebooting their host*. For this reason, I think the above code can/should be retired. Reviewed-by: Laine Stump <laine(a)laine.org&gt; (either way, whether you decide to leave that code in or not.)

All but one of the network types supports port profiles. Rather than duplicating the code to merge profiles 3 times, do it once and then later report an error if used from the wrong place. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 54 +++++++++++++++---------------------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a1ae90a34c..45a45b95d7 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4449,6 +4449,18 @@ networkAllocateActualDevice(virNetworkPtr net, iface->data.network.actual->trustGuestRxFilters = netdef->trustGuestRxFilters; + /* merge virtualports from interface, network, and portgroup to + * arrive at actual virtualport to use + */ + if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, + iface->virtPortProfile, + netdef->virtPortProfile, + portgroup + ? portgroup->virtPortProfile : NULL) < 0) { + goto error; + } + virtport = iface->data.network.actual->virtPortProfile; + switch ((virNetworkForwardType) netdef->forward.type) { case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: @@ -4467,6 +4479,15 @@ networkAllocateActualDevice(virNetworkPtr net, iface->data.network.actual->data.bridge.macTableManager = netdef->macTableManager; + if (virtport) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<virtualport type='%s'> not supported for network " + "'%s' which uses IP forwarding"), + virNetDevVPortTypeToString(virtport->virtPortType), + netdef->name); + goto error; + } + if (networkPlugBandwidth(obj, iface) < 0) goto error; break; @@ -4519,17 +4540,6 @@ networkAllocateActualDevice(virNetworkPtr net, iface->data.network.actual->data.hostdev.def.source.subsys.u.pci.backend = backend; - /* merge virtualports from interface, network, and portgroup to - * arrive at actual virtualport to use - */ - if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, - iface->virtPortProfile, - netdef->virtPortProfile, - portgroup - ? portgroup->virtPortProfile : NULL) < 0) { - goto error; - } - virtport = iface->data.network.actual->virtPortProfile; if (virtport) { /* make sure type is supported for hostdev connections */ if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && @@ -4559,17 +4569,6 @@ networkAllocateActualDevice(virNetworkPtr net, iface->data.network.actual->data.bridge.macTableManager = netdef->macTableManager; - /* merge virtualports from interface, network, and portgroup to - * arrive at actual virtualport to use - */ - if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, - iface->virtPortProfile, - netdef->virtPortProfile, - portgroup - ? portgroup->virtPortProfile : NULL) < 0) { - goto error; - } - virtport = iface->data.network.actual->virtPortProfile; if (virtport) { /* only type='openvswitch' is allowed for bridges */ if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { @@ -4608,17 +4607,6 @@ networkAllocateActualDevice(virNetworkPtr net, iface->data.network.actual->data.direct.mode = virNetDevMacVLanModeTypeFromString(virNetworkForwardTypeToString(netdef->forward.type)); - /* merge virtualports from interface, network, and portgroup to - * arrive at actual virtualport to use - */ - if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile, - iface->virtPortProfile, - netdef->virtPortProfile, - portgroup - ? portgroup->virtPortProfile : NULL) < 0) { - goto error; - } - virtport = iface->data.network.actual->virtPortProfile; if (virtport) { /* make sure type is supported for macvtap connections */ if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG && -- 2.20.1

The virNetDevBandwidthParse method uses the interface type to decide whether to allow use of the "floor" parameter. Using the interface type is not convenient as callers may not have that available, but still wish to allow use of "floor". Switch to an explicit boolean to control its usage. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 4 ++-- src/conf/netdev_bandwidth_conf.c | 22 +++++++--------------- src/conf/netdev_bandwidth_conf.h | 2 +- src/conf/network_conf.c | 4 ++-- tests/virnetdevbandwidthtest.c | 2 +- 5 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 35d965d2a3..d283feaca6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11147,7 +11147,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, if (bandwidth_node && virNetDevBandwidthParse(&actual->bandwidth, bandwidth_node, - actual->type) < 0) + actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) < 0) goto error; vlanNode = virXPathNode("./vlan", ctxt); @@ -11486,7 +11486,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } else if (virXMLNodeNameEqual(cur, "bandwidth")) { if (virNetDevBandwidthParse(&def->bandwidth, cur, - def->type) < 0) + def->type == VIR_DOMAIN_NET_TYPE_NETWORK) < 0) goto error; } else if (virXMLNodeNameEqual(cur, "vlan")) { if (virNetDevVlanParse(cur, ctxt, &def->vlan) < 0) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 3113cde888..014941836d 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -100,18 +100,18 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate) * virNetDevBandwidthParse: * @bandwidth: parsed bandwidth * @node: XML node - * @net_type: one of virDomainNetType + * @allowFloor: whether "floor" setting is supported * * Parse bandwidth XML and return pointer to structure. - * @net_type tell to which type will/is interface connected to. - * Pass -1 if this is not called on interface. + * The @allowFloor attribute indicates whether the caller + * is able to support use of the "floor" setting. * * Returns !NULL on success, NULL on error. */ int virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, xmlNodePtr node, - int net_type) + bool allowFloor) { int ret = -1; virNetDevBandwidthPtr def = NULL; @@ -162,17 +162,9 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, goto cleanup; } - if (def->in->floor && net_type != VIR_DOMAIN_NET_TYPE_NETWORK) { - if (net_type == -1) { - /* 'floor' on network isn't supported */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("floor attribute isn't supported for " - "network's bandwidth yet")); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("floor attribute is supported only for " - "interfaces of type network")); - } + if (def->in->floor && !allowFloor) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("floor attribute is not supported for this config")); goto cleanup; } } diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index cb1ffd29e0..7fe750ce27 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -27,7 +27,7 @@ int virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, xmlNodePtr node, - int net_type) + bool allowFloor) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevBandwidthFormat(virNetDevBandwidthPtr def, virBufferPtr buf); diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 87bf158049..274f482bfc 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1187,7 +1187,7 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, bandwidth_node = virXPathNode("./bandwidth", ctxt); if (bandwidth_node && - virNetDevBandwidthParse(&def->bandwidth, bandwidth_node, -1) < 0) + virNetDevBandwidthParse(&def->bandwidth, bandwidth_node, false) < 0) goto cleanup; vlanNode = virXPathNode("./vlan", ctxt); @@ -1681,7 +1681,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) && - virNetDevBandwidthParse(&def->bandwidth, bandwidthNode, -1) < 0) + virNetDevBandwidthParse(&def->bandwidth, bandwidthNode, false) < 0) goto error; vlanNode = virXPathNode("./vlan", ctxt); diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 96776fa033..23788b4164 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -55,7 +55,7 @@ struct testSetStruct { \ rc = virNetDevBandwidthParse(&(var), \ ctxt->node, \ - VIR_DOMAIN_NET_TYPE_NETWORK); \ + true); \ xmlFreeDoc(doc); \ xmlXPathFreeContext(ctxt); \ if (rc < 0) \ -- 2.20.1

Introduce a virNetworkPortDefPtr struct to represent the data associated with a virtual network port. Add APIs for parsing/formatting XML docs with the data. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/Makefile.inc.am | 2 + src/conf/virnetworkportdef.c | 514 ++++++++++++++++++ src/conf/virnetworkportdef.h | 112 ++++ src/libvirt_private.syms | 10 + tests/Makefile.am | 7 + .../plug-bridge-mactbl.xml | 9 + .../virnetworkportxml2xmldata/plug-bridge.xml | 12 + .../virnetworkportxml2xmldata/plug-direct.xml | 12 + .../plug-hostdev-pci.xml | 12 + tests/virnetworkportxml2xmldata/plug-none.xml | 8 + tests/virnetworkportxml2xmltest.c | 104 ++++ 11 files changed, 802 insertions(+) create mode 100644 src/conf/virnetworkportdef.c create mode 100644 src/conf/virnetworkportdef.h create mode 100644 tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml create mode 100644 tests/virnetworkportxml2xmldata/plug-bridge.xml create mode 100644 tests/virnetworkportxml2xmldata/plug-direct.xml create mode 100644 tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml create mode 100644 tests/virnetworkportxml2xmldata/plug-none.xml create mode 100644 tests/virnetworkportxml2xmltest.c diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index 9b4d80485b..388917c5dd 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -5,6 +5,8 @@ NETDEV_CONF_SOURCES = \ conf/netdev_vport_profile_conf.c \ conf/netdev_vlan_conf.h \ conf/netdev_vlan_conf.c \ + conf/virnetworkportdef.h \ + conf/virnetworkportdef.c \ $(NULL) DOMAIN_CONF_SOURCES = \ diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c new file mode 100644 index 0000000000..7023d9607e --- /dev/null +++ b/src/conf/virnetworkportdef.c @@ -0,0 +1,514 @@ +/* + * virnetworkportdef.c: network port XML processing + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "viralloc.h" +#include "virerror.h" +#include "virstring.h" +#include "virfile.h" +#include "virnetworkportdef.h" +#include "network_conf.h" + +#define VIR_FROM_THIS VIR_FROM_NETWORK + +VIR_ENUM_IMPL(virNetworkPortPlug, VIR_NETWORK_PORT_PLUG_TYPE_LAST, + "none", "bridge", "direct", "hostdev-pci"); + +void +virNetworkPortDefFree(virNetworkPortDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->ownername); + VIR_FREE(def->group); + + virNetDevBandwidthFree(def->bandwidth); + virNetDevVlanClear(&def->vlan); + VIR_FREE(def->virtPortProfile); + + switch ((virNetworkPortPlugType)def->plugtype) { + case VIR_NETWORK_PORT_PLUG_TYPE_NONE: + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE: + VIR_FREE(def->plug.bridge.brname); + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT: + VIR_FREE(def->plug.direct.linkdev); + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_LAST: + default: + break; + } + + VIR_FREE(def); +} + + + +static virNetworkPortDefPtr +virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) +{ + virNetworkPortDefPtr def; + char *uuid = NULL; + xmlNodePtr virtPortNode; + xmlNodePtr vlanNode; + xmlNodePtr bandwidthNode; + xmlNodePtr addressNode; + char *trustGuestRxFilters = NULL; + char *mac = NULL; + char *macmgr = NULL; + char *mode = NULL; + char *plugtype = NULL; + char *managed = NULL; + char *driver = NULL; + char *class_id = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + uuid = virXPathString("string(./uuid)", ctxt); + if (!uuid) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("network port has no uuid")); + goto error; + } + if (virUUIDParse(uuid, def->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse UUID '%s'"), uuid); + goto error; + } + + def->ownername = virXPathString("string(./owner/name)", ctxt); + if (!def->ownername) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("network port has no owner name")); + goto error; + } + + VIR_FREE(uuid); + uuid = virXPathString("string(./owner/uuid)", ctxt); + if (!uuid) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("network port has no owner UUID")); + goto error; + } + + if (virUUIDParse(uuid, def->owneruuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse UUID '%s'"), uuid); + goto error; + } + + def->group = virXPathString("string(./group)", ctxt); + + virtPortNode = virXPathNode("./virtualport", ctxt); + if (virtPortNode && + (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) { + goto error; + } + + mac = virXPathString("string(./mac/@address)", ctxt); + if (!mac) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("network port has no mac")); + goto error; + } + if (virMacAddrParse(mac, &def->mac) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse MAC '%s'"), mac); + goto error; + } + + class_id = virXPathString("string(./class/@id)", ctxt); + if (class_id && + virStrToLong_ui(class_id, NULL, 10, &def->class_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse class id '%s'"), + class_id); + goto error; + } + + bandwidthNode = virXPathNode("./bandwidth", ctxt); + /* + * We don't know if the port will allow the "floor" param or + * not at this stage, so we must just tell virNetDevBandwidthParse + * to allow it regardless. Any bad config must be reported at + * time of use instead. + */ + if (bandwidthNode && + virNetDevBandwidthParse(&def->bandwidth, bandwidthNode, true) < 0) + goto error; + + vlanNode = virXPathNode("./vlan", ctxt); + if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0) + goto error; + + + trustGuestRxFilters + = virXPathString("string(./rxfilters/@trustGuest)", ctxt); + if (trustGuestRxFilters) { + if ((def->trustGuestRxFilters + = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid guest rx filters trust setting '%s' "), + trustGuestRxFilters); + goto error; + } + } + + plugtype = virXPathString("string(./plug/@type)", ctxt); + + if (plugtype && + (def->plugtype = virNetworkPortPlugTypeFromString(plugtype)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid network prt plug type '%s'"), plugtype); + } + + switch (def->plugtype) { + case VIR_NETWORK_PORT_PLUG_TYPE_NONE: + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE: + if (!(def->plug.bridge.brname = virXPathString("string(./plug/@bridge)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing network port bridge name")); + goto error; + } + macmgr = virXPathString("string(./plug/@macTableManager)", ctxt); + if (macmgr && + (def->plug.bridge.macTableManager = + virNetworkBridgeMACTableManagerTypeFromString(macmgr)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid macTableManager setting '%s' " + "in network port"), macmgr); + goto error; + } + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT: + if (!(def->plug.direct.linkdev = virXPathString("string(./plug/@dev)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing network port link device name")); + goto error; + } + mode = virXPathString("string(./plug/@mode)", ctxt); + if (mode && + (def->plug.direct.mode = + virNetDevMacVLanModeTypeFromString(mode)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid mode setting '%s' in network port"), mode); + goto error; + } + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: + managed = virXPathString("string(./plug/@managed)", ctxt); + if (managed && + (def->plug.hostdevpci.managed = + virTristateBoolTypeFromString(managed)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid managed setting '%s' in network port"), mode); + goto error; + } + driver = virXPathString("string(./plug/driver/@name)", ctxt); + if (driver && + (def->plug.hostdevpci.driver = + virNetworkForwardDriverNameTypeFromString(driver)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing network port driver name")); + goto error; + } + if (!(addressNode = virXPathNode("./plug/address", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing network port PCI address")); + goto error; + } + + if (virPCIDeviceAddressParseXML(addressNode, &def->plug.hostdevpci.addr) < 0) + goto error; + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_LAST: + default: + virReportEnumRangeError(virNetworkPortPlugType, def->plugtype); + goto error; + } + + cleanup: + VIR_FREE(class_id); + VIR_FREE(uuid); + VIR_FREE(plugtype); + VIR_FREE(mac); + VIR_FREE(mode); + VIR_FREE(macmgr); + VIR_FREE(driver); + VIR_FREE(managed); + return def; + + error: + virNetworkPortDefFree(def); + def = NULL; + goto cleanup; +} + + +virNetworkPortDefPtr +virNetworkPortDefParseNode(xmlDocPtr xml, + xmlNodePtr root) +{ + xmlXPathContextPtr ctxt = NULL; + virNetworkPortDefPtr def = NULL; + + if (STRNEQ((const char *)root->name, "networkport")) { + virReportError(VIR_ERR_XML_ERROR, + "%s", + _("unknown root element for network port")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(xml); + if (ctxt == NULL) { + virReportOOMError(); + goto cleanup; + } + + ctxt->node = root; + def = virNetworkPortDefParseXML(ctxt); + + cleanup: + xmlXPathFreeContext(ctxt); + return def; +} + + +static virNetworkPortDefPtr +virNetworkPortDefParse(const char *xmlStr, + const char *filename) +{ + virNetworkPortDefPtr def = NULL; + xmlDocPtr xml; + + if ((xml = virXMLParse(filename, xmlStr, _("(networkport_definition)")))) { + def = virNetworkPortDefParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); + } + + return def; +} + + +virNetworkPortDefPtr +virNetworkPortDefParseString(const char *xmlStr) +{ + return virNetworkPortDefParse(xmlStr, NULL); +} + + +virNetworkPortDefPtr +virNetworkPortDefParseFile(const char *filename) +{ + return virNetworkPortDefParse(NULL, filename); +} + + +char * +virNetworkPortDefFormat(const virNetworkPortDef *def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (virNetworkPortDefFormatBuf(&buf, def) < 0) { + virBufferFreeAndReset(&buf); + return NULL; + } + + if (virBufferCheckError(&buf) < 0) + return NULL; + + return virBufferContentAndReset(&buf); +} + + +int +virNetworkPortDefFormatBuf(virBufferPtr buf, + const virNetworkPortDef *def) +{ + char uuid[VIR_UUID_STRING_BUFLEN]; + char macaddr[VIR_MAC_STRING_BUFLEN]; + + virBufferAddLit(buf, "<networkport>\n"); + + virBufferAdjustIndent(buf, 2); + + virUUIDFormat(def->uuid, uuid); + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid); + + virBufferAddLit(buf, "<owner>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<name>%s</name>\n", def->ownername); + virUUIDFormat(def->owneruuid, uuid); + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</owner>\n"); + + if (def->group) + virBufferEscapeString(buf, "<group>%s</group>\n", def->group); + + virMacAddrFormat(&def->mac, macaddr); + virBufferAsprintf(buf, "<mac address='%s'/>\n", macaddr); + + if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) + return -1; + virNetDevBandwidthFormat(def->bandwidth, buf); + if (def->class_id) + virBufferAsprintf(buf, "<class id='%u'/>\n", def->class_id); + if (virNetDevVlanFormat(&def->vlan, buf) < 0) + return -1; + if (def->trustGuestRxFilters) + virBufferAsprintf(buf, "<rxfilters trustGuest='%s'/>\n", + virTristateBoolTypeToString(def->trustGuestRxFilters)); + + if (def->plugtype != VIR_NETWORK_PORT_PLUG_TYPE_NONE) { + virBufferAsprintf(buf, "<plug type='%s'", + virNetworkPortPlugTypeToString(def->plugtype)); + + switch (def->plugtype) { + case VIR_NETWORK_PORT_PLUG_TYPE_NONE: + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE: + virBufferEscapeString(buf, " bridge='%s'", def->plug.bridge.brname); + if (def->plug.bridge.macTableManager) + virBufferAsprintf(buf, " macTableManager='%s'", + virNetworkBridgeMACTableManagerTypeToString( + def->plug.bridge.macTableManager)); + virBufferAddLit(buf, "/>\n"); + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT: + virBufferEscapeString(buf, " dev='%s'", def->plug.direct.linkdev); + virBufferAsprintf(buf, " mode='%s'", + virNetDevMacVLanModeTypeToString( + def->plug.direct.mode)); + virBufferAddLit(buf, "/>\n"); + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: + virBufferAsprintf(buf, " managed='%s'>\n", + def->plug.hostdevpci.managed ? "yes" : "no"); + virBufferAdjustIndent(buf, 2); + if (def->plug.hostdevpci.driver) + virBufferEscapeString(buf, "<driver name='%s'/>\n", + virNetworkForwardDriverNameTypeToString( + def->plug.hostdevpci.driver)); + + virPCIDeviceAddressFormat(buf, def->plug.hostdevpci.addr, false); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</plug>\n"); + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_LAST: + default: + virReportEnumRangeError(virNetworkPortPlugType, def->plugtype); + return -1; + } + } + + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</networkport>\n"); + + return 0; +} + + +static char * +virNetworkPortDefConfigFile(const char *dir, + const char *name) +{ + char *ret = NULL; + + ignore_value(virAsprintf(&ret, "%s/%s.xml", dir, name)); + return ret; +} + + +int +virNetworkPortDefSaveStatus(virNetworkPortDef *def, + const char *dir) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *path; + char *xml = NULL; + int ret = -1; + + virUUIDFormat(def->uuid, uuidstr); + + if (virFileMakePath(dir) < 0) + goto cleanup; + + if (!(path = virNetworkPortDefConfigFile(dir, uuidstr))) + goto cleanup; + + if (!(xml = virNetworkPortDefFormat(def))) + goto cleanup; + + if (virXMLSaveFile(path, uuidstr, "net-port-create", xml) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(xml); + VIR_FREE(path); + return ret; +} + + +int +virNetworkPortDefDeleteStatus(virNetworkPortDef *def, + const char *dir) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *path; + int ret = -1; + + virUUIDFormat(def->uuid, uuidstr); + + if (!(path = virNetworkPortDefConfigFile(dir, uuidstr))) + goto cleanup; + + if (unlink(path) < 0 && errno != ENOENT) { + virReportSystemError(errno, + _("Unable to delete %s"), path); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(path); + return ret; +} diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h new file mode 100644 index 0000000000..3897013a86 --- /dev/null +++ b/src/conf/virnetworkportdef.h @@ -0,0 +1,112 @@ +/* + * virnetworkportdef.h: network port XML processing + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#ifndef LIBVIRT_VIRNETWORKPORTDEF_H +# define LIBVIRT_VIRNETWORKPORTDEF_H + +# include "internal.h" +# include "viruuid.h" +# include "virnetdevvlan.h" +# include "virnetdevvportprofile.h" +# include "virnetdevbandwidth.h" +# include "virpci.h" +# include "virxml.h" +# include "netdev_vport_profile_conf.h" +# include "netdev_bandwidth_conf.h" +# include "netdev_vlan_conf.h" + +typedef struct _virNetworkPortDef virNetworkPortDef; +typedef virNetworkPortDef *virNetworkPortDefPtr; + +typedef enum { + VIR_NETWORK_PORT_PLUG_TYPE_NONE, + VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE, + VIR_NETWORK_PORT_PLUG_TYPE_DIRECT, + VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI, + + VIR_NETWORK_PORT_PLUG_TYPE_LAST, +} virNetworkPortPlugType; + +VIR_ENUM_DECL(virNetworkPortPlug); + +struct _virNetworkPortDef { + unsigned char uuid[VIR_UUID_BUFLEN]; + char *ownername; + unsigned char owneruuid[VIR_UUID_BUFLEN]; + + char *group; + virMacAddr mac; + + virNetDevVPortProfilePtr virtPortProfile; + virNetDevBandwidthPtr bandwidth; + unsigned int class_id; /* class ID for bandwidth 'floor' */ + virNetDevVlan vlan; + int trustGuestRxFilters; /* enum virTristateBool */ + + int plugtype; /* virNetworkPortPlugType */ + union { + struct { + char *brname; + int macTableManager; /* enum virNetworkBridgeMACTableManagerType */ + } bridge; + struct { + char *linkdev; + int mode; /* enum virMacvtapMode from util/macvtap.h */ + } direct; + struct { + virPCIDeviceAddress addr; /* PCI Address of device */ + int driver; /* virNetworkForwardDriverNameType */ + int managed; + } hostdevpci; + } plug; +}; + + +void +virNetworkPortDefFree(virNetworkPortDefPtr port); + +virNetworkPortDefPtr +virNetworkPortDefParseNode(xmlDocPtr xml, + xmlNodePtr root); + +virNetworkPortDefPtr +virNetworkPortDefParseString(const char *xml); + +virNetworkPortDefPtr +virNetworkPortDefParseFile(const char *filename); + +char * +virNetworkPortDefFormat(const virNetworkPortDef *def); + +int +virNetworkPortDefFormatBuf(virBufferPtr buf, + const virNetworkPortDef *def); + +int +virNetworkPortDefSaveStatus(virNetworkPortDef *def, + const char *dir); + +int +virNetworkPortDefDeleteStatus(virNetworkPortDef *def, + const char *dir); + + +#endif /* LIBVIRT_VIRNETWORKPORTDEF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6f5a734fdb..0694db7089 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1061,6 +1061,16 @@ virNetworkObjUpdate; virNetworkObjUpdateAssignDef; +# conf/virnetworkportdef.h +virNetworkPortDefFormat; +virNetworkPortDefFormatBuf; +virNetworkPortDefFree; +virNetworkPortDefParseFile; +virNetworkPortDefParseNode; +virNetworkPortDefParseString; +virNetworkPortDefSaveStatus; + + # conf/virnodedeviceobj.h virNodeDeviceObjEndAPI; virNodeDeviceObjGetDef; diff --git a/tests/Makefile.am b/tests/Makefile.am index 29f1fe2d2a..e39e3658a3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -148,6 +148,7 @@ EXTRA_DIST = \ virmock.h \ virnetdaemondata \ virnetdevtestdata \ + virnetworkportxml2xmldata \ virnwfilterbindingxml2xmldata \ virpcitestdata \ virscsidata \ @@ -334,6 +335,7 @@ endif WITH_YAJL test_programs += \ networkxml2xmltest \ networkxml2xmlupdatetest \ + virnetworkportxml2xmltest \ $(NULL) if WITH_NETWORK @@ -830,6 +832,11 @@ networkxml2xmlupdatetest_SOURCES = \ testutils.c testutils.h networkxml2xmlupdatetest_LDADD = $(LDADDS) +virnetworkportxml2xmltest_SOURCES = \ + virnetworkportxml2xmltest.c \ + testutils.c testutils.h +virnetworkportxml2xmltest_LDADD = $(LDADDS) + if WITH_NETWORK networkxml2conftest_SOURCES = \ networkxml2conftest.c \ diff --git a/tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml b/tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml new file mode 100644 index 0000000000..8036bc2e1c --- /dev/null +++ b/tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml @@ -0,0 +1,9 @@ +<networkport> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid> + <owner> + <name>memtest</name> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid> + </owner> + <mac address='52:54:00:7b:35:93'/> + <plug type='bridge' bridge='virbr0' macTableManager='libvirt'/> +</networkport> diff --git a/tests/virnetworkportxml2xmldata/plug-bridge.xml b/tests/virnetworkportxml2xmldata/plug-bridge.xml new file mode 100644 index 0000000000..6dd576e8a1 --- /dev/null +++ b/tests/virnetworkportxml2xmldata/plug-bridge.xml @@ -0,0 +1,12 @@ +<networkport> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid> + <owner> + <name>memtest</name> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid> + </owner> + <group>web1</group> + <mac address='52:54:00:7b:35:93'/> + <class id='1729'/> + <rxfilters trustGuest='yes'/> + <plug type='bridge' bridge='virbr0'/> +</networkport> diff --git a/tests/virnetworkportxml2xmldata/plug-direct.xml b/tests/virnetworkportxml2xmldata/plug-direct.xml new file mode 100644 index 0000000000..81554b4579 --- /dev/null +++ b/tests/virnetworkportxml2xmldata/plug-direct.xml @@ -0,0 +1,12 @@ +<networkport> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid> + <owner> + <name>memtest</name> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid> + </owner> + <mac address='52:54:00:7b:35:93'/> + <virtualport type='802.1Qbg'> + <parameters managerid='11' typeid='1193047' typeidversion='2'/> + </virtualport> + <plug type='direct' dev='ens3' mode='vepa'/> +</networkport> diff --git a/tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml b/tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml new file mode 100644 index 0000000000..cc4419f3fd --- /dev/null +++ b/tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml @@ -0,0 +1,12 @@ +<networkport> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid> + <owner> + <name>memtest</name> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid> + </owner> + <mac address='52:54:00:7b:35:93'/> + <plug type='hostdev-pci' managed='yes'> + <driver name='vfio'/> + <address domain='0x0001' bus='0x02' slot='0x03' function='0x4'/> + </plug> +</networkport> diff --git a/tests/virnetworkportxml2xmldata/plug-none.xml b/tests/virnetworkportxml2xmldata/plug-none.xml new file mode 100644 index 0000000000..ed7199ec8c --- /dev/null +++ b/tests/virnetworkportxml2xmldata/plug-none.xml @@ -0,0 +1,8 @@ +<networkport> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid> + <owner> + <name>memtest</name> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid> + </owner> + <mac address='52:54:00:7b:35:93'/> +</networkport> diff --git a/tests/virnetworkportxml2xmltest.c b/tests/virnetworkportxml2xmltest.c new file mode 100644 index 0000000000..bb0ae8a8d5 --- /dev/null +++ b/tests/virnetworkportxml2xmltest.c @@ -0,0 +1,104 @@ +/* + * virnetworkportxml2xmltest.c: network port XML processing test suite + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include <unistd.h> + +#include <sys/types.h> +#include <fcntl.h> + +#include "internal.h" +#include "testutils.h" +#include "virnetworkportdef.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + + +static int +testCompareXMLToXMLFiles(const char *expected) +{ + char *actual = NULL; + int ret = -1; + virNetworkPortDefPtr dev = NULL; + + if (!(dev = virNetworkPortDefParseFile(expected))) + goto cleanup; + + if (!(actual = virNetworkPortDefFormat(dev))) + goto cleanup; + + if (virTestCompareToFile(actual, expected) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(actual); + virNetworkPortDefFree(dev); + return ret; +} + +struct testInfo { + const char *name; +}; + +static int +testCompareXMLToXMLHelper(const void *data) +{ + const struct testInfo *info = data; + int ret = -1; + char *xml = NULL; + + if (virAsprintf(&xml, "%s/virnetworkportxml2xmldata/%s.xml", + abs_srcdir, info->name) < 0) + goto cleanup; + + ret = testCompareXMLToXMLFiles(xml); + + cleanup: + VIR_FREE(xml); + + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST(name) \ + do { \ + const struct testInfo info = {name}; \ + if (virTestRun("virnetworkportdeftest " name, \ + testCompareXMLToXMLHelper, &info) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST("plug-none"); + DO_TEST("plug-bridge"); + DO_TEST("plug-bridge-mactbl"); + DO_TEST("plug-direct"); + DO_TEST("plug-hostdev-pci"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) -- 2.20.1

On Fri, Mar 22, 2019 at 11:28:17AM -0400, Laine Stump wrote: I view those as different axis. "bridge" can be implemented with "tap" or "veth" "direct" can be implemented with "macvtap" or "macvlan" IOW, if we did need to distinguish tap & veth later, we can add them as a separate attribute in the XML Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: Reviewed-by: Laine Stump <laine(a)laine.org&gt; (Are you removing device_conf.h from the includes in a later patch?)

Stop passing a virDomainNetDefPtr parameter to networkLogAllocation, instead just pass in the MAC address. The actual device type is also not required, since virNetworkForwardIfDefPtr has a type field that can be used instad. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d4ca2930cc..9eda1c62b0 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4313,32 +4313,29 @@ networkGetDHCPLeases(virNetworkPtr net, static void networkLogAllocation(virNetworkDefPtr netdef, - virDomainNetType actualType, virNetworkForwardIfDefPtr dev, - virDomainNetDefPtr iface, + virMacAddrPtr mac, bool inUse) { char macStr[VIR_MAC_STRING_BUFLEN]; const char *verb = inUse ? "using" : "releasing"; + virMacAddrFormat(mac, macStr); if (!dev) { VIR_INFO("MAC %s %s network %s (%d connections)", - virMacAddrFormat(&iface->mac, macStr), verb, - netdef->name, netdef->connections); + macStr, verb, netdef->name, netdef->connections); } else { - if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (dev->type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI) { VIR_INFO("MAC %s %s network %s (%d connections) " "physical device %04x:%02x:%02x.%x (%d connections)", - virMacAddrFormat(&iface->mac, macStr), verb, - netdef->name, netdef->connections, + macStr, verb, netdef->name, netdef->connections, dev->device.pci.domain, dev->device.pci.bus, dev->device.pci.slot, dev->device.pci.function, dev->connections); } else { VIR_INFO("MAC %s %s network %s (%d connections) " "physical device %s (%d connections)", - virMacAddrFormat(&iface->mac, macStr), verb, - netdef->name, netdef->connections, + macStr, verb, netdef->name, netdef->connections, dev->device.dev, dev->connections); } } @@ -4747,7 +4744,7 @@ networkAllocateActualDevice(virNetworkPtr net, dev->connections--; goto error; } - networkLogAllocation(netdef, actualType, dev, iface, true); + networkLogAllocation(netdef, dev, &iface->mac, true); ret = 0; @@ -4938,7 +4935,7 @@ networkNotifyActualDevice(virNetworkPtr net, netdef->connections--; goto error; } - networkLogAllocation(netdef, actualType, dev, iface, true); + networkLogAllocation(netdef, dev, &iface->mac, true); ret = 0; cleanup: @@ -5105,7 +5102,7 @@ networkReleaseActualDevice(virNetworkPtr net, /* 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, actualType, dev, iface, false); + networkLogAllocation(netdef, dev, &iface->mac, false); } ret = 0; cleanup: -- 2.20.1

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virnetdevvportprofile.c | 16 ++++++++++++++++ src/util/virnetdevvportprofile.h | 2 ++ 3 files changed, 19 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0694db7089..1e405cbe5f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2490,6 +2490,7 @@ virNetDevVlanFree; virNetDevVPortProfileAssociate; virNetDevVPortProfileCheckComplete; virNetDevVPortProfileCheckNoExtras; +virNetDevVPortProfileCopy; virNetDevVPortProfileDisassociate; virNetDevVPortProfileEqual; virNetDevVPortProfileMerge3; diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index fb05190c02..b6e8365e61 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -125,6 +125,22 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr return true; } + +int virNetDevVPortProfileCopy(virNetDevVPortProfilePtr *dst, const virNetDevVPortProfile *src) +{ + if (!src) { + *dst = NULL; + return 0; + } + + if (VIR_ALLOC(*dst) < 0) + return -1; + + memcpy(*dst, src, sizeof(*src)); + return 0; +} + + /* virNetDevVPortProfileCheckComplete() checks that all attributes * required for the type of virtport are specified. When * generateMissing is true, any missing attribute that can be diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index 65b4779861..503c44e086 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -79,6 +79,8 @@ struct _virNetDevVPortProfile { bool virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr b); +int virNetDevVPortProfileCopy(virNetDevVPortProfilePtr *dst, + const virNetDevVPortProfile *src); int virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport, bool generateMissing); -- 2.20.1

Helper APIs are needed to - Populate basic virNetworkPortDef from virDomainNetDef - Set a virDomainActualNetDef from virNetworkPortDef - Populate a full virNetworkPortDef from virDomainActualNetDef Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 272 +++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 17 +++ src/libvirt_private.syms | 3 + 3 files changed, 292 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d283feaca6..ee4d586d77 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -39,6 +39,7 @@ #include "virbuffer.h" #include "virlog.h" #include "nwfilter_conf.h" +#include "virnetworkportdef.h" #include "storage_conf.h" #include "virstoragefile.h" #include "virfile.h" @@ -30161,6 +30162,277 @@ virDomainNetTypeSharesHostView(const virDomainNetDef *net) return false; } +virNetworkPortDefPtr +virDomainNetDefToNetworkPort(virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + virNetworkPortDefPtr port; + + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expected an interface of type 'network' not '%s'"), + virDomainNetTypeToString(iface->type)); + return NULL; + } + + if (VIR_ALLOC(port) < 0) + return NULL; + + virUUIDGenerate(port->uuid); + + memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN); + if (VIR_STRDUP(port->ownername, dom->name) < 0) + goto error; + + if (VIR_STRDUP(port->group, iface->data.network.portgroup) < 0) + goto error; + + memcpy(&port->mac, &iface->mac, VIR_MAC_BUFLEN); + + if (virNetDevVPortProfileCopy(&port->virtPortProfile, iface->virtPortProfile) < 0) + goto error; + + if (virNetDevBandwidthCopy(&port->bandwidth, iface->bandwidth) < 0) + goto error; + + if (virNetDevVlanCopy(&port->vlan, &iface->vlan) < 0) + goto error; + + port->trustGuestRxFilters = iface->trustGuestRxFilters; + + return port; + + error: + virNetworkPortDefFree(port); + return NULL; +} + +int +virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface, + virNetworkPortDefPtr port) +{ + virDomainActualNetDefPtr actual = NULL; + + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expected an interface of type 'network' not '%s'"), + virDomainNetTypeToString(iface->type)); + return -1; + } + + if (VIR_ALLOC(actual) < 0) + return -1; + + switch ((virNetworkPortPlugType)port->plugtype) { + case VIR_NETWORK_PORT_PLUG_TYPE_NONE: + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE: + if (VIR_STRDUP(actual->data.bridge.brname, + port->plug.bridge.brname) < 0) + goto error; + actual->data.bridge.macTableManager = port->plug.bridge.macTableManager; + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT: + if (VIR_STRDUP(actual->data.direct.linkdev, + port->plug.direct.linkdev) < 0) + goto error; + actual->data.direct.mode = port->plug.direct.mode; + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: + actual->data.hostdev.def.parent = iface; + actual->data.hostdev.def.info = &iface->info; + actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + actual->data.hostdev.def.managed = port->plug.hostdevpci.managed; + actual->data.hostdev.def.source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; + actual->data.hostdev.def.source.subsys.u.pci.addr = port->plug.hostdevpci.addr; + switch ((virNetworkForwardDriverNameType)port->plug.hostdevpci.driver) { + case VIR_NETWORK_FORWARD_DRIVER_NAME_DEFAULT: + actual->data.hostdev.def.source.subsys.u.pci.backend = + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT; + break; + + case VIR_NETWORK_FORWARD_DRIVER_NAME_KVM: + actual->data.hostdev.def.source.subsys.u.pci.backend = + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM; + break; + + case VIR_NETWORK_FORWARD_DRIVER_NAME_VFIO: + actual->data.hostdev.def.source.subsys.u.pci.backend = + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + break; + + case VIR_NETWORK_FORWARD_DRIVER_NAME_LAST: + default: + virReportEnumRangeError(virNetworkForwardDriverNameType, + port->plug.hostdevpci.driver); + goto error; + } + + break; + + case VIR_NETWORK_PORT_PLUG_TYPE_LAST: + default: + virReportEnumRangeError(virNetworkPortPlugType, port->plugtype); + goto error; + } + + if (virNetDevVPortProfileCopy(&actual->virtPortProfile, port->virtPortProfile) < 0) + goto error; + + if (virNetDevBandwidthCopy(&actual->bandwidth, port->bandwidth) < 0) + goto error; + + if (virNetDevVlanCopy(&actual->vlan, &port->vlan) < 0) + goto error; + + actual->class_id = port->class_id; + actual->trustGuestRxFilters = port->trustGuestRxFilters; + + virDomainActualNetDefFree(iface->data.network.actual); + iface->data.network.actual = actual; + + return 0; + + error: + virDomainActualNetDefFree(actual); + return -1; +} + +virNetworkPortDefPtr +virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + virDomainActualNetDefPtr actual; + virNetworkPortDefPtr port; + + if (!iface->data.network.actual) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing actual data for interface '%s'"), + iface->ifname); + return NULL; + } + + actual = iface->data.network.actual; + + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expected an interface of type 'network' not '%s'"), + virDomainNetTypeToString(iface->type)); + return NULL; + } + + if (VIR_ALLOC(port) < 0) + return NULL; + + /* Bad - we need to preserve original port uuid */ + virUUIDGenerate(port->uuid); + + memcpy(port->owneruuid, dom->uuid, VIR_UUID_BUFLEN); + if (VIR_STRDUP(port->ownername, dom->name) < 0) + goto error; + + if (VIR_STRDUP(port->group, iface->data.network.portgroup) < 0) + goto error; + + memcpy(&port->mac, &iface->mac, VIR_MAC_BUFLEN); + + switch (virDomainNetGetActualType(iface)) { + case VIR_DOMAIN_NET_TYPE_BRIDGE: + port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE; + if (VIR_STRDUP(port->plug.bridge.brname, + actual->data.bridge.brname) < 0) + goto error; + port->plug.bridge.macTableManager = actual->data.bridge.macTableManager; + break; + + case VIR_DOMAIN_NET_TYPE_DIRECT: + port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_DIRECT; + if (VIR_STRDUP(port->plug.direct.linkdev, + actual->data.direct.linkdev) < 0) + goto error; + port->plug.direct.mode = actual->data.direct.mode; + break; + + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI; + if (actual->data.hostdev.def.mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + actual->data.hostdev.def.source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Actual interface '%s' hostdev was not a PCI device"), + iface->ifname); + goto error; + } + port->plug.hostdevpci.managed = actual->data.hostdev.def.managed; + port->plug.hostdevpci.addr = actual->data.hostdev.def.source.subsys.u.pci.addr; + switch ((virDomainHostdevSubsysPCIBackendType)actual->data.hostdev.def.source.subsys.u.pci.backend) { + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: + port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_DEFAULT; + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: + port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_KVM; + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: + port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_VFIO; + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unexpected PCI backend 'xen'")); + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: + default: + virReportEnumRangeError(virDomainHostdevSubsysPCIBackendType, + actual->data.hostdev.def.source.subsys.u.pci.backend); + goto error; + } + + break; + + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unexpected network port type %s"), + virDomainNetTypeToString(virDomainNetGetActualType(iface))); + goto error; + + case VIR_DOMAIN_NET_TYPE_LAST: + default: + virReportEnumRangeError(virNetworkPortPlugType, port->plugtype); + goto error; + } + + if (virNetDevVPortProfileCopy(&port->virtPortProfile, actual->virtPortProfile) < 0) + goto error; + + if (virNetDevBandwidthCopy(&port->bandwidth, actual->bandwidth) < 0) + goto error; + + if (virNetDevVlanCopy(&port->vlan, &actual->vlan) < 0) + goto error; + + port->class_id = actual->class_id; + port->trustGuestRxFilters = actual->trustGuestRxFilters; + + return port; + + error: + virNetworkPortDefFree(port); + return NULL; +} + static virDomainNetAllocateActualDeviceImpl netAllocate; static virDomainNetNotifyActualDeviceImpl netNotify; static virDomainNetReleaseActualDeviceImpl netRelease; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aa985344..546ee181b1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3510,6 +3510,23 @@ bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action); +// Forward decl to avoid pulling in virnetworkportdef.h because +// that pulls in virhostdev.h which pulls in domain_conf.h (evil) +typedef struct _virNetworkPortDef virNetworkPortDef; +typedef virNetworkPortDef *virNetworkPortDefPtr; + +virNetworkPortDefPtr +virDomainNetDefToNetworkPort(virDomainDefPtr dom, + virDomainNetDefPtr iface); + +int +virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface, + virNetworkPortDefPtr port); + +virNetworkPortDefPtr +virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, + virDomainNetDefPtr iface); + typedef int (*virDomainNetAllocateActualDeviceImpl)(virNetworkPtr net, virDomainDefPtr dom, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e405cbe5f..b23d3c9891 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -457,9 +457,12 @@ virDomainNetAllocateActualDevice; virDomainNetAppendIPAddress; virDomainNetBandwidthChangeAllowed; virDomainNetBandwidthUpdate; +virDomainNetDefActualFromNetworkPort; +virDomainNetDefActualToNetworkPort; virDomainNetDefClear; virDomainNetDefFormat; virDomainNetDefFree; +virDomainNetDefToNetworkPort; virDomainNetFind; virDomainNetFindByName; virDomainNetFindIdx; -- 2.20.1

On Fri, Mar 22, 2019 at 01:11:34PM -0400, Laine Stump wrote: Hm, I swear I fixed that, but perhapos I've mistakenly squashed into a later patch in this series. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: Shouldn't need the "conf/" there. I didn't see any other problem. Reviewed-by: Laine Stump <laine(a)laine.org&gt;

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: Kind of pointless to look at the diff in this case :-), but when I did a side-by-side comparison of the new and old functions, it all checks out. Reviewed-by: Laine Stump <laine(a)laine.org&gt;

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: Don't you want to change this ^^ to "&port->mac"? 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 :-)

When (un)plugging an interface into a network, the 'plugged' and 'unplugged' operations are invoked in the hook script. The data provided to the script contains the network XML, the domain XML and the domain interface XML. When we strictly split the drivers up this will no longer be possible and thus breakage is unavoidable. The hook scripts are not considered to be covered by the API guarantee so this is OK. To avoid existing scripts taking the wrong action, the existing operations are changed to 'port-created' and 'port-deleted' instead. These will receive the network XML and the network port XML. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/hooks.html.in | 24 +++++++++++------------- src/network/bridge_driver.c | 27 +++++++++++---------------- src/util/virhook.c | 4 ++-- src/util/virhook.h | 4 ++-- 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/docs/hooks.html.in b/docs/hooks.html.in index 2c4c39b771..7c9d3ef7f3 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -91,10 +91,8 @@ &lt;/network&gt; &lt;/hookData&gt;</pre> - <p>In the case of an interface - being plugged/unplugged to/from the network, the network XML will be - followed with the full XML description of the domain containing the - interface that is being plugged/unplugged:</p> + <p>In the case of an network port being created / deleted, the network + XML will be followed with the full XML description of the port:</p> <pre>&lt;hookData&gt; &lt;network&gt; @@ -102,11 +100,11 @@ &lt;uuid&gt;afca425a-2c3a-420c-b2fb-dd7b4950d722&lt;/uuid&gt; ... &lt;/network&gt; - &lt;domain type='$domain_type' id='$domain_id'&gt; - &lt;name&gt;$domain_name&lt;/name&gt; - &lt;uuid&gt;afca425a-2c3a-420c-b2fb-dd7b4950d722&lt;/uuid&gt; - ... - &lt;/domain&gt; + &lt;networkport&gt; + &lt;uuid&gt;5d744f21-ba4a-4d6e-bdb2-30a35ff3207d&lt;/uuid&gt; + ... + &lt;plug type='direct' dev='ens3' mode='vepa'/&gt; + &lt;/networkport&gt; &lt;/hookData&gt;</pre> <p>Please note that this approach is different from other cases such as @@ -296,15 +294,15 @@ <pre>/etc/libvirt/hooks/network network_name stopped end -</pre></li> <li>Later, when network is started and there's an interface from a domain to be plugged into the network, the hook script is called as:<br/> - <pre>/etc/libvirt/hooks/network network_name plugged begin -</pre> + <pre>/etc/libvirt/hooks/network network_name port-created begin -</pre> Please note, that in this case, the script is passed both network and - domain XMLs on its stdin.</li> + port XMLs on its stdin.</li> <li>When network is updated, the hook script is called as:<br/> <pre>/etc/libvirt/hooks/network network_name updated begin -</pre></li> <li>When the domain from previous case is shutting down, the interface is unplugged. This leads to another script invocation:<br/> - <pre>/etc/libvirt/hooks/network network_name unplugged begin -</pre> - And again, as in previous case, both network and domain XMLs are passed + <pre>/etc/libvirt/hooks/network network_name port-deleted begin -</pre> + And again, as in previous case, both network and port XMLs are passed onto script's stdin.</li> </ul> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d2bcb81912..1080684043 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -205,14 +205,13 @@ networkObjFromNetwork(virNetworkPtr net) static int networkRunHook(virNetworkObjPtr obj, - virDomainDefPtr dom, - virDomainNetDefPtr iface, + virNetworkPortDefPtr port, int op, int sub_op) { virNetworkDefPtr def; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *xml = NULL, *net_xml = NULL, *dom_xml = NULL; + char *xml = NULL; int hookret; int ret = -1; @@ -226,11 +225,9 @@ networkRunHook(virNetworkObjPtr obj, virBufferAddLit(&buf, "<hookData>\n"); virBufferAdjustIndent(&buf, 2); - if (iface && virDomainNetDefFormat(&buf, iface, NULL, 0) < 0) - goto cleanup; if (virNetworkDefFormatBuf(&buf, def, 0) < 0) goto cleanup; - if (dom && virDomainDefFormatInternal(dom, NULL, 0, &buf, NULL) < 0) + if (port && virNetworkPortDefFormatBuf(&buf, port) < 0) goto cleanup; virBufferAdjustIndent(&buf, -2); @@ -256,8 +253,6 @@ networkRunHook(virNetworkObjPtr obj, cleanup: virBufferFreeAndReset(&buf); VIR_FREE(xml); - VIR_FREE(net_xml); - VIR_FREE(dom_xml); return ret; } @@ -2766,7 +2761,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, /* Run an early hook to set-up missing devices. * If the script raised an error abort the launch. */ - if (networkRunHook(obj, NULL, NULL, + if (networkRunHook(obj, NULL, VIR_HOOK_NETWORK_OP_START, VIR_HOOK_SUBOP_BEGIN) < 0) goto cleanup; @@ -2808,7 +2803,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, } /* finally we can call the 'started' hook script if any */ - if (networkRunHook(obj, NULL, NULL, + if (networkRunHook(obj, NULL, VIR_HOOK_NETWORK_OP_STARTED, VIR_HOOK_SUBOP_BEGIN) < 0) goto cleanup; @@ -2892,7 +2887,7 @@ networkShutdownNetwork(virNetworkDriverStatePtr driver, } /* now that we know it's stopped call the hook if present */ - networkRunHook(obj, NULL, NULL, VIR_HOOK_NETWORK_OP_STOPPED, + networkRunHook(obj, NULL, VIR_HOOK_NETWORK_OP_STOPPED, VIR_HOOK_SUBOP_END); virNetworkObjSetActive(obj, false); @@ -3868,7 +3863,7 @@ networkUpdate(virNetworkPtr net, } /* call the 'updated' network hook script */ - if (networkRunHook(obj, NULL, NULL, VIR_HOOK_NETWORK_OP_UPDATED, + if (networkRunHook(obj, NULL, VIR_HOOK_NETWORK_OP_UPDATED, VIR_HOOK_SUBOP_BEGIN) < 0) goto cleanup; @@ -4689,8 +4684,8 @@ networkAllocateActualDevice(virNetworkPtr net, if (dev) dev->connections++; /* finally we can call the 'plugged' hook script if any */ - if (networkRunHook(obj, dom, iface, - VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, + if (networkRunHook(obj, port, + VIR_HOOK_NETWORK_OP_PORT_CREATED, VIR_HOOK_SUBOP_BEGIN) < 0) { /* adjust for failure */ netdef->connections--; @@ -4883,7 +4878,7 @@ networkNotifyActualDevice(virNetworkPtr net, if (dev) dev->connections++; /* finally we can call the 'plugged' hook script if any */ - if (networkRunHook(obj, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, + if (networkRunHook(obj, port, VIR_HOOK_NETWORK_OP_PORT_CREATED, VIR_HOOK_SUBOP_BEGIN) < 0) { /* adjust for failure */ if (dev) @@ -5034,7 +5029,7 @@ networkReleaseActualDevice(virNetworkPtr net, if (dev) dev->connections--; /* finally we can call the 'unplugged' hook script if any */ - networkRunHook(obj, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, + networkRunHook(obj, port, VIR_HOOK_NETWORK_OP_PORT_DELETED, VIR_HOOK_SUBOP_BEGIN); networkLogAllocation(netdef, dev, &iface->mac, false); diff --git a/src/util/virhook.c b/src/util/virhook.c index 37de0cb39e..0322786719 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -95,8 +95,8 @@ VIR_ENUM_IMPL(virHookNetworkOp, VIR_HOOK_NETWORK_OP_LAST, "start", "started", "stopped", - "plugged", - "unplugged", + "port-created", + "port-deleted", "updated", ); diff --git a/src/util/virhook.h b/src/util/virhook.h index 034fb8f263..fa188c89e3 100644 --- a/src/util/virhook.h +++ b/src/util/virhook.h @@ -79,8 +79,8 @@ typedef enum { VIR_HOOK_NETWORK_OP_START, /* network is about to start */ VIR_HOOK_NETWORK_OP_STARTED, /* network has start */ VIR_HOOK_NETWORK_OP_STOPPED, /* network has stopped */ - VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, /* an interface has been plugged into the network */ - VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, /* an interface was unplugged from the network */ + VIR_HOOK_NETWORK_OP_PORT_CREATED, /* port has been created in the network */ + VIR_HOOK_NETWORK_OP_PORT_DELETED, /* port has been deleted in the network */ VIR_HOOK_NETWORK_OP_UPDATED, /* network has been updated */ VIR_HOOK_NETWORK_OP_LAST, -- 2.20.1

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: This comment is more confusing than it is helpful. Especially since it's wrong :-) The new version of the code is calling virDomainNetBandwidthUpdate() and virNetDevBandwidthSet() in reverse order from what it previously did. If either of the functions were to modify newBandwidth's contents, then the results would change. Fortunately it looks like neither of them do that, so I think we're safe. Reviewed-by: Laine Stump <laine(a)laine.org&gt;

Separate network port allocation code from the domain driver network callback implementation. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 143 +++++++++++++++++++----------------- 1 file changed, 77 insertions(+), 66 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 534d144464..374416e692 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4346,60 +4346,38 @@ networkLogAllocation(virNetworkDefPtr netdef, * "backend" function table. */ -/* networkAllocateActualDevice: - * @dom: domain definition that @iface belongs to - * @iface: the original NetDef from the domain +/* networkAllocatePort: + * @obj: the network to allocate from + * @port: the port definition to allocate * - * Looks up the network reference by iface, allocates a physical + * Looks up the network reference by port, allocates a physical * device from that network (if appropriate), and returns with the - * virDomainActualNetDef filled in accordingly. If there are no - * changes to be made in the netdef, then just leave the actualdef - * empty. + * port configuration filled in accordingly. * * Returns 0 on success, -1 on failure. */ static int -networkAllocateActualDevice(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface) +networkAllocatePort(virNetworkObjPtr obj, + virNetworkPortDefPtr port) { virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj = NULL; virNetworkDefPtr netdef = NULL; virPortGroupDefPtr portgroup = NULL; virNetworkForwardIfDefPtr dev = NULL; size_t i; int ret = -1; virNetDevVPortProfilePtr portprofile = NULL; - virNetworkPortDefPtr port = NULL; - - VIR_DEBUG("Allocating port from net %s", net->name); - obj = virNetworkObjFindByName(driver->networks, net->name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - net->name); - goto error; - } - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - goto error; - } netdef = virNetworkObjGetDef(obj); + VIR_DEBUG("Allocating port from net %s", netdef->name); if (!virNetworkObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("network '%s' is not active"), netdef->name); - goto error; + goto cleanup; } - if (!(port = virDomainNetDefToNetworkPort(dom, iface))) - goto error; - VIR_DEBUG("Interface port group %s", port->group); /* portgroup can be present for any type of network, in particular * for bandwidth information, so we need to check for that and @@ -4411,7 +4389,7 @@ networkAllocateActualDevice(virNetworkPtr net, if (portgroup && portgroup->bandwidth && virNetDevBandwidthCopy(&port->bandwidth, portgroup->bandwidth) < 0) - goto error; + goto cleanup; } if (port->vlan.nTags == 0) { @@ -4422,7 +4400,7 @@ networkAllocateActualDevice(virNetworkPtr net, vlan = &netdef->vlan; if (vlan && virNetDevVlanCopy(&port->vlan, vlan) < 0) - goto error; + goto cleanup; } if (!port->trustGuestRxFilters) { @@ -4440,7 +4418,7 @@ networkAllocateActualDevice(virNetworkPtr net, netdef->virtPortProfile, portgroup ? portgroup->virtPortProfile : NULL) < 0) { - goto error; + goto cleanup; } if (portprofile) { VIR_FREE(port->virtPortProfile); @@ -4456,7 +4434,7 @@ networkAllocateActualDevice(virNetworkPtr net, port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE; if (VIR_STRDUP(port->plug.bridge.brname, netdef->bridge) < 0) - goto error; + goto cleanup; port->plug.bridge.macTableManager = netdef->macTableManager; if (port->virtPortProfile) { @@ -4465,18 +4443,18 @@ networkAllocateActualDevice(virNetworkPtr net, "'%s' which uses IP forwarding"), virNetDevVPortTypeToString(port->virtPortProfile->virtPortType), netdef->name); - goto error; + goto cleanup; } if (networkPlugBandwidth(obj, &port->mac, port->bandwidth, &port->class_id) < 0) - goto error; + goto cleanup; break; case VIR_NETWORK_FORWARD_HOSTDEV: { port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI; if (networkCreateInterfacePool(netdef) < 0) - goto error; + goto cleanup; /* pick first dev with 0 connections */ for (i = 0; i < netdef->forward.nifs; i++) { @@ -4490,7 +4468,7 @@ networkAllocateActualDevice(virNetworkPtr net, _("network '%s' requires exclusive access " "to interfaces, but none are available"), netdef->name); - goto error; + goto cleanup; } port->plug.hostdevpci.addr = dev->device.pci; port->plug.hostdevpci.driver = netdef->forward.driverName; @@ -4506,7 +4484,7 @@ networkAllocateActualDevice(virNetworkPtr net, "via PCI passthrough"), virNetDevVPortTypeToString(port->virtPortProfile->virtPortType), netdef->name); - goto error; + goto cleanup; } } break; @@ -4520,7 +4498,7 @@ networkAllocateActualDevice(virNetworkPtr net, port->plugtype = VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE; if (VIR_STRDUP(port->plug.bridge.brname, netdef->bridge) < 0) - goto error; + goto cleanup; port->plug.bridge.macTableManager = netdef->macTableManager; if (port->virtPortProfile) { @@ -4531,12 +4509,12 @@ networkAllocateActualDevice(virNetworkPtr net, "'%s' which uses a bridge device"), virNetDevVPortTypeToString(port->virtPortProfile->virtPortType), netdef->name); - goto error; + goto cleanup; } } if (networkPlugBandwidth(obj, &port->mac, port->bandwidth, &port->class_id) < 0) - goto error; + goto cleanup; break; } @@ -4570,7 +4548,7 @@ networkAllocateActualDevice(virNetworkPtr net, "'%s' which uses a macvtap device"), virNetDevVPortTypeToString(port->virtPortProfile->virtPortType), netdef->name); - goto error; + goto cleanup; } } @@ -4582,12 +4560,12 @@ networkAllocateActualDevice(virNetworkPtr net, _("network '%s' uses a direct mode, but " "has no forward dev and no interface pool"), netdef->name); - goto error; + goto cleanup; } else { /* pick an interface from the pool */ if (networkCreateInterfacePool(netdef) < 0) - goto error; + goto cleanup; /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both * require exclusive access to a device, so current @@ -4622,26 +4600,26 @@ networkAllocateActualDevice(virNetworkPtr net, _("network '%s' requires exclusive access " "to interfaces, but none are available"), netdef->name); - goto error; + goto cleanup; } if (VIR_STRDUP(port->plug.direct.linkdev, dev->device.dev) < 0) - goto error; + goto cleanup; } break; case VIR_NETWORK_FORWARD_LAST: default: virReportEnumRangeError(virNetworkForwardType, netdef->forward.type); - goto error; + goto cleanup; } if (virNetworkObjMacMgrAdd(obj, driver->dnsmasqStateDir, - dom->name, &port->mac) < 0) - goto error; + port->ownername, &port->mac) < 0) + goto cleanup; if (virNetDevVPortProfileCheckComplete(port->virtPortProfile, true) < 0) - goto error; + goto cleanup; /* make sure that everything now specified for the device is * actually supported on this type of network. NB: network, @@ -4666,7 +4644,7 @@ networkAllocateActualDevice(virNetworkPtr net, "is requesting a vlan tag, but that is not " "supported for this type of network"), netdef->name); - goto error; + goto cleanup; } } @@ -4677,7 +4655,7 @@ networkAllocateActualDevice(virNetworkPtr net, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("bandwidth settings are not supported " "for hostdev interfaces")); - goto error; + goto cleanup; } netdef->connections++; @@ -4691,28 +4669,61 @@ networkAllocateActualDevice(virNetworkPtr net, netdef->connections--; if (dev) dev->connections--; - goto error; + goto cleanup; } - networkLogAllocation(netdef, dev, &iface->mac, true); - - VIR_DEBUG("Populating net def"); - if (virDomainNetDefActualFromNetworkPort(iface, port) < 0) - goto error; + networkLogAllocation(netdef, dev, &port->mac, true); VIR_DEBUG("Port allocated"); - ret = 0; + ret = 0; cleanup: - virNetworkPortDefFree(port); - virNetworkObjEndAPI(&obj); return ret; +} - error: - if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + +static int +networkAllocateActualDevice(virNetworkPtr net, + virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + virNetworkDriverStatePtr driver = networkGetDriver(); + virNetworkPortDefPtr port = NULL; + virNetworkObjPtr obj; + int ret = -1; + + obj = virNetworkObjFindByName(driver->networks, net->name); + if (!obj) { + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + net->name); + return -1; + } + + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Expected a interface for a virtual network")); + goto cleanup; + } + + if (!(port = virDomainNetDefToNetworkPort(dom, iface))) + goto cleanup; + + if (networkAllocatePort(obj, port) < 0) + goto cleanup; + + VIR_DEBUG("Populating net def"); + if (virDomainNetDefActualFromNetworkPort(iface, port) < 0) + goto cleanup; + + ret = 0; + cleanup: + if (ret < 0) { virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; } - goto cleanup; + virNetworkPortDefFree(port); + virNetworkObjEndAPI(&obj); + return ret; } -- 2.20.1

Separate network port notification code from the domain driver network callback implementation. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 106 +++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 43 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 374416e692..d953215315 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4727,74 +4727,51 @@ networkAllocateActualDevice(virNetworkPtr net, } -/* networkNotifyActualDevice: - * @dom: domain definition that @iface belongs to - * @iface: the domain's NetDef with an "actual" device already filled in. +/* networkNotifyPort: + * @obj: the network to notify + * @port: the port definition to notify * * Called to notify the network driver when libvirtd is restarted and * finds an already running domain. If appropriate it will force an * allocation of the actual->direct.linkdev to get everything back in * order. - * - * Returns 0 on success, -1 on failure. */ static int -networkNotifyActualDevice(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface) +networkNotifyPort(virNetworkObjPtr obj, + virNetworkPortDefPtr port) { - virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj; virNetworkDefPtr netdef; virNetworkForwardIfDefPtr dev = NULL; - virNetworkPortDefPtr port = NULL; size_t i; int ret = -1; - obj = virNetworkObjFindByName(driver->networks, net->name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - net->name); - goto error; - } - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - goto error; - } - netdef = virNetworkObjGetDef(obj); if (!virNetworkObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("network '%s' is not active"), netdef->name); - goto error; - } - - if (!(port = virDomainNetDefActualToNetworkPort(dom, iface))) goto cleanup; + } switch (port->plugtype) { case VIR_NETWORK_PORT_PLUG_TYPE_NONE: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpectedly got a network port without a plug")); - goto error; + goto cleanup; case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE: /* see if we're connected to the correct bridge */ if (!netdef->bridge) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpectedly got a network port plugged into a bridge")); - goto error; + goto cleanup; } break; case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT: if (networkCreateInterfacePool(netdef) < 0) - goto error; + goto cleanup; /* find the matching interface and increment its connections */ for (i = 0; i < netdef->forward.nifs; i++) { @@ -4813,7 +4790,7 @@ networkNotifyActualDevice(virNetworkPtr net, "in use by network port '%s'"), netdef->name, port->plug.direct.linkdev, port->uuid); - goto error; + goto cleanup; } /* PASSTHROUGH mode and PRIVATE Mode + 802.1Qbh both require @@ -4829,14 +4806,14 @@ networkNotifyActualDevice(virNetworkPtr net, _("network '%s' claims dev='%s' is already in " "use by a different port"), netdef->name, port->plug.direct.linkdev); - goto error; + goto cleanup; } break; case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: if (networkCreateInterfacePool(netdef) < 0) - goto error; + goto cleanup; /* find the matching interface and increment its connections */ for (i = 0; i < netdef->forward.nifs; i++) { @@ -4858,7 +4835,7 @@ networkNotifyActualDevice(virNetworkPtr net, port->plug.hostdevpci.addr.bus, port->plug.hostdevpci.addr.slot, port->plug.hostdevpci.addr.function); - goto error; + goto cleanup; } /* PASSTHROUGH mode, PRIVATE Mode + 802.1Qbh, and hostdev (PCI @@ -4874,7 +4851,7 @@ networkNotifyActualDevice(virNetworkPtr net, netdef->name, dev->device.pci.domain, dev->device.pci.bus, dev->device.pci.slot, dev->device.pci.function); - goto error; + goto cleanup; } break; @@ -4882,7 +4859,7 @@ networkNotifyActualDevice(virNetworkPtr net, case VIR_NETWORK_PORT_PLUG_TYPE_LAST: default: virReportEnumRangeError(virNetworkPortPlugType, port->plugtype); - goto error; + goto cleanup; } netdef->connections++; @@ -4895,18 +4872,61 @@ networkNotifyActualDevice(virNetworkPtr net, if (dev) dev->connections--; netdef->connections--; - goto error; + goto cleanup; } - networkLogAllocation(netdef, dev, &iface->mac, true); + networkLogAllocation(netdef, dev, &port->mac, true); + ret = 0; + cleanup: + return ret; +} + + +static int +networkNotifyActualDevice(virNetworkPtr net, + virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + virNetworkDriverStatePtr driver = networkGetDriver(); + virNetworkObjPtr obj; + virNetworkDefPtr netdef; + virNetworkPortDefPtr port = NULL; + int ret = -1; + + obj = virNetworkObjFindByName(driver->networks, net->name); + if (!obj) { + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + net->name); + goto cleanup; + } + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Expected a interface for a virtual network")); + goto cleanup; + } + + netdef = virNetworkObjGetDef(obj); + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + netdef->name); + goto cleanup; + } + + if (!(port = virDomainNetDefActualToNetworkPort(dom, iface))) + goto cleanup; + + if (networkNotifyPort(obj, port) < 0) + goto cleanup; + + ret = 0; cleanup: virNetworkObjEndAPI(&obj); virNetworkPortDefFree(port); return ret; - - error: - goto cleanup; } -- 2.20.1

Separate network port deletion code from the domain driver network callback implementation. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 91 ++++++++++++++++++++++++------------- 1 file changed, 59 insertions(+), 32 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d953215315..3c2fcd16e5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4930,9 +4930,9 @@ networkNotifyActualDevice(virNetworkPtr net, } -/* networkReleaseActualDevice: - * @dom: domain definition that @iface belongs to - * @iface: a domain's NetDef (interface definition) +/* networkReleasePort: + * @obj: the network to release from + * @port: the port definition to release * * Given a domain <interface> element that previously had its <actual> * element filled in (and possibly a physical device allocated to it), @@ -4942,40 +4942,15 @@ networkNotifyActualDevice(virNetworkPtr net, * Returns 0 on success, -1 on failure. */ static int -networkReleaseActualDevice(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface) +networkReleasePort(virNetworkObjPtr obj, + virNetworkPortDefPtr port) { virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj; virNetworkDefPtr netdef; virNetworkForwardIfDefPtr dev = NULL; - virNetworkPortDefPtr port = NULL; size_t i; int ret = -1; - obj = virNetworkObjFindByName(driver->networks, net->name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - net->name); - goto cleanup; - } - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - goto cleanup; - } - - if (iface->data.network.actual == NULL) { - ret = 0; - goto cleanup; - } - - if (!(port = virDomainNetDefActualToNetworkPort(dom, iface))) - goto cleanup; - netdef = virNetworkObjGetDef(obj); switch ((virNetworkPortPlugType)port->plugtype) { @@ -5054,7 +5029,7 @@ networkReleaseActualDevice(virNetworkPtr net, goto cleanup; } - virNetworkObjMacMgrDel(obj, driver->dnsmasqStateDir, dom->name, &iface->mac); + virNetworkObjMacMgrDel(obj, driver->dnsmasqStateDir, port->ownername, &port->mac); netdef->connections--; if (dev) @@ -5062,7 +5037,59 @@ networkReleaseActualDevice(virNetworkPtr net, /* finally we can call the 'unplugged' hook script if any */ networkRunHook(obj, port, VIR_HOOK_NETWORK_OP_PORT_DELETED, VIR_HOOK_SUBOP_BEGIN); - networkLogAllocation(netdef, dev, &iface->mac, false); + networkLogAllocation(netdef, dev, &port->mac, false); + + ret = 0; + cleanup: + return ret; +} + + +/* networkReleaseActualDevice: + * @dom: domain definition that @iface belongs to + * @iface: a domain's NetDef (interface definition) + * + * Given a domain <interface> element that previously had its <actual> + * element filled in (and possibly a physical device allocated to it), + * free up the physical device for use by someone else, and free the + * virDomainActualNetDef. + * + * Returns 0 on success, -1 on failure. + */ +static int +networkReleaseActualDevice(virNetworkPtr net, + virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + virNetworkDriverStatePtr driver = networkGetDriver(); + virNetworkObjPtr obj; + virNetworkPortDefPtr port = NULL; + int ret = -1; + + obj = virNetworkObjFindByName(driver->networks, net->name); + if (!obj) { + virReportError(VIR_ERR_NO_NETWORK, + _("no network with matching name '%s'"), + net->name); + goto cleanup; + } + + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Expected a interface for a virtual network")); + goto cleanup; + } + + if (iface->data.network.actual == NULL) { + ret = 0; + goto cleanup; + } + + if (!(port = virDomainNetDefActualToNetworkPort(dom, iface))) + goto cleanup; + + if (networkReleasePort(obj, port) < 0) + goto cleanup; ret = 0; cleanup: -- 2.20.1

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: The fact that the virNetworkPortDefPtr isn't in the args made me realize that this function is changing the bandwidth on the device, but the bandwidth in the virNetworkPortDefPtr is changed elsewhere. This seems a bit odd. But again, it is existing behavior, and you don't want to change everything at once. (seems like that's the way it should work once there is a public API though - the device and the virNetworkPortDef should both be updated by the same function). Here's that message again. Shouldn't we have already weeded out any requests to set/change bandwidth for other network types as invalid (and logged an error) before we could ever get to this point? (but again, that's an existing issue, not something new. The comment really bothers me though :-) Reviewed-by: Laine Stump <laine(a)laine.org&gt; (I unfortunately won't be able to continue with the rest of the series until Thursday...)

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/access/genpolkit.pl | 2 +- src/access/viraccessdriver.h | 6 ++++ src/access/viraccessdrivernop.c | 11 ++++++++ src/access/viraccessdriverpolkit.c | 26 ++++++++++++++++++ src/access/viraccessdriverstack.c | 25 +++++++++++++++++ src/access/viraccessmanager.c | 16 +++++++++++ src/access/viraccessmanager.h | 6 ++++ src/access/viraccessperm.c | 6 ++++ src/access/viraccessperm.h | 44 ++++++++++++++++++++++++++++++ 9 files changed, 141 insertions(+), 1 deletion(-) diff --git a/src/access/genpolkit.pl b/src/access/genpolkit.pl index e074c90eb6..f8f20caf65 100755 --- a/src/access/genpolkit.pl +++ b/src/access/genpolkit.pl @@ -21,7 +21,7 @@ use strict; use warnings; my @objects = ( - "CONNECT", "DOMAIN", "INTERFACE", + "CONNECT", "DOMAIN", "INTERFACE", "NETWORK_PORT", "NETWORK","NODE_DEVICE", "NWFILTER_BINDING", "NWFILTER", "SECRET", "STORAGE_POOL", "STORAGE_VOL", ); diff --git a/src/access/viraccessdriver.h b/src/access/viraccessdriver.h index 2cc3950f60..590d86fdf0 100644 --- a/src/access/viraccessdriver.h +++ b/src/access/viraccessdriver.h @@ -39,6 +39,11 @@ typedef int (*virAccessDriverCheckNetworkDrv)(virAccessManagerPtr manager, const char *driverName, virNetworkDefPtr network, virAccessPermNetwork av); +typedef int (*virAccessDriverCheckNetworkPortDrv)(virAccessManagerPtr manager, + const char *driverName, + virNetworkDefPtr network, + virNetworkPortDefPtr port, + virAccessPermNetworkPort av); typedef int (*virAccessDriverCheckNodeDeviceDrv)(virAccessManagerPtr manager, const char *driverName, virNodeDeviceDefPtr nodedev, @@ -82,6 +87,7 @@ struct _virAccessDriver { virAccessDriverCheckDomainDrv checkDomain; virAccessDriverCheckInterfaceDrv checkInterface; virAccessDriverCheckNetworkDrv checkNetwork; + virAccessDriverCheckNetworkPortDrv checkNetworkPort; virAccessDriverCheckNodeDeviceDrv checkNodeDevice; virAccessDriverCheckNWFilterDrv checkNWFilter; virAccessDriverCheckNWFilterBindingDrv checkNWFilterBinding; diff --git a/src/access/viraccessdrivernop.c b/src/access/viraccessdrivernop.c index 98ef9206c5..5e9d9db759 100644 --- a/src/access/viraccessdrivernop.c +++ b/src/access/viraccessdrivernop.c @@ -57,6 +57,16 @@ virAccessDriverNopCheckNetwork(virAccessManagerPtr manager ATTRIBUTE_UNUSED, return 1; /* Allow */ } +static int +virAccessDriverNopCheckNetworkPort(virAccessManagerPtr manager ATTRIBUTE_UNUSED, + const char *driverName ATTRIBUTE_UNUSED, + virNetworkDefPtr network ATTRIBUTE_UNUSED, + virNetworkPortDefPtr port ATTRIBUTE_UNUSED, + virAccessPermNetworkPort perm ATTRIBUTE_UNUSED) +{ + return 1; /* Allow */ +} + static int virAccessDriverNopCheckNodeDevice(virAccessManagerPtr manager ATTRIBUTE_UNUSED, const char *driverName ATTRIBUTE_UNUSED, @@ -119,6 +129,7 @@ virAccessDriver accessDriverNop = { .checkDomain = virAccessDriverNopCheckDomain, .checkInterface = virAccessDriverNopCheckInterface, .checkNetwork = virAccessDriverNopCheckNetwork, + .checkNetworkPort = virAccessDriverNopCheckNetworkPort, .checkNodeDevice = virAccessDriverNopCheckNodeDevice, .checkNWFilter = virAccessDriverNopCheckNWFilter, .checkNWFilterBinding = virAccessDriverNopCheckNWFilterBinding, diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index 6954d74a15..b1473cd0a4 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -237,6 +237,31 @@ virAccessDriverPolkitCheckNetwork(virAccessManagerPtr manager, attrs); } +static int +virAccessDriverPolkitCheckNetworkPort(virAccessManagerPtr manager, + const char *driverName, + virNetworkDefPtr network, + virNetworkPortDefPtr port, + virAccessPermNetworkPort perm) +{ + char uuidstr1[VIR_UUID_STRING_BUFLEN]; + char uuidstr2[VIR_UUID_STRING_BUFLEN]; + const char *attrs[] = { + "connect_driver", driverName, + "network_name", network->name, + "network_uuid", uuidstr1, + "port_uuid", uuidstr2, + NULL, + }; + virUUIDFormat(network->uuid, uuidstr1); + virUUIDFormat(port->uuid, uuidstr2); + + return virAccessDriverPolkitCheck(manager, + "network-port", + virAccessPermNetworkPortTypeToString(perm), + attrs); +} + static int virAccessDriverPolkitCheckNodeDevice(virAccessManagerPtr manager, const char *driverName, @@ -427,6 +452,7 @@ virAccessDriver accessDriverPolkit = { .checkDomain = virAccessDriverPolkitCheckDomain, .checkInterface = virAccessDriverPolkitCheckInterface, .checkNetwork = virAccessDriverPolkitCheckNetwork, + .checkNetworkPort = virAccessDriverPolkitCheckNetworkPort, .checkNodeDevice = virAccessDriverPolkitCheckNodeDevice, .checkNWFilter = virAccessDriverPolkitCheckNWFilter, .checkNWFilterBinding = virAccessDriverPolkitCheckNWFilterBinding, diff --git a/src/access/viraccessdriverstack.c b/src/access/viraccessdriverstack.c index 0ffc6abaf3..238caef115 100644 --- a/src/access/viraccessdriverstack.c +++ b/src/access/viraccessdriverstack.c @@ -151,6 +151,30 @@ virAccessDriverStackCheckNetwork(virAccessManagerPtr manager, return ret; } +static int +virAccessDriverStackCheckNetworkPort(virAccessManagerPtr manager, + const char *driverName, + virNetworkDefPtr network, + virNetworkPortDefPtr port, + virAccessPermNetworkPort perm) +{ + virAccessDriverStackPrivatePtr priv = virAccessManagerGetPrivateData(manager); + int ret = 1; + size_t i; + + for (i = 0; i < priv->managersLen; i++) { + int rv; + /* We do not short-circuit on first denial - always check all drivers */ + rv = virAccessManagerCheckNetworkPort(priv->managers[i], driverName, network, port, perm); + if (rv == 0 && ret != -1) + ret = 0; + else if (rv < 0) + ret = -1; + } + + return ret; +} + static int virAccessDriverStackCheckNodeDevice(virAccessManagerPtr manager, const char *driverName, @@ -298,6 +322,7 @@ virAccessDriver accessDriverStack = { .checkDomain = virAccessDriverStackCheckDomain, .checkInterface = virAccessDriverStackCheckInterface, .checkNetwork = virAccessDriverStackCheckNetwork, + .checkNetworkPort = virAccessDriverStackCheckNetworkPort, .checkNodeDevice = virAccessDriverStackCheckNodeDevice, .checkNWFilter = virAccessDriverStackCheckNWFilter, .checkNWFilterBinding = virAccessDriverStackCheckNWFilterBinding, diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index f5d62604cf..24d9713cfd 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -268,6 +268,22 @@ int virAccessManagerCheckNetwork(virAccessManagerPtr manager, return virAccessManagerSanitizeError(ret, driverName); } +int virAccessManagerCheckNetworkPort(virAccessManagerPtr manager, + const char *driverName, + virNetworkDefPtr network, + virNetworkPortDefPtr port, + virAccessPermNetworkPort perm) +{ + int ret = 0; + VIR_DEBUG("manager=%p(name=%s) driver=%s network=%p port=%p perm=%d", + manager, manager->drv->name, driverName, network, port, perm); + + if (manager->drv->checkNetworkPort) + ret = manager->drv->checkNetworkPort(manager, driverName, network, port, perm); + + return virAccessManagerSanitizeError(ret, driverName); +} + int virAccessManagerCheckNodeDevice(virAccessManagerPtr manager, const char *driverName, virNodeDeviceDefPtr nodedev, diff --git a/src/access/viraccessmanager.h b/src/access/viraccessmanager.h index ab5ef87585..bedd6ba475 100644 --- a/src/access/viraccessmanager.h +++ b/src/access/viraccessmanager.h @@ -30,6 +30,7 @@ # include "conf/secret_conf.h" # include "conf/interface_conf.h" # include "conf/virnwfilterbindingdef.h" +# include "conf/virnetworkportdef.h" # include "access/viraccessperm.h" typedef struct _virAccessManager virAccessManager; @@ -66,6 +67,11 @@ int virAccessManagerCheckNetwork(virAccessManagerPtr manager, const char *driverName, virNetworkDefPtr network, virAccessPermNetwork perm); +int virAccessManagerCheckNetworkPort(virAccessManagerPtr manager, + const char *driverName, + virNetworkDefPtr network, + virNetworkPortDefPtr port, + virAccessPermNetworkPort perm); int virAccessManagerCheckNodeDevice(virAccessManagerPtr manager, const char *driverName, virNodeDeviceDefPtr nodedev, diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index 67f751ef9c..74993e9f29 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -57,6 +57,12 @@ VIR_ENUM_IMPL(virAccessPermNetwork, VIR_ACCESS_PERM_NETWORK_LAST, "getattr", "read", "write", "save", "delete", "start", "stop", + "search_ports", +); + +VIR_ENUM_IMPL(virAccessPermNetworkPort, + VIR_ACCESS_PERM_NETWORK_PORT_LAST, + "getattr", "read", "write", "create", "delete", ); VIR_ENUM_IMPL(virAccessPermNodeDevice, diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index ce3865b359..c96e4f46e1 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -404,6 +404,12 @@ typedef enum { */ VIR_ACCESS_PERM_NETWORK_START, + /** + * @desc: List network ports + * @message: Listing network ports requires authorization + */ + VIR_ACCESS_PERM_NETWORK_SEARCH_PORTS, + /** * @desc: Stop network * @message: Stopping network requires authorization @@ -413,6 +419,43 @@ typedef enum { VIR_ACCESS_PERM_NETWORK_LAST } virAccessPermNetwork; +typedef enum { + + /** + * @desc: Access network port + * @message: Accessing network port requires authorization + * @anonymous: 1 + */ + VIR_ACCESS_PERM_NETWORK_PORT_GETATTR, + + /** + * @desc: Read network port + * @message: Reading network port configuration requires authorization + * @anonymous: 1 + */ + VIR_ACCESS_PERM_NETWORK_PORT_READ, + + /** + * @desc: Read network port + * @message: Writing network port configuration requires authorization + */ + VIR_ACCESS_PERM_NETWORK_PORT_WRITE, + + /** + * @desc: Create network port + * @message: Creating network port configuration requires authorization + */ + VIR_ACCESS_PERM_NETWORK_PORT_CREATE, + + /** + * @desc: Delete network port + * @message: Deleting network port configuration requires authorization + */ + VIR_ACCESS_PERM_NETWORK_PORT_DELETE, + + VIR_ACCESS_PERM_NETWORK_PORT_LAST +} virAccessPermNetworkPort; + typedef enum { /** @@ -692,6 +735,7 @@ VIR_ENUM_DECL(virAccessPermConnect); VIR_ENUM_DECL(virAccessPermDomain); VIR_ENUM_DECL(virAccessPermInterface); VIR_ENUM_DECL(virAccessPermNetwork); +VIR_ENUM_DECL(virAccessPermNetworkPort); VIR_ENUM_DECL(virAccessPermNodeDevice); VIR_ENUM_DECL(virAccessPermNWFilter); VIR_ENUM_DECL(virAccessPermNWFilterBinding); -- 2.20.1

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virsh-completer.c | 50 +++++ tools/virsh-completer.h | 4 + tools/virsh-network.c | 399 +++++++++++++++++++++++++++++++++++++++- tools/virsh-network.h | 5 + 4 files changed, 457 insertions(+), 1 deletion(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index c4adbb70d0..6ff9e6c5b9 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -442,6 +442,56 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED, } +char ** +virshNetworkPortUUIDCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virNetworkPtr net = NULL; + virNetworkPortPtr *ports = NULL; + int nports = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (!(net = virshCommandOptNetwork(ctl, cmd, NULL))) + return false; + + if ((nports = virNetworkListAllPorts(net, &ports, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nports + 1) < 0) + goto error; + + for (i = 0; i < nports; i++) { + char uuid[VIR_UUID_STRING_BUFLEN]; + + if (virNetworkPortGetUUIDString(ports[i], uuid) < 0 || + VIR_STRDUP(ret[i], uuid) < 0) + goto error; + + virNetworkPortFree(ports[i]); + } + VIR_FREE(ports); + + return ret; + + error: + for (; i < nports; i++) + virNetworkPortFree(ports[i]); + VIR_FREE(ports); + for (i = 0; i < nports; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} + + char ** virshNodeDeviceNameCompleter(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED, diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 2e2e1edafb..991c8ded73 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -59,6 +59,10 @@ char ** virshNetworkEventNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshNetworkPortUUIDCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + char ** virshNodeDeviceNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 9adc63a8fa..6d8ecd5b09 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1,7 +1,7 @@ /* * virsh-network.c: Commands to manage network * - * Copyright (C) 2005, 2007-2016 Red Hat, Inc. + * Copyright (C) 2005, 2007-2019 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -53,6 +53,16 @@ #define VIRSH_COMMON_OPT_NETWORK_OT_STRING_FULL(cflags) \ VIRSH_COMMON_OPT_NETWORK_OT_STRING(N_("network name or uuid"), cflags) +#define VIRSH_COMMON_OPT_NETWORK_PORT(cflags) \ + {.name = "port", \ + .type = VSH_OT_DATA, \ + .flags = VSH_OFLAG_REQ, \ + .help = N_("port UUID"), \ + .completer = virshNetworkPortUUIDCompleter, \ + .completer_flags = cflags, \ + } + + virNetworkPtr virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, const char **name, unsigned int flags) @@ -91,6 +101,35 @@ virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, return network; } + +virNetworkPortPtr +virshCommandOptNetworkPort(vshControl *ctl, const vshCmd *cmd, + virNetworkPtr net, + const char **name) +{ + virNetworkPortPtr port = NULL; + const char *n = NULL; + const char *optname = "port"; + + if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + return NULL; + + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + cmd->def->name, optname, n); + + if (name) + *name = n; + + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network UUID\n", + cmd->def->name, optname); + port = virNetworkPortLookupByUUIDString(net, n); + + if (!port) + vshError(ctl, _("failed to get network port '%s'"), n); + + return port; +} + /* * "net-autostart" command */ @@ -1434,6 +1473,340 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "net-port-create" command + */ +static const vshCmdInfo info_network_port_create[] = { + {.name = "help", + .data = N_("create a network port from an XML file") + }, + {.name = "desc", + .data = N_("Create a network port.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_network_port_create[] = { + VIRSH_COMMON_OPT_NETWORK_FULL(0), + VIRSH_COMMON_OPT_FILE(N_("file containing an XML network port description")), + {.name = NULL} +}; + +static bool +cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd) +{ + virNetworkPortPtr port = NULL; + const char *from = NULL; + bool ret = false; + char *buffer = NULL; + virNetworkPtr network = NULL; + + network = virshCommandOptNetwork(ctl, cmd, NULL); + if (network == NULL) + goto cleanup; + + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) + goto cleanup; + + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) + goto cleanup; + + port = virNetworkPortCreateXML(network, buffer, 0); + + if (port != NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virNetworkPortGetUUIDString(port, uuidstr); + vshPrintExtra(ctl, _("Network port %s created from %s\n"), + uuidstr, from); + } else { + vshError(ctl, _("Failed to create network from %s"), from); + goto cleanup; + } + + ret = true; + cleanup: + VIR_FREE(buffer); + if (port) + virNetworkPortFree(port); + if (network) + virNetworkFree(network); + return ret; +} + +/* + * "net-port-dumpxml" command + */ +static const vshCmdInfo info_network_port_dumpxml[] = { + {.name = "help", + .data = N_("network port information in XML") + }, + {.name = "desc", + .data = N_("Output the network port information as an XML dump to stdout.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_network_port_dumpxml[] = { + VIRSH_COMMON_OPT_NETWORK_FULL(0), + VIRSH_COMMON_OPT_NETWORK_PORT(0), + {.name = NULL} +}; + +static bool +cmdNetworkPortDumpXML(vshControl *ctl, const vshCmd *cmd) +{ + virNetworkPtr network; + virNetworkPortPtr port = NULL; + bool ret = true; + char *dump; + unsigned int flags = 0; + + if (!(network = virshCommandOptNetwork(ctl, cmd, NULL))) + goto cleanup; + + if (!(port = virshCommandOptNetworkPort(ctl, cmd, network, NULL))) + goto cleanup; + + dump = virNetworkPortGetXMLDesc(port, flags); + + if (dump != NULL) { + vshPrint(ctl, "%s", dump); + VIR_FREE(dump); + } else { + ret = false; + } + + cleanup: + if (port) + virNetworkPortFree(port); + if (network) + virNetworkFree(network); + return ret; +} + + +/* + * "net-port-delete" command + */ +static const vshCmdInfo info_network_port_delete[] = { + {.name = "help", + .data = N_("network port information in XML") + }, + {.name = "desc", + .data = N_("Output the network port information as an XML dump to stdout.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_network_port_delete[] = { + VIRSH_COMMON_OPT_NETWORK_FULL(0), + VIRSH_COMMON_OPT_NETWORK_PORT(0), + {.name = NULL} +}; + +static bool +cmdNetworkPortDelete(vshControl *ctl, const vshCmd *cmd) +{ + virNetworkPtr network = NULL; + virNetworkPortPtr port = NULL; + bool ret = true; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (!(network = virshCommandOptNetwork(ctl, cmd, NULL))) + goto cleanup; + + if (!(port = virshCommandOptNetworkPort(ctl, cmd, network, NULL))) + goto cleanup; + + if (virNetworkPortGetUUIDString(port, uuidstr) < 0) + goto cleanup; + + if (virNetworkPortDelete(port, 0) < 0) { + vshError(ctl, _("Failed to delete network port %s"), uuidstr); + goto cleanup; + } else { + vshPrintExtra(ctl, _("Network port %s deleted\n"), uuidstr); + } + + ret = true; + cleanup: + if (port) + virNetworkPortFree(port); + if (network) + virNetworkFree(network); + return ret; +} + + +static int +virshNetworkPortSorter(const void *a, const void *b) +{ + virNetworkPortPtr *na = (virNetworkPortPtr *) a; + virNetworkPortPtr *nb = (virNetworkPortPtr *) b; + unsigned char uuida[VIR_UUID_BUFLEN]; + unsigned char uuidb[VIR_UUID_BUFLEN]; + + if (*na && !*nb) + return -1; + + if (!*na) + return *nb != NULL; + + if (virNetworkPortGetUUID(*na, uuida) < 0 || + virNetworkPortGetUUID(*nb, uuidb) < 0) + return -1; + + return memcmp(uuida, uuidb, VIR_UUID_BUFLEN); +} + +struct virshNetworkPortList { + virNetworkPortPtr *ports; + size_t nports; +}; +typedef struct virshNetworkPortList *virshNetworkPortListPtr; + +static void +virshNetworkPortListFree(virshNetworkPortListPtr list) +{ + size_t i; + + if (list && list->ports) { + for (i = 0; i < list->nports; i++) { + if (list->ports[i]) + virNetworkPortFree(list->ports[i]); + } + VIR_FREE(list->ports); + } + VIR_FREE(list); +} + +static virshNetworkPortListPtr +virshNetworkPortListCollect(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + virshNetworkPortListPtr list = vshMalloc(ctl, sizeof(*list)); + int ret; + virNetworkPtr network = NULL; + bool success = false; + + if (!(network = virshCommandOptNetwork(ctl, cmd, NULL))) + goto cleanup; + + /* try the list with flags support (0.10.2 and later) */ + if ((ret = virNetworkListAllPorts(network, + &list->ports, + flags)) < 0) + goto cleanup; + + list->nports = ret; + + /* sort the list */ + if (list->ports && list->nports) + qsort(list->ports, list->nports, + sizeof(*list->ports), virshNetworkPortSorter); + + success = true; + + cleanup: + if (!success) { + virshNetworkPortListFree(list); + list = NULL; + } + + if (network) + virNetworkFree(network); + + return list; +} + +/* + * "net-list" command + */ +static const vshCmdInfo info_network_port_list[] = { + {.name = "help", + .data = N_("list network ports") + }, + {.name = "desc", + .data = N_("Returns list of network ports.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_network_port_list[] = { + VIRSH_COMMON_OPT_NETWORK_FULL(0), + {.name = "uuid", + .type = VSH_OT_BOOL, + .help = N_("list uuid's only") + }, + {.name = "table", + .type = VSH_OT_BOOL, + .help = N_("list table (default)") + }, + {.name = NULL} +}; + +#define FILTER(NAME, FLAG) \ + if (vshCommandOptBool(cmd, NAME)) \ + flags |= (FLAG) +static bool +cmdNetworkPortList(vshControl *ctl, const vshCmd *cmd) +{ + virshNetworkPortListPtr list = NULL; + size_t i; + bool ret = false; + bool optTable = vshCommandOptBool(cmd, "table"); + bool optUUID = vshCommandOptBool(cmd, "uuid"); + char uuid[VIR_UUID_STRING_BUFLEN]; + unsigned int flags = 0; + vshTablePtr table = NULL; + + if (optTable + optUUID > 1) { + vshError(ctl, "%s", + _("Only one argument from --table and --uuid " + "may be specified.")); + return false; + } + + if (!optUUID) + optTable = true; + + if (!(list = virshNetworkPortListCollect(ctl, cmd, flags))) + return false; + + if (optTable) { + table = vshTableNew(_("UUID"), NULL); + if (!table) + goto cleanup; + } + + for (i = 0; i < list->nports; i++) { + virNetworkPortPtr port = list->ports[i]; + + if (virNetworkPortGetUUIDString(port, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get network's UUID")); + goto cleanup; + } + if (optTable) { + if (vshTableRowAppend(table, uuid, NULL) < 0) + goto cleanup; + } else if (optUUID) { + vshPrint(ctl, "%s\n", uuid); + } + } + + if (optTable) + vshTablePrintToStdout(table, ctl); + + ret = true; + cleanup: + vshTableFree(table); + virshNetworkPortListFree(list); + return ret; +} +#undef FILTER + + const vshCmdDef networkCmds[] = { {.name = "net-autostart", .handler = cmdNetworkAutostart, @@ -1525,5 +1898,29 @@ const vshCmdDef networkCmds[] = { .info = info_network_uuid, .flags = 0 }, + {.name = "net-port-list", + .handler = cmdNetworkPortList, + .opts = opts_network_port_list, + .info = info_network_port_list, + .flags = 0 + }, + {.name = "net-port-create", + .handler = cmdNetworkPortCreate, + .opts = opts_network_port_create, + .info = info_network_port_create, + .flags = 0 + }, + {.name = "net-port-dumpxml", + .handler = cmdNetworkPortDumpXML, + .opts = opts_network_port_dumpxml, + .info = info_network_port_dumpxml, + .flags = 0 + }, + {.name = "net-port-delete", + .handler = cmdNetworkPortDelete, + .opts = opts_network_port_delete, + .info = info_network_port_delete, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virsh-network.h b/tools/virsh-network.h index 9c86eb5bc9..18b8e4365f 100644 --- a/tools/virsh-network.h +++ b/tools/virsh-network.h @@ -27,6 +27,11 @@ virNetworkPtr virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, const char **name, unsigned int flags); +virNetworkPortPtr +virshCommandOptNetworkPort(vshControl *ctl, const vshCmd *cmd, + virNetworkPtr net, + const char **name); + /* default is lookup by Name and UUID */ # define virshCommandOptNetwork(_ctl, _cmd, _name) \ virshCommandOptNetworkBy(_ctl, _cmd, _name, \ -- 2.20.1

This initial implementation just wires up the APIs and does tracking of the port XML definitions. It is not yet integrated into the resource allocation logic. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/network/bridge_driver.c | 400 ++++++++++++++++++++++++++++++++++++ 1 file changed, 400 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 74dcd0c44a..daeedf2995 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2755,6 +2755,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver, VIR_DEBUG("Beginning network startup process"); + virNetworkObjDeleteAllPorts(obj, driver->stateDir); + VIR_DEBUG("Setting current network def as transient"); if (virNetworkObjSetDefTransient(obj, true) < 0) goto cleanup; @@ -3930,6 +3932,9 @@ networkDestroy(virNetworkPtr net) if ((ret = networkShutdownNetwork(driver, obj)) < 0) goto cleanup; + + virNetworkObjDeleteAllPorts(obj, driver->stateDir); + /* @def replaced in virNetworkObjUnsetDefTransient*/ def = virNetworkObjGetDef(obj); @@ -5511,6 +5516,394 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, } +static virNetworkPortPtr +networkPortLookupByUUID(virNetworkPtr net, + const unsigned char *uuid) +{ + virNetworkObjPtr obj; + virNetworkDefPtr def; + virNetworkPortDefPtr portdef = NULL; + virNetworkPortPtr ret = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + + if (!(obj = networkObjFromNetwork(net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (!(portdef = virNetworkObjLookupPort(obj, uuid))) + goto cleanup; + + if (virNetworkPortLookupByUUIDEnsureACL(net->conn, def, portdef) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + ret = virGetNetworkPort(net, uuid); + + cleanup: + virNetworkObjEndAPI(&obj); + return ret; +} + + +static virNetworkPortPtr +networkPortCreateXML(virNetworkPtr net, + const char *xmldesc, + unsigned int flags) +{ + virNetworkDriverStatePtr driver = networkGetDriver(); + virNetworkObjPtr obj; + virNetworkDefPtr def; + virNetworkPortDefPtr portdef = NULL; + virNetworkPortPtr ret = NULL; + int rc; + + virCheckFlags(VIR_NETWORK_PORT_CREATE_RECLAIM, NULL); + + if (!(obj = networkObjFromNetwork(net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (!(portdef = virNetworkPortDefParseString(xmldesc))) + goto cleanup; + + if (virNetworkPortCreateXMLEnsureACL(net->conn, def, portdef) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + if (portdef->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_NONE) { + if (flags & VIR_NETWORK_PORT_CREATE_RECLAIM) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Port reclaim requested but plug type is none")); + goto cleanup; + } + } else { + if (!(flags & VIR_NETWORK_PORT_CREATE_RECLAIM)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Port reclaim not requested but plug type is not none")); + goto cleanup; + } + } + + if (flags & VIR_NETWORK_PORT_CREATE_RECLAIM) + rc = networkNotifyPort(obj, portdef); + else + rc = networkAllocatePort(obj, portdef); + if (rc < 0) { + virErrorPtr saved; + saved = virSaveLastError(); + ignore_value(networkReleasePort(obj, portdef)); + virSetError(saved); + virFreeError(saved); + goto cleanup; + } + + if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) { + virNetworkPortDefFree(portdef); + goto cleanup; + } + + ret = virGetNetworkPort(net, portdef->uuid); + cleanup: + virNetworkObjEndAPI(&obj); + return ret; +} + + +static char * +networkPortGetXMLDesc(virNetworkPortPtr port, + unsigned int flags) +{ + virNetworkObjPtr obj; + virNetworkDefPtr def; + virNetworkPortDefPtr portdef = NULL; + char *ret = NULL; + + virCheckFlags(0, NULL); + + if (!(obj = networkObjFromNetwork(port->net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (!(portdef = virNetworkObjLookupPort(obj, port->uuid))) + goto cleanup; + + if (virNetworkPortGetXMLDescEnsureACL(port->net->conn, def, portdef) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + if (!(ret = virNetworkPortDefFormat(portdef))) + goto cleanup; + + cleanup: + virNetworkObjEndAPI(&obj); + return ret; +} + + +static int +networkPortDelete(virNetworkPortPtr port, + unsigned int flags) +{ + virNetworkDriverStatePtr driver = networkGetDriver(); + virNetworkObjPtr obj; + virNetworkDefPtr def; + virNetworkPortDefPtr portdef; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(port->net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (!(portdef = virNetworkObjLookupPort(obj, port->uuid))) + goto cleanup; + + if (virNetworkPortDeleteEnsureACL(port->net->conn, def, portdef) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + if (networkReleasePort(obj, portdef) < 0) + goto cleanup; + + virNetworkObjDeletePort(obj, port->uuid, driver->stateDir); + + ret = 0; + cleanup: + virNetworkObjEndAPI(&obj); + return ret; +} + + +static int +networkPortSetParameters(virNetworkPortPtr port, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virNetworkDriverStatePtr driver = networkGetDriver(); + virNetworkObjPtr obj; + virNetworkDefPtr def; + virNetworkPortDefPtr portdef; + virNetDevBandwidthPtr bandwidth = NULL; + char *dir = NULL; + int ret = -1; + size_t i; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(port->net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (!(portdef = virNetworkObjLookupPort(obj, port->uuid))) + goto cleanup; + + if (virNetworkPortSetParametersEnsureACL(port->net->conn, def, portdef) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + if (!(dir = virNetworkObjGetPortStatusDir(obj, driver->stateDir))) + goto cleanup; + + if ((VIR_ALLOC(bandwidth) < 0) || + (VIR_ALLOC(bandwidth->in) < 0) || + (VIR_ALLOC(bandwidth->out) < 0)) + goto cleanup; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = &params[i]; + + if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_IN_AVERAGE)) { + bandwidth->in->average = param->value.ui; + } else if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_IN_PEAK)) { + bandwidth->in->peak = param->value.ui; + } else if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_IN_BURST)) { + bandwidth->in->burst = param->value.ui; + } else if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_IN_FLOOR)) { + bandwidth->in->floor = param->value.ui; + } else if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_OUT_AVERAGE)) { + bandwidth->out->average = param->value.ui; + } else if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_OUT_PEAK)) { + bandwidth->out->peak = param->value.ui; + } else if (STREQ(param->field, VIR_NETWORK_PORT_BANDWIDTH_OUT_BURST)) { + bandwidth->out->burst = param->value.ui; + } + } + + /* average or floor are mandatory, peak and burst are optional. + * So if no average or floor is given, we free inbound/outbound + * here which causes inbound/outbound to not be set. */ + if (!bandwidth->in->average && !bandwidth->in->floor) + VIR_FREE(bandwidth->in); + if (!bandwidth->out->average) + VIR_FREE(bandwidth->out); + + if (networkUpdatePortBandwidth(obj, + &portdef->mac, + &portdef->class_id, + portdef->bandwidth, + bandwidth) < 0) + goto cleanup; + + virNetDevBandwidthFree(portdef->bandwidth); + portdef->bandwidth = bandwidth; + bandwidth = NULL; + + if (virNetworkPortDefSaveStatus(portdef, dir) < 0) + goto cleanup; + + ret = 0; + cleanup: + virNetDevBandwidthFree(bandwidth); + virNetworkObjEndAPI(&obj); + VIR_FREE(dir); + return ret; +} + + +static int +networkPortGetParameters(virNetworkPortPtr port, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virNetworkObjPtr obj; + virNetworkDefPtr def; + virNetworkPortDefPtr portdef; + int maxparams = 0; + int ret = -1; + + virCheckFlags(0, -1); + + *params = NULL; + *nparams = 0; + + if (!(obj = networkObjFromNetwork(port->net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (!(portdef = virNetworkObjLookupPort(obj, port->uuid))) + goto cleanup; + + if (virNetworkPortGetParametersEnsureACL(port->net->conn, def, portdef) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + if (portdef->bandwidth) { + if ((portdef->bandwidth->in != NULL) && + (virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_AVERAGE, + portdef->bandwidth->in->average) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_PEAK, + portdef->bandwidth->in->peak) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_FLOOR, + portdef->bandwidth->in->floor) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_BURST, + portdef->bandwidth->in->burst) < 0)) + goto cleanup; + + if ((portdef->bandwidth->out != NULL) && + (virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_OUT_AVERAGE, + portdef->bandwidth->out->average) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_OUT_PEAK, + portdef->bandwidth->out->peak) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_OUT_BURST, + portdef->bandwidth->out->burst) < 0)) + goto cleanup; + } + + ret = 0; + cleanup: + virNetworkObjEndAPI(&obj); + return ret; +} + + +static int +networkListAllPorts(virNetworkPtr net, + virNetworkPortPtr **ports, + unsigned int flags) +{ + virNetworkObjPtr obj; + virNetworkDefPtr def; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(net))) + return ret; + + def = virNetworkObjGetDef(obj); + + if (virNetworkListAllPortsEnsureACL(net->conn, def) < 0) + goto cleanup; + + if (!virNetworkObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("network '%s' is not active"), + def->name); + goto cleanup; + } + + ret = virNetworkObjPortListExport(net, obj, ports, + virNetworkListAllPortsCheckACL); + + cleanup: + virNetworkObjEndAPI(&obj); + return ret; +} + + static virNetworkDriver networkDriver = { .name = "bridge", .connectNumOfNetworks = networkConnectNumOfNetworks, /* 0.2.0 */ @@ -5535,6 +5928,13 @@ static virNetworkDriver networkDriver = { .networkIsActive = networkIsActive, /* 0.7.3 */ .networkIsPersistent = networkIsPersistent, /* 0.7.3 */ .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.2.6 */ + .networkPortLookupByUUID = networkPortLookupByUUID, /* 5.2.0 */ + .networkPortCreateXML = networkPortCreateXML, /* 5.2.0 */ + .networkPortGetXMLDesc = networkPortGetXMLDesc, /* 5.2.0 */ + .networkPortDelete = networkPortDelete, /* 5.2.0 */ + .networkListAllPorts = networkListAllPorts, /* 5.2.0 */ + .networkPortGetParameters = networkPortGetParameters, /* 5.2.0 */ + .networkPortSetParameters = networkPortSetParameters, /* 5.2.0 */ }; -- 2.20.1

The various steps involved in reconnecting to a domain may cause updates to the virDomainObj struct that need to be reflected in the saved status file. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libxl/libxl_driver.c | 3 +++ src/lxc/lxc_process.c | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 04f218303a..dbca81e94d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -454,6 +454,9 @@ libxlReconnectDomain(virDomainObjPtr vm, libxlReconnectNotifyNets(vm->def); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, cfg->caps) < 0) + VIR_WARN("Cannot update XML for running Xen guest %s", vm->def->name); + /* now that we know it's reconnected call the hook if present */ if (virHookPresent(VIR_HOOK_DRIVER_LIBXL) && STRNEQ("Domain-0", vm->def->name)) { diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index a66bf66839..90148ce877 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1676,6 +1676,7 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm, { virLXCDriverPtr driver = opaque; virLXCDomainObjPrivatePtr priv; + virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); int ret = -1; virObjectLock(vm); @@ -1718,6 +1719,9 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm, virLXCProcessReconnectNotifyNets(vm->def); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) + VIR_WARN("Cannot update XML for running LXC guest %s", vm->def->name); + /* now that we know it's reconnected call the hook if present */ if (virHookPresent(VIR_HOOK_DRIVER_LXC)) { char *xml = virDomainDefFormat(vm->def, driver->caps, 0); @@ -1738,6 +1742,7 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm, ret = 0; cleanup: + virObjectUnref(cfg); virObjectUnlock(vm); return ret; -- 2.20.1

Change the domain conf so invoke the new network port public APIs instead of the network callbacks. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_conf.c | 262 ++++++++++++++++++++++++------------ src/conf/domain_conf.h | 26 ---- src/libvirt_private.syms | 1 - src/network/bridge_driver.c | 200 --------------------------- 4 files changed, 178 insertions(+), 311 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fee5df8f40..486638135b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30252,6 +30252,7 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface, if (VIR_STRDUP(actual->data.bridge.brname, port->plug.bridge.brname) < 0) goto error; + actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; actual->data.bridge.macTableManager = port->plug.bridge.macTableManager; break; @@ -30259,10 +30260,12 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface, if (VIR_STRDUP(actual->data.direct.linkdev, port->plug.direct.linkdev) < 0) goto error; + actual->type = VIR_DOMAIN_NET_TYPE_DIRECT; actual->data.direct.mode = port->plug.direct.mode; break; case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: + actual->type = VIR_DOMAIN_NET_TYPE_HOSTDEV; actual->data.hostdev.def.parent = iface; actual->data.hostdev.def.info = &iface->info; actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; @@ -30454,45 +30457,100 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, return NULL; } -static virDomainNetAllocateActualDeviceImpl netAllocate; -static virDomainNetNotifyActualDeviceImpl netNotify; -static virDomainNetReleaseActualDeviceImpl netRelease; -static virDomainNetBandwidthUpdateImpl netBandwidthUpdate; - -void -virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, - virDomainNetNotifyActualDeviceImpl notify, - virDomainNetReleaseActualDeviceImpl release, - virDomainNetBandwidthUpdateImpl bandwidthUpdate) -{ - netAllocate = allocate; - netNotify = notify; - netRelease = release; - netBandwidthUpdate = bandwidthUpdate; -} - -int -virDomainNetAllocateActualDevice(virConnectPtr conn, - virDomainDefPtr dom, - virDomainNetDefPtr iface) +static int +virDomainNetCreatePort(virConnectPtr conn, + virDomainDefPtr dom, + virDomainNetDefPtr iface, + unsigned int flags) { virNetworkPtr net = NULL; int ret = -1; + virNetworkPortDefPtr portdef = NULL; + virNetworkPortPtr port = NULL; + char *portxml = NULL; + virErrorPtr saved; - if (!netAllocate) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", - _("Virtual networking driver is not available")); + if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) return -1; + + if (flags & VIR_NETWORK_PORT_CREATE_RECLAIM) { + virDomainNetType actualType = virDomainNetGetActualType(iface); + + /* if we're restarting libvirtd after an upgrade from a version + * that didn't save bridge name in actualNetDef for + * actualType==network, we need to copy it in so that it will be + * available in all cases + */ + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK && + !iface->data.network.actual->data.bridge.brname) { + char *bridge = virNetworkGetBridgeName(net); + if (!bridge) + goto cleanup; + VIR_FREE(iface->data.network.actual->data.bridge.brname); + iface->data.network.actual->data.bridge.brname = bridge; + } + + /* Older libvirtd uses actualType==network, but we now + * just use actualType==bridge, as nothing needs to + * distinguish the two cases, and this simplifies virt + * drive code */ + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { + iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + actualType = VIR_DOMAIN_NET_TYPE_BRIDGE; + } + + if (!(portdef = virDomainNetDefActualToNetworkPort(dom, iface))) + goto cleanup; + } else { + if (!(portdef = virDomainNetDefToNetworkPort(dom, iface))) + goto cleanup; } - if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) - return -1; + if (!(portxml = virNetworkPortDefFormat(portdef))) + goto cleanup; - ret = netAllocate(net, dom, iface); + virNetworkPortDefFree(portdef); + portdef = NULL; + if (!(port = virNetworkPortCreateXML(net, portxml, flags))) + goto cleanup; + + VIR_FREE(portxml); + + if (!(portxml = virNetworkPortGetXMLDesc(port, 0))) + goto deleteport; + + if (!(portdef = virNetworkPortDefParseString(portxml))) + goto deleteport; + + if (virDomainNetDefActualFromNetworkPort(iface, portdef) < 0) + goto deleteport; + + virNetworkPortGetUUID(port, iface->data.network.portid); + + ret = 0; + cleanup: + virNetworkPortDefFree(portdef); + VIR_FREE(portxml); + virObjectUnref(port); virObjectUnref(net); return ret; + + deleteport: + saved = virSaveLastError(); + virNetworkPortDelete(port, 0); + virSetError(saved); + virFreeError(saved); + goto cleanup; +} + +int +virDomainNetAllocateActualDevice(virConnectPtr conn, + virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + return virDomainNetCreatePort(conn, dom, iface, 0); } void @@ -30500,94 +30558,130 @@ virDomainNetNotifyActualDevice(virConnectPtr conn, virDomainDefPtr dom, virDomainNetDefPtr iface) { - virDomainNetType actualType = virDomainNetGetActualType(iface); - virNetworkPtr net = NULL; - - if (!netNotify) - return; - - if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) - return; - - /* if we're restarting libvirtd after an upgrade from a version - * that didn't save bridge name in actualNetDef for - * actualType==network, we need to copy it in so that it will be - * available in all cases - */ - if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK && - !iface->data.network.actual->data.bridge.brname) { - char *bridge = virNetworkGetBridgeName(net); - if (!bridge) - goto cleanup; - VIR_FREE(iface->data.network.actual->data.bridge.brname); - iface->data.network.actual->data.bridge.brname = bridge; - } - - /* Older libvirtd uses actualType==network, but we now - * just use actualType==bridge, as nothing needs to - * distinguish the two cases, and this simplifies virt - * drive code */ - if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { - iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - actualType = VIR_DOMAIN_NET_TYPE_BRIDGE; + if (!virUUIDIsValid(iface->data.network.portid)) { + if (virDomainNetCreatePort(conn, dom, iface, + VIR_NETWORK_PORT_CREATE_RECLAIM) < 0) + return; } - - if (netNotify(net, dom, iface) < 0) - goto cleanup; - if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_BRIDGE) { /* * NB: we can't notify the guest of any MTU change anyway, * so there is no point in trying to learn the actualMTU * (final arg to virNetDevTapReattachBridge()) */ - if (virNetDevTapReattachBridge(iface->ifname, - iface->data.network.actual->data.bridge.brname, - &iface->mac, dom->uuid, - virDomainNetGetActualVirtPortProfile(iface), - virDomainNetGetActualVlan(iface), - iface->mtu, NULL) < 0) - goto cleanup; + ignore_value(virNetDevTapReattachBridge(iface->ifname, + iface->data.network.actual->data.bridge.brname, + &iface->mac, dom->uuid, + virDomainNetGetActualVirtPortProfile(iface), + virDomainNetGetActualVlan(iface), + iface->mtu, NULL)); } - - cleanup: - virObjectUnref(net); } int virDomainNetReleaseActualDevice(virConnectPtr conn, - virDomainDefPtr dom, + virDomainDefPtr dom ATTRIBUTE_UNUSED, virDomainNetDefPtr iface) { virNetworkPtr net = NULL; - int ret; - - if (!netRelease) - return 0; + virNetworkPortPtr port = NULL; + int ret = -1; if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) - return -1; + goto cleanup; - ret = netRelease(net, dom, iface); + if (!(port = virNetworkPortLookupByUUID(net, iface->data.network.portid))) + goto cleanup; + + if (virNetworkPortDelete(port, 0) < 0) + goto cleanup; + cleanup: + virObjectUnref(port); virObjectUnref(net); return ret; } +static int +virDomainNetBandwidthToTypedParams(virNetDevBandwidthPtr bandwidth, + virTypedParameterPtr *params, + int *nparams) +{ + int maxparams = 0; + + if ((bandwidth->in != NULL) && + (virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_AVERAGE, + bandwidth->in->average) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_PEAK, + bandwidth->in->peak) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_FLOOR, + bandwidth->in->floor) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_IN_BURST, + bandwidth->in->burst) < 0)) + goto error; + + if ((bandwidth->out != NULL) && + (virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_OUT_AVERAGE, + bandwidth->out->average) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_OUT_PEAK, + bandwidth->out->peak) < 0 || + virTypedParamsAddUInt(params, nparams, &maxparams, + VIR_NETWORK_PORT_BANDWIDTH_OUT_BURST, + bandwidth->out->burst) < 0)) + goto error; + + return 0; + + error: + virTypedParamsFree(*params, *nparams); + *params = NULL; + *nparams = 0; + return -1; +} + + int virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth) { - if (!netBandwidthUpdate) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", - _("Virtual networking driver is not available")); - return -1; - } + virNetworkPtr net = NULL; + virNetworkPortPtr port = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + virConnectPtr conn = NULL; + int ret = -1; + + if (!(conn = virGetConnectNetwork())) + goto cleanup; - return netBandwidthUpdate(iface, newBandwidth); + if (!(net = virNetworkLookupByName(conn, iface->data.network.name))) + goto cleanup; + + if (!(port = virNetworkPortLookupByUUID(net, iface->data.network.portid))) + goto cleanup; + + if (virDomainNetBandwidthToTypedParams(newBandwidth, &params, &nparams) < 0) + goto cleanup; + + if (virNetworkPortSetParameters(port, params, nparams, 0) < 0) + goto cleanup; + + ret = 0; + cleanup: + virObjectUnref(conn); + virTypedParamsFree(params, nparams); + virObjectUnref(port); + virObjectUnref(net); + return ret; } /* virDomainNetResolveActualType: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4df4c51474..7b1ee33c72 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3531,32 +3531,6 @@ virNetworkPortDefPtr virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, virDomainNetDefPtr iface); -typedef int -(*virDomainNetAllocateActualDeviceImpl)(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface); - -typedef int -(*virDomainNetNotifyActualDeviceImpl)(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface); - -typedef int -(*virDomainNetReleaseActualDeviceImpl)(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface); - -typedef int -(*virDomainNetBandwidthUpdateImpl)(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth); - - -void -virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, - virDomainNetNotifyActualDeviceImpl notify, - virDomainNetReleaseActualDeviceImpl release, - virDomainNetBandwidthUpdateImpl bandwidthUpdate); - int virDomainNetAllocateActualDevice(virConnectPtr conn, virDomainDefPtr dom, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 640ac00d1c..3cff1a5c1a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -483,7 +483,6 @@ virDomainNetReleaseActualDevice; virDomainNetRemove; virDomainNetRemoveHostdev; virDomainNetResolveActualType; -virDomainNetSetDeviceImpl; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index daeedf2995..5ef5b91581 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4686,52 +4686,6 @@ networkAllocatePort(virNetworkObjPtr obj, } -static int -networkAllocateActualDevice(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface) -{ - virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkPortDefPtr port = NULL; - virNetworkObjPtr obj; - int ret = -1; - - obj = virNetworkObjFindByName(driver->networks, net->name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - net->name); - return -1; - } - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - goto cleanup; - } - - if (!(port = virDomainNetDefToNetworkPort(dom, iface))) - goto cleanup; - - if (networkAllocatePort(obj, port) < 0) - goto cleanup; - - VIR_DEBUG("Populating net def"); - if (virDomainNetDefActualFromNetworkPort(iface, port) < 0) - goto cleanup; - - ret = 0; - cleanup: - if (ret < 0) { - virDomainActualNetDefFree(iface->data.network.actual); - iface->data.network.actual = NULL; - } - virNetworkPortDefFree(port); - virNetworkObjEndAPI(&obj); - return ret; -} - - /* networkNotifyPort: * @obj: the network to notify * @port: the port definition to notify @@ -4887,54 +4841,6 @@ networkNotifyPort(virNetworkObjPtr obj, } -static int -networkNotifyActualDevice(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface) -{ - virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj; - virNetworkDefPtr netdef; - virNetworkPortDefPtr port = NULL; - int ret = -1; - - obj = virNetworkObjFindByName(driver->networks, net->name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - net->name); - goto cleanup; - } - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - goto cleanup; - } - - netdef = virNetworkObjGetDef(obj); - - if (!virNetworkObjIsActive(obj)) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("network '%s' is not active"), - netdef->name); - goto cleanup; - } - - if (!(port = virDomainNetDefActualToNetworkPort(dom, iface))) - goto cleanup; - - if (networkNotifyPort(obj, port) < 0) - goto cleanup; - - ret = 0; - cleanup: - virNetworkObjEndAPI(&obj); - virNetworkPortDefFree(port); - return ret; -} - - /* networkReleasePort: * @obj: the network to release from * @port: the port definition to release @@ -5050,63 +4956,6 @@ networkReleasePort(virNetworkObjPtr obj, } -/* networkReleaseActualDevice: - * @dom: domain definition that @iface belongs to - * @iface: a domain's NetDef (interface definition) - * - * Given a domain <interface> element that previously had its <actual> - * element filled in (and possibly a physical device allocated to it), - * free up the physical device for use by someone else, and free the - * virDomainActualNetDef. - * - * Returns 0 on success, -1 on failure. - */ -static int -networkReleaseActualDevice(virNetworkPtr net, - virDomainDefPtr dom, - virDomainNetDefPtr iface) -{ - virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj; - virNetworkPortDefPtr port = NULL; - int ret = -1; - - obj = virNetworkObjFindByName(driver->networks, net->name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - net->name); - goto cleanup; - } - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - goto cleanup; - } - - if (iface->data.network.actual == NULL) { - ret = 0; - goto cleanup; - } - - if (!(port = virDomainNetDefActualToNetworkPort(dom, iface))) - goto cleanup; - - if (networkReleasePort(obj, port) < 0) - goto cleanup; - - ret = 0; - cleanup: - virNetworkObjEndAPI(&obj); - if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - virDomainActualNetDefFree(iface->data.network.actual); - iface->data.network.actual = NULL; - } - virNetworkPortDefFree(port); - return ret; -} - /** * networkCheckBandwidth: @@ -5474,48 +5323,6 @@ networkUpdatePortBandwidth(virNetworkObjPtr obj, } -static int -networkBandwidthUpdate(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) -{ - virNetworkDriverStatePtr driver = networkGetDriver(); - virNetworkObjPtr obj = NULL; - virNetDevBandwidthPtr oldBandwidth = virDomainNetGetActualBandwidth(iface); - int ret = -1; - - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Expected a interface for a virtual network")); - return -1; - } - - if (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_BRIDGE || - iface->data.network.actual->data.bridge.brname == NULL) { - /* This is not an interface that's plugged into a network. - * We don't care. Thus from our POV bandwidth change is allowed. */ - return 0; - } - - obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - iface->data.network.name); - return ret; - } - - ret = networkUpdatePortBandwidth(obj, - &iface->mac, - iface->data.network.actual ? - &iface->data.network.actual->class_id : NULL, - newBandwidth, - oldBandwidth); - - virNetworkObjEndAPI(&obj); - return ret; -} - - static virNetworkPortPtr networkPortLookupByUUID(virNetworkPtr net, const unsigned char *uuid) @@ -5972,12 +5779,5 @@ networkRegister(void) return -1; if (virRegisterStateDriver(&networkStateDriver) < 0) return -1; - - virDomainNetSetDeviceImpl( - networkAllocateActualDevice, - networkNotifyActualDevice, - networkReleaseActualDevice, - networkBandwidthUpdate); - return 0; } -- 2.20.1