[ovs-dev] [PATCH] xenserver: Set fail_mode on internal bridges.

Ethan Jackson ethan at nicira.com
Wed Feb 9 07:05:01 UTC 2011


On Tue, Feb 8, 2011 at 10:43 PM, Justin Pettit <jpettit at nicira.com> wrote:
> On Feb 8, 2011, at 7:19 PM, Ethan Jackson wrote:
>
>> +# By default, the "bridge-id" external id in the Bridge table is the
>> +# same as "xs-network-uuids".  This may be overridden by defining a
>> +# "nicira-bridge-id" key in the "other_config" field of the network
>> +# record of XAPI.
>> +def get_bridge_id(br_name, default=None):
>> +    rec = get_network_by_bridge(br_name)
>> +    if rec:
>>         return rec['other_config'].get('nicira-bridge-id', default)
>
> The old code behaved similarly, but I would have expected get_bridge_id() to return "default" if "rec" is not found.

I did it this way because the old code did, but I'm fine with changing
it as part of this commit.

>
>> -def set_external_id(table, record, key, value):
>> -    col = 'external-ids:"' + key + '"="' + value + '"'
>> -    cmd = [vsctl, "--timeout=30", "-vANY:console:emer", "set", table, record, col]
>> +def vsctl_call(args):
>> +    cmd = [vsctl, "--timeout=30", "-vANY:console:emer"] + args
>
> This is pedantic, but most of the calls in this function start with a verb, so "call_vsctl" would be more consistent.

Noted

>
>> +def update_fail_mode(name):
>> +    rec = get_network_by_bridge(name)
>> +
>> +    if not rec:
>> +        return
>> +
>> +    fail_mode = rec['other_config'].get('vswitch-controller-fail-mode')
>> +
>> +    if not fail_mode:
>> +        pools = session.xenapi.pool.get_all()
>> +        if len(pools) == 1:
>> +            prec = session.xenapi.pool.get_record(pools[0])
>> +            fail_mode = prec['other_config'].get('vswitch-controller-fail-mode')
>> +
>> +    if fail_mode not in ['standalone', 'secure']:
>> +        fail_mode = 'standalone'
>> +
>> +    vsctl_call(["set", "bridge", name, "fail_mode=" + fail_mode])
>
> I didn't check that these records are all referenced properly, but it looks reasonable to me.
>

I tested it before submitting so it does work in the basic case at least.

>> @@ -257,6 +285,7 @@ def main(argv):
>>                 # so only check for "network-uuids" on creation.
>>                 if name not in bridges:
>>                     update_network_uuids(name, ids)
>> +                    update_fail_mode(name)
>
> I believe this will only get called on bridge creation.  Do you handle the case externally where the fail-mode is changed after the bridge is created?

No I don't.  I'm not sure what the requirements are on this, but my
judgement is that we don't really need to handle that case.  The xapi
fail mode setting will change very rarely so I think it's fine to
require either a reboot, call of the update plugin, or hup of the
script.  I can't think of a good way to do it otherwise without
regularly polling xapi.

>
> We should probably look at renaming this file at some point, since it's no longer descriptive.
>

yes

I'll fix this up and resubmit it tomorrow morning.




More information about the dev mailing list