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

From: "Laine Stump" <laine(a)laine.org&gt; To: libvir-list(a)redhat.com Cc: "Michal Privoznik" <mprivozn(a)redhat.com&gt; 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.

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(a)redhat.com&gt; > --- > 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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list