[libvirt] [PATCH] For comment/critique only - netcf backend for virInterface*.

Laine Stump laine at laine.org
Tue Jun 23 18:08:29 UTC 2009


This is the backend for the interface driver (virInterface*()) that
uses netcf. There are a few issues with it:

1) It doesn't yet implement the backend for
   virConnectListDefinedInterfaces() and
   virConnectNumOfDefinedInterfaces() (although it does use my patched
   netcf API to list only active interfaces for
   virConnectListInterfaces()). (That patch isn't committed to netcf
   yet, btw; it's functional, but a bit rough and hackish)

2) interfaceLookupByMACString() still uses the old arg convention for
   ncf_lookup_by_mac_string(), which returned a single interface. That
   function has been changed to potentially return multiple
   interfaces.  To fully support that, we would need to change the
   libvirt API. What should we do here?

3) The XML in interfaceDefineXML() isn't validated, although the spot to
   do it is clearly marked.

4) There's no comment in the place where we could, in the future, transform
   the XML returned to interfaceGetXMLDesc() by netcf. Currently the two
   use the same RNG. Since it would be a null transform, and that will only
   change under our control, that isn't an *immediate* problem, but shouldn't
   be forgotten.

Parts that I'm unsure of:

1) changes to the Makefile, particularly surrounding "DRIVER_MODULES".

2) Is it registered in the proper places?

3) Could it (should it?) be added to other hypervisor drivers?
   (Currently I only put it in qemud.c)

---
 qemud/Makefile.am      |    4 +
 qemud/qemud.c          |    6 +
 src/Makefile.am        |   16 ++-
 src/interface_driver.c |  381 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/interface_driver.h |   34 +++++
 5 files changed, 440 insertions(+), 1 deletions(-)
 create mode 100644 src/interface_driver.c
 create mode 100644 src/interface_driver.h

diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index 403846a..9f982ba 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -134,6 +134,10 @@ if WITH_NETWORK
     libvirtd_LDADD += ../src/libvirt_driver_network.la
 endif
 
+if WITH_NETCF
+    libvirtd_LDADD += ../src/libvirt_driver_interface.la
+endif
+
 if WITH_NODE_DEVICES
     libvirtd_LDADD += ../src/libvirt_driver_nodedev.la
 endif
diff --git a/qemud/qemud.c b/qemud/qemud.c
index b5e3665..9326ce9 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -81,6 +81,9 @@
 #ifdef WITH_NETWORK
 #include "network_driver.h"
 #endif
+#ifdef WITH_NETCF
+#include "interface_driver.h"
+#endif
 #ifdef WITH_STORAGE_DIR
 #include "storage_driver.h"
 #endif
