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

[Laine Stump]

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.

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 at 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));
> +    }
> +}