[libvirt] [PATCH] interface: Check for interface (in-)activity on some operations

[Michal Privoznik]

Right now it is possible to undefine an active interface, or
destroy inactive. This patch add some checking to these operations
to prevent this. Also fix test driver.
---
 src/interface/netcf_driver.c |   83 ++++++++++++++++++++++++++++++++++++------
 src/test/test_driver.c       |    5 +++
 2 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
index 855b5a3..bf590f3 100644
--- a/src/interface/netcf_driver.c
+++ b/src/interface/netcf_driver.c
@@ -119,6 +119,40 @@ static struct netcf_if *interfaceDriverGetNetcfIF(struct netcf *ncf, virInterfac
     return iface;
 }
 
+/*
+ * interfaceObjIsActive:
+ * @iface - interface
+ *
+ * Caller MUST have driver locked to prevent race condition.
+ * Returns:
+ * -1 on error
+ *  0 on inactive interface
+ *  1 on active interface
+ */
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+interfaceObjIsActive(struct interface_driver *driver,
+                     struct netcf_if *iface,
+                     virInterfacePtr ifinfo)
+{
+    int ret = -1;
+    unsigned int flags = 0;
+
+    if (ncf_if_status(iface, &flags) < 0) {
+        const char *errmsg, *details;
+        int errcode = ncf_error(driver->netcf, &errmsg, &details);
+        interfaceReportError(netcf_to_vir_err(errcode),
+                             _("failed to get status of interface %s: %s%s%s"),
+                             ifinfo->name, errmsg, details ? " - " : "",
+                             details ? details : "");
+        goto cleanup;
+    }
+
+    ret = flags & NETCF_IFACE_ACTIVE ? 1 : 0;
+
+cleanup:
+    return ret;
+}
+
 static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn,
                                                virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                                                unsigned int flags)
@@ -438,6 +472,7 @@ static int interfaceUndefine(virInterfacePtr ifinfo) {
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
     int ret = -1;
+    int active;
 
     interfaceDriverLock(driver);
 
@@ -447,6 +482,17 @@ static int interfaceUndefine(virInterfacePtr ifinfo) {
         goto cleanup;
     }
 
+    active = interfaceObjIsActive(driver, iface, ifinfo);
+    if (active < 0) {
+        /* helper already reported error */
+        goto cleanup;
+    } else if (active) {
+        interfaceReportError(VIR_ERR_OPERATION_INVALID,
+                             _("interface %s is active"),
+                             ifinfo->name);
+        goto cleanup;
+    }
+
     ret = ncf_if_undefine(iface);
     if (ret < 0) {
         const char *errmsg, *details;
@@ -470,6 +516,7 @@ static int interfaceCreate(virInterfacePtr ifinfo,
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
     int ret = -1;
+    int active;
 
     virCheckFlags(0, -1);
 
@@ -481,6 +528,17 @@ static int interfaceCreate(virInterfacePtr ifinfo,
         goto cleanup;
     }
 
+    active = interfaceObjIsActive(driver, iface, ifinfo);
+    if (active < 0) {
+        /* helper already reported error */
+        goto cleanup;
+    } else if (active) {
+        interfaceReportError(VIR_ERR_OPERATION_INVALID,
+                             _("interface %s is already active"),
+                             ifinfo->name);
+        goto cleanup;
+    }
+
     ret = ncf_if_up(iface);
     if (ret < 0) {
         const char *errmsg, *details;
@@ -504,6 +562,7 @@ static int interfaceDestroy(virInterfacePtr ifinfo,
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
     int ret = -1;
+    int active;
 
     virCheckFlags(0, -1);
 
@@ -515,6 +574,17 @@ static int interfaceDestroy(virInterfacePtr ifinfo,
         goto cleanup;
     }
 
+    active = interfaceObjIsActive(driver, iface, ifinfo);
+    if (active < 0) {
+        /* helper already reported error */
+        goto cleanup;
+    } else if (!active) {
+        interfaceReportError(VIR_ERR_OPERATION_INVALID,
+                             _("interface %s is not active"),
+                             ifinfo->name);
+        goto cleanup;
+    }
+
     ret = ncf_if_down(iface);
     if (ret < 0) {
         const char *errmsg, *details;
@@ -536,7 +606,6 @@ static int interfaceIsActive(virInterfacePtr ifinfo)
 {
     struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
     struct netcf_if *iface = NULL;
-    unsigned int flags = 0;
     int ret = -1;
 
     interfaceDriverLock(driver);
@@ -547,17 +616,7 @@ static int interfaceIsActive(virInterfacePtr ifinfo)
         goto cleanup;
     }
 
-    if (ncf_if_status(iface, &flags) < 0) {
-        const char *errmsg, *details;
-        int errcode = ncf_error(driver->netcf, &errmsg, &details);
-        interfaceReportError(netcf_to_vir_err(errcode),
-                             _("failed to get status of interface %s: %s%s%s"),
-                             ifinfo->name, errmsg, details ? " - " : "",
-                             details ? details : "");
-        goto cleanup;
-    }
-
-    ret = flags & NETCF_IFACE_ACTIVE ? 1 : 0;
+    ret = interfaceObjIsActive(driver, iface, ifinfo);
 
 cleanup:
     ncf_if_free(iface);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index f3fb320..d165586 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3648,6 +3648,11 @@ static int testInterfaceUndefine(virInterfacePtr iface)
         goto cleanup;
     }
 
+    if (privinterface->active != 0) {
+        testError(VIR_ERR_OPERATION_INVALID, NULL);
+        goto cleanup;
+    }
+
     virInterfaceRemove(&privconn->ifaces,
                        privinterface);
     ret = 0;
-- 
1.7.5.rc3