@@ -822,6 +825,9 @@ static struct qemud_server *qemudInitialize(int sigread) {
 #ifdef WITH_NETWORK
     networkRegister();
 #endif
+#ifdef WITH_NETCF
+    interfaceRegister();
+#endif
 #ifdef WITH_STORAGE_DIR
     storageRegister();
 #endif
diff --git a/src/Makefile.am b/src/Makefile.am
index f65e7ad..f5bf1b8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -154,6 +154,9 @@ ONE_DRIVER_SOURCES =					\
 NETWORK_DRIVER_SOURCES =					\
 		network_driver.h network_driver.c
 
+INTERFACE_DRIVER_SOURCES =					\
+		interface_driver.h interface_driver.c
+
 # Storage backend specific impls
 STORAGE_DRIVER_SOURCES =					\
 		storage_driver.h storage_driver.c		\
@@ -381,8 +384,18 @@ libvirt_driver_network_la_SOURCES = $(NETWORK_DRIVER_SOURCES)
 endif
 
 if WITH_NETCF
-libvirt_driver_interface_la_CFLAGS = $(NETCF_CFLAGS)
 libvirt_driver_interface_la_LDFLAGS = $(NETCF_LIBS)
+libvirt_driver_interface_la_CFLAGS = $(NETCF_CFLAGS)
+if WITH_DRIVER_MODULES
+mod_LTLIBRARIES += libvirt_driver_interface.la
+else
+noinst_LTLIBRARIES += libvirt_driver_interface.la
+libvirt_la_LIBADD += libvirt_driver_interface.la
+endif
+if WITH_DRIVER_MODULES
+libvirt_driver_interface_la_LDFLAGS += -module -avoid-version
+endif
+libvirt_driver_interface_la_SOURCES = $(INTERFACE_DRIVER_SOURCES)
 endif
 
 # Needed to keep automake quiet about conditionals
@@ -467,6 +480,7 @@ EXTRA_DIST +=							\
 		$(OPENVZ_DRIVER_SOURCES)			\
 		$(VBOX_DRIVER_SOURCES)				\
 		$(NETWORK_DRIVER_SOURCES)			\
+		$(INTERFACE_DRIVER_SOURCES)			\
 		$(STORAGE_DRIVER_SOURCES)			\
 		$(STORAGE_DRIVER_FS_SOURCES)			\
 		$(STORAGE_DRIVER_LVM_SOURCES)			\
diff --git a/src/interface_driver.c b/src/interface_driver.c
new file mode 100644
index 0000000..b6cf510
--- /dev/null
+++ b/src/interface_driver.c
@@ -0,0 +1,381 @@
+/*
+ * interface_driver.c: backend driver methods to handle physical
+ *                     interface configuration using the netcf library.
+ *
+ * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006 Daniel P. Berrange
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#include <config.h>
+
+#include <sys/types.h>
+#include <sys/poll.h>
+#include <dirent.h>
+#include <limits.h>
+#include <string.h>
+#include <stdio.h>
+#include <strings.h>
+#include <stdarg.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/utsname.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <paths.h>
+#include <pwd.h>
+#include <stdio.h>
+#include <sys/wait.h>
+#include <sys/ioctl.h>
+#include <netcf.h>
+
+#include "virterror_internal.h"
+#include "datatypes.h"
+#include "interface_driver.h"
+#include "driver.h"
+#include "event.h"
+#include "buf.h"
+#include "util.h"
+#include "memory.h"
+#include "logging.h"
+
+#define VIR_FROM_THIS VIR_FROM_INTERFACE
+
+#define interfaceReportError(conn, dom, net, code, fmt...)            \
+    virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__,         \
+                           __FUNCTION__, __LINE__, fmt)
+
+/* Main driver state */
+struct interface_driver
+{
+    virMutex lock;
+    struct netcf *netcf;
+};
+
+
+static void interfaceDriverLock(struct interface_driver *driver)
+{
+    virMutexLock(&driver->lock);
+}
+
+static void interfaceDriverUnlock(struct interface_driver *driver)
+{
+    virMutexUnlock(&driver->lock);
+}
+
+
+static struct interface_driver *driverState = NULL;
+
+static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn,
+                                               virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+                                               int flags ATTRIBUTE_UNUSED)
+{
+    if (VIR_ALLOC(driverState) < 0)
+    {
+        virReportOOMError(conn);
+        goto alloc_error;
+    }
+
+    /* initialize non-0 stuff in driverState */
+    if (virMutexInit(&driverState->lock) < 0)
+    {
+        /* what error to report? */
+        goto mutex_error;
+    }
+
+    /* open netcf */
+    if (ncf_init(&driverState->netcf, NULL) != 0)
+    {
+        /* what error to report? */
+        goto netcf_error;
+    }
+
+    conn->interfacePrivateData = driverState;
+    return 0;
+
+netcf_error:
+    if (driverState->netcf)
+    {
+        ncf_close(driverState->netcf);
+    }
+    virMutexDestroy (&driverState->lock);
+mutex_error:
+    VIR_FREE(driverState);
+alloc_error:
+    return -1;
+}
+
+static int interfaceCloseInterface(virConnectPtr conn)
+{
+
+    if (conn->interfacePrivateData != NULL)
+    {
+        struct interface_driver *driver
+            = (struct interface_driver *)conn->interfacePrivateData;
+
+        /* close netcf instance */
+        ncf_close(driver->netcf);
+        /* destroy lock */
+        virMutexDestroy(&driver->lock);
+        /* free driverState */
+        VIR_FREE(driver);
+    }
+    conn->interfacePrivateData = NULL;
+    return 0;
+}
+
+static int interfaceNumInterfaces(virConnectPtr conn)
+{
+    int count;
+    struct interface_driver *driver = conn->interfacePrivateData;
+
+    interfaceDriverLock(driver);
+    count = ncf_num_of_interfaces(driver->netcf, NETCF_FLAG_ACTIVE);
+    interfaceDriverUnlock(driver);
+    return count;
+}
+
+static int interfaceListInterfaces(virConnectPtr conn, char **const names, int nnames)
+{
+    struct interface_driver *driver = conn->interfacePrivateData;
+    int count;
+
+    interfaceDriverLock(driver);
+
+    count = ncf_list_interfaces(driver->netcf, nnames, names, NETCF_FLAG_ACTIVE);
+    /* netcf cleans up in the case of errors */
+    interfaceDriverUnlock(driver);
+
+    return count;
+
+}
+
+static virInterfacePtr interfaceLookupByName(virConnectPtr conn,
+                                             const char *name)
+{
+    struct interface_driver *driver = conn->interfacePrivateData;
+    struct netcf_if *iface;
+    virInterfacePtr ret = NULL;
+
+    interfaceDriverLock(driver);
+    iface = ncf_lookup_by_name(driver->netcf, name);
+
+    if (!iface) {
+        interfaceReportError(conn, NULL, NULL, VIR_ERR_NO_INTERFACE,
+                             "%s", _("no interface with matching name"));
+        goto cleanup;
+    }
+
+    ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface));
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static virInterfacePtr interfaceLookupByMACString(virConnectPtr conn,
+                                                  const char *macstr)
+{
+    struct interface_driver *driver = conn->interfacePrivateData;
+    struct netcf_if *iface;
+    virInterfacePtr ret = NULL;
+
+    interfaceDriverLock(driver);
+    iface = ncf_lookup_by_mac_string(driver->netcf, macstr);
+
+    if (!iface) {
+        interfaceReportError(conn, NULL, NULL, VIR_ERR_NO_INTERFACE,
+                             "%s", _("no interface with matching MAC address"));
+        goto cleanup;
+    }
+
+    ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface));
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static char *interfaceGetXMLDesc(virInterfacePtr ifinfo,
+                                 unsigned int flags ATTRIBUTE_UNUSED)
+{
+    struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
+    struct netcf_if *iface = NULL;
+    char *ret = NULL;
+
+    interfaceDriverLock(driver);
+    iface = ncf_lookup_by_name(driver->netcf, ifinfo->name);
+
+    if (!iface) {
+        /* May want to check by name here, but probably pointless */
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("no interface with matching MAC Address"));
+        goto cleanup;
+    }
+
+    ret = ncf_if_xml_desc(iface);
+    if (!ret) {
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("could not get interface XML description"));
+        goto cleanup;
+    }
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static virInterfacePtr interfaceDefineXML(virConnectPtr conn,
+                                          const char *xml,
+                                          unsigned int flags ATTRIBUTE_UNUSED)
+{
+    struct interface_driver *driver = conn->interfacePrivateData;
+    struct netcf_if *iface = NULL;
+    virInterfacePtr ret = NULL;
+
+    interfaceDriverLock(driver);
+
+    /*
+     * This is where we will want to validate the XML, and possibly
+     * transform it before sending it on.
+     */
+
+    iface = ncf_define(driver->netcf, xml);
+    if (!iface) {
+        interfaceReportError(conn, NULL, NULL, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("failed to define new interface from XML"));
+        goto cleanup;
+    }
+
+    ret = virGetInterface(conn, ncf_if_name(iface), ncf_if_mac_string(iface));
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static int interfaceUndefine(virInterfacePtr ifinfo) {
+    struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
+    struct netcf_if *iface = NULL;
+    int ret = -1;
+
+    interfaceDriverLock(driver);
+
+    iface = ncf_lookup_by_name(driver->netcf, ifinfo->name);
+    if (!iface) {
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("no interface with matching MAC address"));
+        goto cleanup;
+    }
+
+    ret = ncf_if_undefine(iface);
+    if (ret < 0) {
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("failed to undefine interface"));
+        goto cleanup;
+    }
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static int interfaceCreate(virInterfacePtr ifinfo,
+                           unsigned int flags ATTRIBUTE_UNUSED)
+{
+    struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
+    struct netcf_if *iface = NULL;
+    int ret = -1;
+
+    interfaceDriverLock(driver);
+
+    iface = ncf_lookup_by_name(driver->netcf, ifinfo->name);
+    if (!iface) {
+        interfaceReportError(ifinfo->conn, NULL, ifincof, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("no interface with matching MAC address"));
+        goto cleanup;
+    }
+
+    ret = ncf_if_up(iface);
+    if (ret < 0) {
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                           "%s", _("failed to create (start) interface"));
+        goto cleanup;
+    }
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static int interfaceDestroy(virInterfacePtr ifinfo,
+                            unsigned int flags ATTRIBUTE_UNUSED)
+{
+    struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
+    struct netcf_if *iface = NULL;
+    int ret = -1;
+
+    interfaceDriverLock(driver);
+
+    iface = ncf_lookup_by_name(driver->netcf, ifinfo->name);
+    if (!iface) {
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                             "%s", _("no interface with matching MAC address"));
+        goto cleanup;
+    }
+
+    ret = ncf_if_down(iface);
+    if (ret < 0) {
+        interfaceReportError(ifinfo->conn, NULL, ifinfo, VIR_ERR_INVALID_INTERFACE,
+                             "%s", _("failed to destroy (stop) interface"));
+        goto cleanup;
+    }
+
+cleanup:
+    ncf_if_free(iface);
+    interfaceDriverUnlock(driver);
+    return ret;
+}
+
+static virInterfaceDriver interfaceDriver = {
+    "Interface",
+    interfaceOpenInterface,     /* open */
+    interfaceCloseInterface,    /* close */
+    interfaceNumInterfaces,     /* numOfInterfaces */
+    interfaceListInterfaces,    /* listInterfaces */
+    interfaceLookupByName,      /* interfaceLookupByName */
+    interfaceLookupByMACString, /* interfaceLookupByMACSTring */
+    interfaceGetXMLDesc,        /* interfaceGetXMLDesc */
+    interfaceDefineXML,         /* interfaceDefineXML */
+    interfaceUndefine,          /* interfaceUndefine */
+    interfaceCreate,            /* interfaceCreate */
+    interfaceDestroy,          /* interfaceDestroy */
+};
+
+int interfaceRegister(void) {
+    virRegisterInterfaceDriver(&interfaceDriver);
+    return 0;
+}
diff --git a/src/interface_driver.h b/src/interface_driver.h
new file mode 100644
index 0000000..be00e0b
--- /dev/null
+++ b/src/interface_driver.h
@@ -0,0 +1,34 @@
+/*
+ * interface_driver.h: core driver methods for managing physical host interfaces
+ *
+ * Copyright (C) 2006, 2007 Red Hat, Inc.
+ * Copyright (C) 2006 Daniel P. Berrange
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Daniel P. Berrange <berrange at redhat.com>
+ */
+
+
+#ifndef __VIR_INTERFACE__DRIVER_H
+#define __VIR_INTERFACE__DRIVER_H
+
+#include <config.h>
+
+#include "internal.h"
+
+int interfaceRegister(void);
+
+#endif /* __VIR_INTERFACE__DRIVER_H */
-- 
1.6.0.6




More information about the libvir-list mailing list