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

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

On 10.02.2014 15:52, Michal Privoznik wrote:
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 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
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
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
xml, e.g. <bandwidth hooked="1"> so that libvirt would know that that
has been taken care of by the hook. Of course, it might be a bad idea
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.

On a second thought, this is not such a clever idea. Users are not expected to have knowledge about libvirt internals. So for example, if they were to set <bandwidth/> themselves, they may had used some iptables rules to mark packets. So their hook script removes <bandwidth/> from the XML definition. But they leave nwfilter in, which will interfere with their iptables rules. Or even the less obvious one: <ip address=''/> with range (in which case libvirt inserts some iptables rules too).

Long story short, each configuration knob in the network xml may affect others and since it is viewed as internal implementation, it is subject to change. Hence, updating the libvirt may lead to breaking users' hook scripts.


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