Re: [PATCH v2 3/4] xl: add vif.default.bridge

["Roger Pau Monné" <roger.pau@citrix.com>]

On 12/03/13 17:25, Ian Campbell wrote:
> On Wed, 2013-02-06 at 18:04 +0000, Roger Pau Monne wrote:
>> This is a replacement for defaultbridge xl.conf option. The now
>> deprecated defaultbridge is still supported.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> ---
>>  tools/examples/xl.conf |    3 +++
>>  tools/libxl/xl.c       |   13 +++++++++++--
> 
> No change to docs/man/xl.conf.pod.5? 

My bad, I'm going to add it now.

> 2 files changed, 14 insertions(+),
> 2 deletions(-)
>>
>> diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
>> index 9a03fff..4451176 100644
>> --- a/tools/examples/xl.conf
>> +++ b/tools/examples/xl.conf
>> @@ -23,3 +23,6 @@
>>  
>>  # default gateway device to use with vif-route hotplug script
>>  #vif.default.gatewaydev="eth0"
>> +
>> +# default bridge device to use with vif-bridge hotplug scripts
>> +#vif.default.bridge="bridge0"
> 
> Common names (at least in Linux-land) are "xenbr0" and "br0". Unless
> this conflicts massively with *BSD perhaps using one of those would be
> useful?

I use bridgeX on both Linux and BSD, but that's just my choice. br0 or
xenbr0 is probably more common, so I'm going to change it.

> 
> 
>> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
>> index f657216..100ab32 100644
>> --- a/tools/libxl/xl.c
>> +++ b/tools/libxl/xl.c
>> @@ -89,8 +89,17 @@ static void parse_global_config(const char *configfile,
>>      if (!xlu_cfg_get_string (config, "vifscript", &buf, 0))
>>          default_vifscript = strdup(buf);
>>  
>> -    if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0))
>> -	default_bridge = strdup(buf);
>> +    if (!xlu_cfg_get_string (config, "defaultbridge", &buf, 0)) {
>> +        fprintf(stderr, "the global config option defaultbridge is deprecated, "
>> +                        "please switch to vif.default.bridge\n");
>> +        free(default_bridge);
>> +        default_bridge = strdup(buf);
>> +    }
>> +
>> +    if (!xlu_cfg_get_string (config, "vif.default.bridge", &buf, 0)) {
>> +        free(default_bridge);
>> +        default_bridge = strdup(buf);
>> +    }
> 
> Put else if (!xlu.... ("defaultbridge")... ) here instead of above so we
> only warn if the user gave defaultbridge but not vif.default.bridge?

Since "defaultbridge" is now deprecated I think we should warn the user
about it, even if vif.default.bridge is also specified (which also
doesn't make much sense to use both)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel