Re: [PATCH V13 5/5] xl: add pvusb commands

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Feb 16, 2016 at 05:53:18PM +0000, Ian Jackson wrote:
> Jim Fehlig writes ("Re: [Xen-devel] [PATCH V13 5/5] xl: add pvusb commands"):
> > I just noticed this is the case with network devices as well. E.g.
> > 
> > #xl network-attach hvm-domU mac=00:16:3e:xx:yy:zz,bridge=br0
> > libxl: error: libxl_device.c:1095:device_hotplug_child_death_cb: script: Could
> > not find bridge device xenbr0
> 
> This is clearly a bug.
> 
> > main_networkattach() in tools/libxl/xl_cmdimpl.c doesn't split on
> > the ',', so everything after mac=00:16:3e:xx:yy:zz is ignored.
> 
> That's quite silly, isn't it.  Looking at the code it would also accept
>   mac=00:16:3e:aa:bb:cc:THIS:SHOULD:NOT:BE:HERE
> 
> The bug seems to be that the ad-hoc strtoul-based mac address parser
> in xl_cmdimpl.c:parse_nic_config doesn't notice garbage after its
> option.
> 
> > I'd need advice on how to fix this though. Based on
> > xl-network-configuration doc and Xen tool's long history of
> > network-attach supporting that syntax, I'd say main_networkattach()
> > should be changed to split on ','. I could also change the docs. Do
> > tools maintainers have a preference, or alternative option?
> 
> I don't have a strong opinion, but I would lean towards insisting that
> on command lines each setting should be in its own argument.
> 

We should support both IMHO. Libxlu's disk spec parser is able to do
that with parse_disk_config_multistring.

Wei.

> Ian.