Now that we have the ability to easily open connections to secondary
drivers, eg network:///system, it is possible to reimplement the
virDomainNetResolveActualType method in terms of the public API. This
avoids the need to have the network driver provide a callback for it.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/conf/domain_conf.c | 80 ++++++++++++++++++++++++++++++++++++++++-----
src/conf/domain_conf.h | 11 +------
src/network/bridge_driver.c | 76 +-----------------------------------------
tests/Makefile.am | 7 +---
4 files changed, 75 insertions(+), 99 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fb732a0c2a..4f50547580 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28823,7 +28823,6 @@ static virDomainNetNotifyActualDeviceImpl netNotify;
static virDomainNetReleaseActualDeviceImpl netRelease;
static virDomainNetBandwidthChangeAllowedImpl netBandwidthChangeAllowed;
static virDomainNetBandwidthUpdateImpl netBandwidthUpdate;
-static virDomainNetResolveActualTypeImpl netResolveActualType;
void
@@ -28831,15 +28830,13 @@ virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate,
virDomainNetNotifyActualDeviceImpl notify,
virDomainNetReleaseActualDeviceImpl release,
virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed,
- virDomainNetBandwidthUpdateImpl bandwidthUpdate,
- virDomainNetResolveActualTypeImpl resolveActualType)
+ virDomainNetBandwidthUpdateImpl bandwidthUpdate)
{
netAllocate = allocate;
netNotify = notify;
netRelease = release;
netBandwidthChangeAllowed = bandwidthChangeAllowed;
netBandwidthUpdate = bandwidthUpdate;
- netResolveActualType = resolveActualType;
}
int
@@ -28908,16 +28905,83 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
return netBandwidthUpdate(iface, newBandwidth);
}
+/* virDomainNetResolveActualType:
+ * @iface: the original NetDef from the domain
+ *
+ * Looks up the network reference by iface, and returns the actual
+ * type of the connection without allocating any resources.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
int
virDomainNetResolveActualType(virDomainNetDefPtr iface)
{
- if (!netResolveActualType) {
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
- _("Network device resolve type not available"));
+ virConnectPtr conn = NULL;
+ virNetworkPtr net = NULL;
+ char *xml = NULL;
+ virNetworkDefPtr def = NULL;
+ int ret = -1;
+
+ if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
+ return iface->type;
+
+ if (iface->data.network.actual)
+ return iface->data.network.actual->type;
+
+ if (!(conn = virGetConnectNetwork()))
return -1;
+
+ if (!(net = virNetworkLookupByName(conn, iface->data.network.name)))
+ goto cleanup;
+
+ if (!(xml = virNetworkGetXMLDesc(net, 0)))
+ goto cleanup;
+
+ if (!(def = virNetworkDefParseString(xml)))
+ goto cleanup;
+
+ if ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
+ (def->forward.type == 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
+ */
+ ret = VIR_DOMAIN_NET_TYPE_NETWORK;
+
+ } else if ((def->forward.type == VIR_NETWORK_FORWARD_BRIDGE) &&
+ def->bridge) {
+
+ /* <forward type='bridge'/> <bridge name='xxx'/>
+ * is VIR_DOMAIN_NET_TYPE_BRIDGE
+ */
+
+ ret = VIR_DOMAIN_NET_TYPE_BRIDGE;
+
+ } else if (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) {
+
+ ret = VIR_DOMAIN_NET_TYPE_HOSTDEV;
+
+ } else if ((def->forward.type == VIR_NETWORK_FORWARD_BRIDGE) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_PRIVATE) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_VEPA) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
+
+ /* <forward type='bridge|private|vepa|passthrough'> are all
+ * VIR_DOMAIN_NET_TYPE_DIRECT.
+ */
+
+ ret = VIR_DOMAIN_NET_TYPE_DIRECT;
+
}
- return netResolveActualType(iface);
+ cleanup:
+ virNetworkDefFree(def);
+ VIR_FREE(xml);
+ virObjectUnref(conn);
+ virObjectUnref(net);
+ return ret;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6cd81ef2de..7b450ce8f1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3476,17 +3476,13 @@ typedef int
(*virDomainNetBandwidthUpdateImpl)(virDomainNetDefPtr iface,
virNetDevBandwidthPtr newBandwidth);
-typedef int
-(*virDomainNetResolveActualTypeImpl)(virDomainNetDefPtr iface);
-
void
virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate,
virDomainNetNotifyActualDeviceImpl notify,
virDomainNetReleaseActualDeviceImpl release,
virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed,
- virDomainNetBandwidthUpdateImpl bandwidthUpdate,
- virDomainNetResolveActualTypeImpl resolveActualType);
+ virDomainNetBandwidthUpdateImpl bandwidthUpdate);
int
virDomainNetAllocateActualDevice(virDomainDefPtr dom,
@@ -3513,11 +3509,6 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
virNetDevBandwidthPtr newBandwidth)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-/* XXX this is a nasty hack and should be removed. It should
- * be by via public API by fetching XML and parsing it. Not
- * easy right now as code paths in QEMU reying on this don't
- * have a virConnectPtr handy.
- */
int
virDomainNetResolveActualType(virDomainNetDefPtr iface)
ATTRIBUTE_NONNULL(1);
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index dd6e3402ea..898c946101 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5146,79 +5146,6 @@ networkReleaseActualDevice(virDomainDefPtr dom,
}
-
-/* networkResolveActualType:
- * @iface: the original NetDef from the domain
- *
- * Looks up the network reference by iface, and returns the actual
- * type of the connection without allocating any resources.
- *
- * Returns 0 on success, -1 on failure.
- */
-static int
-networkResolveActualType(virDomainNetDefPtr iface)
-{
- virNetworkDriverStatePtr driver = networkGetDriver();
- virNetworkObjPtr obj = NULL;
- virNetworkDefPtr netdef = NULL;
- int ret = -1;
-
- if (!driver || iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
- return iface->type;
-
- if (iface->data.network.actual)
- return iface->data.network.actual->type;
-
- 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 -1;
- }
- netdef = virNetworkObjGetDef(obj);
-
- if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
- (netdef->forward.type == 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
- */
- ret = VIR_DOMAIN_NET_TYPE_NETWORK;
-
- } else if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) &&
- netdef->bridge) {
-
- /* <forward type='bridge'/> <bridge name='xxx'/>
- * is VIR_DOMAIN_NET_TYPE_BRIDGE
- */
-
- ret = VIR_DOMAIN_NET_TYPE_BRIDGE;
-
- } else if (netdef->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) {
-
- ret = VIR_DOMAIN_NET_TYPE_HOSTDEV;
-
- } else if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_PRIVATE) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_VEPA) ||
- (netdef->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
-
- /* <forward type='bridge|private|vepa|passthrough'> are all
- * VIR_DOMAIN_NET_TYPE_DIRECT.
- */
-
- ret = VIR_DOMAIN_NET_TYPE_DIRECT;
-
- }
-
- virNetworkObjEndAPI(&obj);
- return ret;
-}
-
-
/**
* networkCheckBandwidth:
* @net: network QoS
@@ -5717,8 +5644,7 @@ networkRegister(void)
networkNotifyActualDevice,
networkReleaseActualDevice,
networkBandwidthChangeAllowed,
- networkBandwidthUpdate,
- networkResolveActualType);
+ networkBandwidthUpdate);
return 0;
}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4ff3fa742a..d9b3a99477 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -280,7 +280,7 @@ test_libraries += virmocklibxl.la
endif WITH_LIBXL
if WITH_QEMU
-test_programs += qemuxml2xmltest \
+test_programs += qemuxml2argvtest qemuxml2xmltest \
qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
qemumonitortest qemumonitorjsontest qemuhotplugtest \
qemuagenttest qemucapabilitiestest qemucaps2xmltest \
@@ -288,11 +288,6 @@ test_programs += qemuxml2xmltest \
qemucommandutiltest \
qemublocktest \
$(NULL)
-if WITH_NETWORK
-# Dep on the network driver callback for resolving NIC
-# actual type. XXX remove this dep.
-test_programs += qemuxml2argvtest
-endif WITH_NETWORK
test_helpers += qemucapsprobe
test_libraries += libqemumonitortestutils.la \
libqemutestdriver.la \
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn Thu, Feb 15, 2018 at 05:50 PM +0100, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> Now that we have the ability to easily open connections to secondary
> drivers, eg network:///system, it is possible to reimplement the
> virDomainNetResolveActualType method in terms of the public API. This
> avoids the need to have the network driver provide a callback for it.
>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> src/conf/domain_conf.c | 80 ++++++++++++++++++++++++++++++++++++++++-----
> src/conf/domain_conf.h | 11 +------
> src/network/bridge_driver.c | 76 +-----------------------------------------
> tests/Makefile.am | 7 +---
> 4 files changed, 75 insertions(+), 99 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index fb732a0c2a..4f50547580 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28823,7 +28823,6 @@ static virDomainNetNotifyActualDeviceImpl netNotify;
> static virDomainNetReleaseActualDeviceImpl netRelease;
> static virDomainNetBandwidthChangeAllowedImpl netBandwidthChangeAllowed;
> static virDomainNetBandwidthUpdateImpl netBandwidthUpdate;
> -static virDomainNetResolveActualTypeImpl netResolveActualType;
>
>
> void
> @@ -28831,15 +28830,13 @@ virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate,
> virDomainNetNotifyActualDeviceImpl notify,
> virDomainNetReleaseActualDeviceImpl release,
> virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed,
> - virDomainNetBandwidthUpdateImpl bandwidthUpdate,
> - virDomainNetResolveActualTypeImpl resolveActualType)
> + virDomainNetBandwidthUpdateImpl bandwidthUpdate)
> {
> netAllocate = allocate;
> netNotify = notify;
> netRelease = release;
> netBandwidthChangeAllowed = bandwidthChangeAllowed;
> netBandwidthUpdate = bandwidthUpdate;
> - netResolveActualType = resolveActualType;
> }
>
> int
> @@ -28908,16 +28905,83 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
> return netBandwidthUpdate(iface, newBandwidth);
> }
>
> +/* virDomainNetResolveActualType:
> + * @iface: the original NetDef from the domain
> + *
> + * Looks up the network reference by iface, and returns the actual
> + * type of the connection without allocating any resources.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> int
> virDomainNetResolveActualType(virDomainNetDefPtr iface)
> {
> - if (!netResolveActualType) {
> - virReportError(VIR_ERR_NO_SUPPORT, "%s",
> - _("Network device resolve type not available"));
> + virConnectPtr conn = NULL;
> + virNetworkPtr net = NULL;
> + char *xml = NULL;
> + virNetworkDefPtr def = NULL;
> + int ret = -1;
> +
> + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> + return iface->type;
> +
> + if (iface->data.network.actual)
> + return iface->data.network.actual->type;
> +
> + if (!(conn = virGetConnectNetwork()))
> return -1;
> +
> + if (!(net = virNetworkLookupByName(conn, iface->data.network.name)))
> + goto cleanup;
> +
> + if (!(xml = virNetworkGetXMLDesc(net, 0)))
> + goto cleanup;
> +
> + if (!(def = virNetworkDefParseString(xml)))
> + goto cleanup;
> +
> + if ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
> + (def->forward.type == 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
> + */
> + ret = VIR_DOMAIN_NET_TYPE_NETWORK;
> +
> + } else if ((def->forward.type == VIR_NETWORK_FORWARD_BRIDGE) &&
> + def->bridge) {
> +
> + /* <forward type='bridge'/> <bridge name='xxx'/>
> + * is VIR_DOMAIN_NET_TYPE_BRIDGE
> + */
> +
> + ret = VIR_DOMAIN_NET_TYPE_BRIDGE;
> +
> + } else if (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) {
> +
> + ret = VIR_DOMAIN_NET_TYPE_HOSTDEV;
> +
> + } else if ((def->forward.type == VIR_NETWORK_FORWARD_BRIDGE) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_PRIVATE) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_VEPA) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
> +
> + /* <forward type='bridge|private|vepa|passthrough'> are all
> + * VIR_DOMAIN_NET_TYPE_DIRECT.
> + */
> +
> + ret = VIR_DOMAIN_NET_TYPE_DIRECT;
> +
> }
>
> - return netResolveActualType(iface);
> + cleanup:
> + virNetworkDefFree(def);
> + VIR_FREE(xml);
> + virObjectUnref(conn);
> + virObjectUnref(net);
To follow the order of ref/unref, I would swap the calls
virObjectUnref(conn) and virObjectUnref(net). Even if it's not necessary
here.
> + return ret;
> }
[…snip]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.