12:42 p.m.
so we can use it from the LXC driver as well.
---
I couldn't find a nice place to add this so I went for a separate file. I'm
happy to move this elsewhere.
Cheers,
-- Guido
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libvirt_private.syms | 4 ++
src/qemu/qemu_driver.c | 104 +----------------------------------
src/util/virdhcpinterfaces.c | 127 +++++++++++++++++++++++++++++++++++++++++++
src/util/virdhcpinterfaces.h | 34 ++++++++++++
6 files changed, 169 insertions(+), 102 deletions(-)
create mode 100644 src/util/virdhcpinterfaces.c
create mode 100644 src/util/virdhcpinterfaces.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 82e8d3e..64eaa9d 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -186,6 +186,7 @@ src/util/vircommand.c
src/util/virconf.c
src/util/vircrypto.c
src/util/virdbus.c
+src/util/virdhcpinterfaces.c
src/util/virdnsmasq.c
src/util/virerror.c
src/util/virerror.h
diff --git a/src/Makefile.am b/src/Makefile.am
index a4aef0f..bddfd21 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -104,6 +104,7 @@ UTIL_SOURCES = \
util/virconf.c util/virconf.h \
util/vircrypto.c util/vircrypto.h \
util/virdbus.c util/virdbus.h util/virdbuspriv.h \
+ util/virdhcpinterfaces.c util/virdhcpinterfaces.h \
util/virdnsmasq.c util/virdnsmasq.h \
util/virebtables.c util/virebtables.h \
util/virendian.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3d0ec9d..ea3e83c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1377,6 +1377,10 @@ virDBusMessageRead;
virDBusSetSharedBus;
+# util/virdhcpinterfaces.h
+virGetDHCPInterfaces;
+
+
# util/virdnsmasq.h
dnsmasqAddDhcpHost;
dnsmasqAddHost;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index abcdbe6..fe1cb6d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -101,6 +101,7 @@
#include "virnuma.h"
#include "dirname.h"
#include "network/bridge_driver.h"
+#include "virdhcpinterfaces.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -173,10 +174,6 @@ static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
const char *path, int oflags,
bool *needUnlink, bool *bypassSecurityDriver);
-static int qemuGetDHCPInterfaces(virDomainPtr dom,
- virDomainObjPtr vm,
- virDomainInterfacePtr **ifaces);
-
virQEMUDriverPtr qemu_driver = NULL;
@@ -19794,7 +19791,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom,
switch (source) {
case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
- ret = qemuGetDHCPInterfaces(dom, vm, ifaces);
+ ret = virGetDHCPInterfaces(dom, vm, ifaces);
break;
case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT:
@@ -19825,103 +19822,6 @@ qemuDomainInterfaceAddresses(virDomainPtr dom,
return ret;
}
-static int
-qemuGetDHCPInterfaces(virDomainPtr dom,
- virDomainObjPtr vm,
- virDomainInterfacePtr **ifaces)
-{
- int rv = -1;
- int n_leases = 0;
- size_t i, j;
- size_t ifaces_count = 0;
- virNetworkPtr network = NULL;
- char macaddr[VIR_MAC_STRING_BUFLEN];
- virDomainInterfacePtr iface = NULL;
- virNetworkDHCPLeasePtr *leases = NULL;
- virDomainInterfacePtr *ifaces_ret = NULL;
-
- if (!dom->conn->networkDriver ||
- !dom->conn->networkDriver->networkGetDHCPLeases) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Network driver does not support DHCP lease query"));
- return -1;
- }
-
- for (i = 0; i < vm->def->nnets; i++) {
- if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
- continue;
-
- virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
- virObjectUnref(network);
- network = virNetworkLookupByName(dom->conn,
- vm->def->nets[i]->data.network.name);
-
- if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
- &leases, 0)) < 0)
- goto error;
-
- if (n_leases) {
- if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
- goto error;
-
- if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
- goto error;
-
- iface = ifaces_ret[ifaces_count - 1];
- /* Assuming each lease corresponds to a separate IP */
- iface->naddrs = n_leases;
-
- if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
- goto error;
-
- if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0)
- goto cleanup;
-
- if (VIR_STRDUP(iface->hwaddr, macaddr) < 0)
- goto cleanup;
- }
-
- for (j = 0; j < n_leases; j++) {
- virNetworkDHCPLeasePtr lease = leases[j];
- virDomainIPAddressPtr ip_addr = &iface->addrs[j];
-
- if (VIR_STRDUP(ip_addr->addr, lease->ipaddr) < 0)
- goto cleanup;
-
- ip_addr->type = lease->type;
- ip_addr->prefix = lease->prefix;
- }
-
- for (j = 0; j < n_leases; j++)
- virNetworkDHCPLeaseFree(leases[j]);
-
- VIR_FREE(leases);
- }
-
- *ifaces = ifaces_ret;
- ifaces_ret = NULL;
- rv = ifaces_count;
-
- cleanup:
- virObjectUnref(network);
- if (leases) {
- for (i = 0; i < n_leases; i++)
- virNetworkDHCPLeaseFree(leases[i]);
- }
- VIR_FREE(leases);
-
- return rv;
-
- error:
- if (ifaces_ret) {
- for (i = 0; i < ifaces_count; i++)
- virDomainInterfaceFree(ifaces_ret[i]);
- }
- VIR_FREE(ifaces_ret);
-
- goto cleanup;
-}
-
static int
qemuDomainSetUserPassword(virDomainPtr dom,
diff --git a/src/util/virdhcpinterfaces.c b/src/util/virdhcpinterfaces.c
new file mode 100644
index 0000000..80135f7
--- /dev/null
+++ b/src/util/virdhcpinterfaces.c
@@ -0,0 +1,127 @@
+/*
+ * virdhcpinterfaces.c: get a domains dhcp managed interfaces
+ *
+ * Copyright (C) 2016 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/>;.
+ *
+ * Author: Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#include <config.h>
+
+#include "virdhcpinterfaces.h"
+#include "viralloc.h"
+#include "virstring.h"
+#include "datatypes.h"
+
+#define VIR_FROM_THIS VIR_FROM_AUDIT
+
+int
+virGetDHCPInterfaces(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainInterfacePtr **ifaces)
+{
+ int rv = -1;
+ int n_leases = 0;
+ size_t i, j;
+ size_t ifaces_count = 0;
+ virNetworkPtr network = NULL;
+ char macaddr[VIR_MAC_STRING_BUFLEN];
+ virDomainInterfacePtr iface = NULL;
+ virNetworkDHCPLeasePtr *leases = NULL;
+ virDomainInterfacePtr *ifaces_ret = NULL;
+
+ if (!dom->conn->networkDriver ||
+ !dom->conn->networkDriver->networkGetDHCPLeases) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Network driver does not support DHCP lease query"));
+ return -1;
+ }
+
+ for (i = 0; i < vm->def->nnets; i++) {
+ if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
+ continue;
+
+ virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
+ virObjectUnref(network);
+ network = virNetworkLookupByName(dom->conn,
+ vm->def->nets[i]->data.network.name);
+
+ if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
+ &leases, 0)) < 0)
+ goto error;
+
+ if (n_leases) {
+ if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
+ goto error;
+
+ if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
+ goto error;
+
+ iface = ifaces_ret[ifaces_count - 1];
+ /* Assuming each lease corresponds to a separate IP */
+ iface->naddrs = n_leases;
+
+ if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
+ goto error;
+
+ if (VIR_STRDUP(iface->name, vm->def->nets[i]->ifname) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(iface->hwaddr, macaddr) < 0)
+ goto cleanup;
+ }
+
+ for (j = 0; j < n_leases; j++) {
+ virNetworkDHCPLeasePtr lease = leases[j];
+ virDomainIPAddressPtr ip_addr = &iface->addrs[j];
+
+ if (VIR_STRDUP(ip_addr->addr, lease->ipaddr) < 0)
+ goto cleanup;
+
+ ip_addr->type = lease->type;
+ ip_addr->prefix = lease->prefix;
+ }
+
+ for (j = 0; j < n_leases; j++)
+ virNetworkDHCPLeaseFree(leases[j]);
+
+ VIR_FREE(leases);
+ }
+
+ *ifaces = ifaces_ret;
+ ifaces_ret = NULL;
+ rv = ifaces_count;
+
+ cleanup:
+ virObjectUnref(network);
+ if (leases) {
+ for (i = 0; i < n_leases; i++)
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ VIR_FREE(leases);
+
+ return rv;
+
+ error:
+ if (ifaces_ret) {
+ for (i = 0; i < ifaces_count; i++)
+ virDomainInterfaceFree(ifaces_ret[i]);
+ }
+ VIR_FREE(ifaces_ret);
+
+ goto cleanup;
+}
diff --git a/src/util/virdhcpinterfaces.h b/src/util/virdhcpinterfaces.h
new file mode 100644
index 0000000..c7acb8f
--- /dev/null
+++ b/src/util/virdhcpinterfaces.h
@@ -0,0 +1,34 @@
+/*
+ * virdhcpinterfaces.h: get a domains dhcp managed interfaces
+ *
+ * Copyright (C) 2016 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/>;.
+ *
+ * Author: Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#ifndef __VIR_DHCP_INTERFACES_H__
+# define __VIR_DHCP_INTERFACES_H__
+
+# include "internal.h"
+# include "virdomainobjlist.h"
+
+int
+virGetDHCPInterfaces(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainInterfacePtr **ifaces);
+
+#endif /* __VIR_DHCP_INTERFACES_H__ */
--
2.7.0.rc3
12:42 p.m.
New subject: [libvirt] [PATCH 2/2] Implement lxcDomainInterfaceAddresses
relying on DHCP so far.
---
src/lxc/lxc_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 67088c8..7d55bd3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -74,6 +74,7 @@
#include "viraccessapichecklxc.h"
#include "virhostdev.h"
#include "netdev_bandwidth_conf.h"
+#include "virdhcpinterfaces.h"
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -5771,6 +5772,47 @@ lxcDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
}
+static int
+lxcDomainInterfaceAddresses(virDomainPtr dom,
+ virDomainInterfacePtr **ifaces,
+ unsigned int source,
+ unsigned int flags)
+{
+ virDomainObjPtr vm = NULL;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto cleanup;
+ }
+
+ switch (source) {
+ case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
+ ret = virGetDHCPInterfaces(dom, vm, ifaces);
+ break;
+
+ default:
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Unknown IP address data source %d"),
+ source);
+ break;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
+
/* Function Tables */
static virHypervisorDriver lxcHypervisorDriver = {
.name = LXC_DRIVER_NAME,
@@ -5864,6 +5906,7 @@ static virHypervisorDriver lxcHypervisorDriver = {
.nodeGetFreePages = lxcNodeGetFreePages, /* 1.2.6 */
.nodeAllocPages = lxcNodeAllocPages, /* 1.2.9 */
.domainHasManagedSaveImage = lxcDomainHasManagedSaveImage, /* 1.2.13 */
+ .domainInterfaceAddresses = lxcDomainInterfaceAddresses, /* 1.3.2 */
};
static virConnectDriver lxcConnectDriver = {
--
2.7.0.rc3
12:35 p.m.
On 01/31/2016 01:42 PM, Guido Günther wrote:
so we can use it from the LXC driver as well.
---
I couldn't find a nice place to add this so I went for a separate file. I'm
happy to move this elsewhere.
A separate file is fine, but it can't be in the util directory, because
you're including a file in the conf directory (virdomainobjlist), and
files in util aren't allowed to reference files in conf (ugh, I see
we're doing it in several places anyway; I thought those had all been
cleared out; I know we at least *discussed* it). Also the function is
calling public libvirt API functions (virNetworkLookupByName and
virNetworkGetDHCPLeases, virNetworkDHCPLeaseFree,
virDomainInterfaceFree) and using datatypes from the public API, also
not allowed in the util directory.
When faced with a similar problem several years back and not seeing a
reasonable alternative, I "temporarily" created backdoor functions into
the network driver
(networkAllocateActualDevice/networkNotifyActualDevice/networkReleaseActualDevice)
which are called directly from the hypervisor drivers to allocate
physical ethernet devices from the pools of devices managed by the
network driver. This is problematic because a stub function needs to be
provided in case the network driver isn't built, and also because it
creates a hard dependency for daemon-driver-network in
daemon-driver-qemu and daemon-driver-lxc when the network driver *is*
built. I actually expected that it would be shot down in review and I'd
have to come up with something else (or, even better, that someone would
suggest a cleaner alternative), but it was acked and has been in place
since 2011 or so with apparently no complaints (commit 04711a0f3).
But of course you have a function that trawls through the domain list as
well, so it's not really appropriate to have it in the network driver
either, so...
1:56 a.m.
Hi Laine,
thanks for having a look!
On Tue, Feb 02, 2016 at 01:35:28PM -0500, Laine Stump wrote:
On 01/31/2016 01:42 PM, Guido Günther wrote:
>so we can use it from the LXC driver as well.
>---
>I couldn't find a nice place to add this so I went for a separate file. I'm
>happy to move this elsewhere.
A separate file is fine, but it can't be in the util directory, because
you're including a file in the conf directory (virdomainobjlist), and files
in util aren't allowed to reference files in conf (ugh, I see we're doing it
in several places anyway; I thought those had all been cleared out; I know
we at least *discussed* it). Also the function is calling public libvirt API
functions (virNetworkLookupByName and virNetworkGetDHCPLeases,
virNetworkDHCPLeaseFree, virDomainInterfaceFree) and using datatypes from
the public API, also not allowed in the util directory.
It kind of occurred to me while moving code around that s.th. like this
might happen but I couldn't come up with good reasons (that wouldn't
allow for exceptions), I couldn't find anything in the contribution
guidelines ether why we wouldn't in util/
* use files from conf
* use public API
except for keeping things self contained (i.e. to avoid deadlocks).
When faced with a similar problem several years back and not seeing
a
reasonable alternative, I "temporarily" created backdoor functions into the
network driver
(networkAllocateActualDevice/networkNotifyActualDevice/networkReleaseActualDevice)
which are called directly from the hypervisor drivers to allocate physical
ethernet devices from the pools of devices managed by the network driver.
This is problematic because a stub function needs to be provided in case the
network driver isn't built, and also because it creates a hard dependency
for daemon-driver-network in daemon-driver-qemu and daemon-driver-lxc when
the network driver *is* built. I actually expected that it would be shot
down in review and I'd have to come up with something else (or, even better,
that someone would suggest a cleaner alternative), but it was acked and has
been in place since 2011 or so with apparently no complaints (commit
04711a0f3).
But of course you have a function that trawls through the domain list as
well, so it's not really appropriate to have it in the network driver
either, so...
I wonder if it wouldn't be simpler to just c'n'p the function into the
LXC driver then? I didn't do so in the first place since other drivers
could benefit from this as well?
Cheers,
-- Guido
3245
days inactive
3248
days old
3 comments
2 participants
participants (2)
-
Guido Günther -
Laine Stump