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@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?,

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.
 
 
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?
 
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".
 
Either way, let's get some votes in a timely manner so we could manage to implement this for 3.4.


Thanks Assaf! ;)
 

Assaf Muller, Cloud Networking Engineer
Red Hat

----- Original Message -----
From: "Adam Litke" <alitke@redhat.com>
To: "Miguel Angel" <miguelangel@ajo.es>
Cc: dsulliva@redhat.com, arch@ovirt.org, vdsm-devel@fedorahosted.org
Sent: Monday, January 6, 2014 3:32:07 PM
Subject: Re: [vdsm] Smarter network_setup hooks

On 04/01/14 20:08 +0100, Miguel Angel wrote:
>Hi Adam
>
>Thanks for the feedback
>
>2014/1/3 Adam Litke <alitke@redhat.com>
>
>> On 03/01/14 12:20 +0000, Dan Kenigsberg wrote:
>>
>>> Recently, Miguel Angel Ajo (CCed) has added a nice functionality to the
>>> implementation of setupNetworks in Vdsm: two hook points where added:
>>> before and after the setupNetworks verb takes place.
>>> [....]
>>
>> Seems like a logical thing to do.  What specific mechanism do you
>> suggest for passing the JSON strings to the hook script?  If passed as
>> arguments to the hook script we would need to consider shell escaping
>> and argv length restrictions.
>>
>> As for the libvirt domain xml we pass to other hooks, we write a temporary
>file
>and we set an environment variable pointing to it before calling the script
>
>
>> What about writing these out to a special file and adding a new
>> getContext() call to the hooking module.  A script that is unconcerned
>> with the context would not require any changes.  But a script that
>> wants access would simply do:
>>
>>    ctx = hooking.getContext()
>>
>> and ctx would be the contents of the special file already decoded into
>> a native Python object for easy consumption.  This could easily be
>> extended to any hook which may want to provide some context to
>> implementors.
>>
>
>That would be nice, so scripts written in python wouldn't need to look for,
>and parse
>the file.
>
>This is an example of a simple hook:
>
>http://gerrit.ovirt.org/#/c/20330/7/tests/functional/networkTests.py  (look
>inside the ValidatesHook decorator)
>
>
>It'd be quite simplified. We would also need a "setContext()..." to update
>context with changes.

Yeah seems reasonable.  Then hook scripts can become much simpler and
easier to read and write:

ctx = hooking.getContext()
ctx['foo'] = 'bar'
hooking.setContext(ctx)

>
>
>>
>> One more question comes to mind:  Are there any pieces of information
>> that we would need to redact from the context (passwords or other
>> sensitive information)?
>>
>>
>I think there is no sensitive information as far as I know.
>
>Greetings,
>Miguel Ángel.
_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel