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

On 08.02.2014 11:51, Laine Stump wrote: > On 02/07/2014 10:52 PM, Antoni Segura Puimedon wrote: >> >> ----- Original Message ----- >>> 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. >> 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. > > The reason I don't like option (2) is that it requires trusting the hook > to leave its mark if it modifies anything, and that's exactly why we > want to taint the networks that call a hook - because we don't/can't > trust the hook. > > I wonder if there might be some way to allow a hook to add information > to the network's xml in some well-defined location, though. This > information would not be used/trusted by libvirt at all, but would only > be there, for example, so that a later "stop/unplug" hook could retrieve > it, rather than being required to keep its state externally. > Well, we may make the hook script to return the network xml that libvirt will parse and startup. For example: 1) network with <bandwidth/> is about to start. The network XML is passed to the script. 2) The script sees <network> ... <bandwidth/> ... </network> and do all the tc magic. Then it produces the same XML minus <bandwidth/> 3) Libvirt parses the <network> ... </network> without the bandwidth knowing that the script has taken care of it. If it doesn't we may error out because <bandwidth/> is not supported yet (assuming the right type of network for this little example). The whole network startup process would be aborted then.