[vdsm] Smarter network_setup hooks

[Dan Kenigsberg]

On Wed, Jan 08, 2014 at 03:34:24PM +0100, Miguel Angel wrote:
> Hi Assaf, Thank you very much for the summary,
> 
> Just a few questions, there are things I don't understand yet:
> 
> 2014/1/8 Assaf Muller <amuller at redhat.com>
> 
> > Mid-thread summary:
> >
> > First some terminology so we're all on the same page:
> > 'request' - The setupNetworks that just arrived
> > 'running config' - If unified persistence is being used - All networking
> > on the host as VDSM sees it
> > 'expected' - If unified persistence is being used - The running config
> > after the effects of the request, but before the hook is ran
> >
> >
> correct
> 
> 
> > I think we all agree that we have to expose the 'request' to the hook.
> > However, there's two different approaches:
> > 1) Allow the hook to rewrite parts of the request (For example, if two
> > networks were sent, and the hook handled one of them, it could
> >    then delete that network from the request, so VDSM will only continue
> > to configure the 2nd network).
> >
> 
> In this case ,if the hook deleted the "2nd network", VDSM couldn't handle
> the network config persistence anymore, right?,

Right. If the before_network_setup hook decides to drop an item from the
'request', it means that neither following hooks, nor Vdsm proper, would
see it. I find it as a useful and simple sematics: the hook practically
says "I'm taking it from here", and from then on, it is repsonsible for
everything. Implementation and persistence alike.

>
> I didn't expect this use case, I only expected tweaks, etc (before or after
> network setup), for example, setting special hardware
> capabilities using ethtool or those kind of things.

Neither have I expected this. But it's a powerful tool that's relatively
easy to implement.

>
> > 2) Toni's idea with marking devices as hook-configured. So if a network is
> > over a bridge over a VLAN over a bond over three NICs, hooks
> >    could configure only the NICs (For example) and VDSM would configure
> > the rest, whereas the first idea means that the hook would
> >    have to configure the entire network (Bridge, VLANs, bonds, and all 3
> > NICs) and then remove that network from the request.
> >    The disadvantage of this method is that it would be harder to
> > implement, and have references to the hooks flag throughout all of VDSM.
> >
> >
> You mean that if the hook marked a certain device as "hook handled" then,
> VDSM would just keep this information along with
> the network config, etc... and won't do any setup, just config-keeping,
> right?

That's my understanding, too. And I share the feeling that this is error
prone: Vdsm sees the config, but must remember that it should not touch
it or act upon it.

> > Then there's the matter of IF to expose the 'running config' and
> > 'expected'.
> >
> > My main argument is that it's very easy to expose to the hooks (But then
> > why not expose other 'random' data that is easy for VDSM to calculate?),
> > and that we don't understand all usages for the setupNetworks hooks. I'd
> > rather we expose too much information than not enough and screw
> > over hook writers.
> >
> >
> We have something important here, the hooks don't need to be
> python-written, they could be bash scripts, or any other thing...
> that means that they wouldn't have access to get "running config", but they
> could calculate "expected".
>
> So, my vote here goes for "request" + "running config".
>

The "running config" is accessible to any process, albeit not in its
Vdsm representation. All the information is available if you do `ip a`
and `virsh net-list`.

I do not imagine why a hook would need the "running config"; If it does
end up needing it, it could re-calculate it just as Vdsm does. Passing
this as argument to each hook script is a premature optimization imho.

If I end up wrong, it would not be hard to add the "running config" to
the Vdsm/hook API. Removing something from an API is a much harder
task.

My vote goes to only "request".

> 
> > Either way, let's get some votes in a timely manner so we could manage to
> > implement this for 3.4.
> >
> >
> Thanks Assaf! ;)