Re: [libvirt] [PATCH v2 3/3] network: Taint networks that are using hook script

7 Feb 2014

----- Original Message -----
...
From: "Laine Stump" <laine@laine.org>
To: libvir-list@redhat.com
Cc: "Michal Privoznik" <mprivozn@redhat.com>
Sent: Friday, February 7, 2014 1:17:10 PM
Subject: Re: [libvirt] [PATCH v2 3/3] network: Taint networks that are using hook script
On 02/05/2014 12:11 PM, Michal Privoznik wrote:
...
Basically, the idea is copied from domain code, where tainting
exists for a while. Currently, only one taint reason exists -
VIR_NETWORK_TAINT_HOOK to mark those networks which caused invoking
of hook script.
What's missing here is that the network status XML doesn't include a
<taint> element.
Also, I think if a network is tainted, and domain that connects to that
network should be tainted as well.
Of course what would make this more useful would be if would could
determine when a hook script actually *did* something for a particular
network/interface (since presumably people are usually going to write
their network hook scripts to only take action for particular networks
and/or domains, not for *all* networks). I don't know that there's a way
to do that without either 1) having a different hook script for each
network, or 2) trusting the hook script to return some sort of status
indicating whether or not it did anything. Obviously (2) is not a good
idea, but we may want to think about (1) in the future (for qemu and lxc
hook scripts as well) - instead of just looking for
/etc/libvirt/hook/network, we could first look for
/etc/libvirt/hook/network.${netname} and exec that instead if found (or
in addition). But I think that can be deferred until later.
Actually I kind of like the option (2). I think it could make a lot of sense
that the hook would be able to add an attribute to the network definition
xml, e.g. <bandwidth hooked="1"> so that libvirt would know that that part
has been taken care of by the hook. Of course, it might be a bad idea for
libvirt to blindly accept any kind of modification, but something like what
I propose does not seem eminently dangerous.
...
ACK if you add the <taint> element to the network status XML, and taint
the domain any time it uses a tainted network.
...
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/network_conf.c     | 16 ++++++++++++++++
 src/conf/network_conf.h     | 17 +++++++++++++++++
 src/libvirt_private.syms    |  3 +++
 src/network/bridge_driver.c | 26 ++++++++++++++++++++++++++
 4 files changed, 62 insertions(+)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index e59938c..aa881d8 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -72,6 +72,22 @@ VIR_ENUM_IMPL(virNetworkDNSForwardPlainNames,
               "yes",
               "no")
+VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
+              "hook-script");
+
+bool
+virNetworkObjTaint(virNetworkObjPtr obj,
+                   enum virNetworkTaintFlags taint)
+{
+    unsigned int flag = (1 << taint);
+
+    if (obj->taint & flag)
+        return false;
+
+    obj->taint |= flag;
+    return true;
+}
+
 virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets,
                                       const unsigned char *uuid)
 {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index b84762a..edcc49f 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -287,6 +287,8 @@ struct _virNetworkObj {
virBitmapPtr class_id; /* bitmap of class IDs for QoS */
     unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs
     */
+
+    unsigned int taint;
 };
typedef struct _virNetworkObjList virNetworkObjList;
@@ -296,12 +298,26 @@ struct _virNetworkObjList {
     virNetworkObjPtr *objs;
 };
+enum virNetworkTaintFlags {
+    VIR_NETWORK_TAINT_HOOK,                 /* Hook script was executed
over
+                                               network. We can't guarantee
+                                               connectivity or other
settings
+                                               as the script may have
played
+                                               with iptables, tc, you name
it.
+                                             */
+
+    VIR_NETWORK_TAINT_LAST
+};
+
 static inline int
 virNetworkObjIsActive(const virNetworkObj *net)
 {
     return net->active;
 }
+bool virNetworkObjTaint(virNetworkObjPtr obj,
+                        enum virNetworkTaintFlags taint);
+
 virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets,
                                       const unsigned char *uuid);
 virNetworkObjPtr virNetworkFindByName(virNetworkObjListPtr nets,
@@ -452,4 +468,5 @@ virNetworkDefUpdateSection(virNetworkDefPtr def,
                            const char *xml,
                            unsigned int flags);  /* virNetworkUpdateFlags
                            */
+VIR_ENUM_DECL(virNetworkTaint)
 #endif /* __NETWORK_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5a7637..0759d73 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -525,6 +525,7 @@ virNetworkObjListFree;
 virNetworkObjLock;
 virNetworkObjReplacePersistentDef;
 virNetworkObjSetDefTransient;
+virNetworkObjTaint;
 virNetworkObjUnlock;
 virNetworkObjUnsetDefTransient;
 virNetworkObjUpdate;
@@ -533,6 +534,8 @@ virNetworkSaveConfig;
 virNetworkSaveStatus;
 virNetworkSetBridgeMacAddr;
 virNetworkSetBridgeName;
+virNetworkTaintTypeFromString;
+virNetworkTaintTypeToString;
 virPortGroupFindByName;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1ba2b2d..f2aef48 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -112,6 +112,9 @@ static int networkPlugBandwidth(virNetworkObjPtr net,
 static int networkUnplugBandwidth(virNetworkObjPtr net,
                                   virDomainNetDefPtr iface);
+static void networkNetworkObjTaint(virNetworkObjPtr net,
+                                   enum virNetworkTaintFlags taint);
+
 static virNetworkDriverStatePtr driverState = NULL;
static virNetworkObjPtr
@@ -2024,6 +2027,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
          */
         if (hookret < 0)
             goto cleanup;
+
+        networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK);
     }
switch (network->def->forward.type) {
@@ -2067,6 +2072,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
          */
         if (hookret < 0)
             goto cleanup;
+
+        networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK);
     }
network->active = 1;
@@ -3649,6 +3656,8 @@ validate:
          */
         if (hookret < 0)
             goto error;
+
+        networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK);
     }
if (dev) {
@@ -4023,6 +4032,8 @@ success:
                     VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,
                     VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL);
         VIR_FREE(xml);
+
+        networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK);
     }
VIR_DEBUG("Releasing network %s, %d connections",
@@ -4359,3 +4370,18 @@ networkUnplugBandwidth(virNetworkObjPtr net,
 cleanup:
     return ret;
 }
+
+static void
+networkNetworkObjTaint(virNetworkObjPtr net,
+                       enum virNetworkTaintFlags taint)
+{
+    if (virNetworkObjTaint(net, taint)) {
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(net->def->uuid, uuidstr);
+
+        VIR_WARN("Network name='%s' uuid=%s is tainted: %s",
+                 net->def->name,
+                 uuidstr,
+                 virNetworkTaintTypeToString(taint));
+    }
+}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


    

Re: [libvirt] [PATCH v2 3/3] network: Taint networks that are using hook script

Antoni Segura Puimedon