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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




----- Original Message -----
> From: "Laine Stump" <laine@xxxxxxxxx>
> To: libvir-list@xxxxxxxxxx
> Cc: "Michal Privoznik" <mprivozn@xxxxxxxxxx>
> Sent: Friday, February 7, 2014 1:17:10 PM
> Subject: Re:  [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@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]