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

From: Wei Liu <wei.liu2@citrix.com>
To: Jim Fehlig <jfehlig@suse.com>
Cc: jgross@suse.com, wei.liu2@citrix.com, ian.campbell@citrix.com,
	george.dunlap@eu.citrix.com,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Chunyan Liu <cyliu@suse.com>,
	xen-devel@lists.xen.org, Simon Cao <caobosimon@gmail.com>
Subject: Re: [PATCH V13 5/5] xl: add pvusb commands
Date: Thu, 21 Jan 2016 12:12:23 +0000	[thread overview]
Message-ID: <20160121121223.GU1691@citrix.com> (raw)
In-Reply-To: <569EFA8F.1000301@suse.com>

On Tue, Jan 19, 2016 at 08:10:07PM -0700, Jim Fehlig wrote:
> On 01/19/2016 11:11 AM, Ian Jackson wrote:
> > Chunyan Liu writes ("[PATCH V13 5/5] xl: add pvusb commands"):
> >> Add pvusb commands: usbctrl-attach, usbctrl-detach, usb-list,
> >> usbdev-attach and usbdev-detach.
> > Thanks for swapping this with the other patch.  It is better now.
> >
> >> +=item B<usbctrl-attach> I<domain-id> I<usbctrl-device>
> > However, I think you need to explictly state that the user may (and
> > indeed, must) pass multiple settings as separate arguments.  AFAICT
> > the parser here doesn't do the ,-splitting.
> 
> 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
> 
> 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. 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?
> 

It is splitting on " " at the moment which is not very desirable.

I think this is a bug. I've added it to my list and will look into
fixing it.

Wei.

> Regards,
> Jim
> 