From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH V13 5/5] xl: add pvusb commands Date: Thu, 21 Jan 2016 12:29:03 +0000 Message-ID: <20160121122903.GV1691@citrix.com> References: <1453192795-15693-1-git-send-email-cyliu@suse.com> <1453192795-15693-6-git-send-email-cyliu@suse.com> <22174.31841.372743.506457@mariner.uk.xensource.com> <569EFA8F.1000301@suse.com> <20160121121223.GU1691@citrix.com> <1453378871.4320.28.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <1453378871.4320.28.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: jgross@suse.com, Wei Liu , george.dunlap@eu.citrix.com, Ian Jackson , Chunyan Liu , xen-devel@lists.xen.org, Jim Fehlig , Simon Cao List-Id: xen-devel@lists.xenproject.org On Thu, Jan 21, 2016 at 12:21:11PM +0000, Ian Campbell wrote: > On Thu, 2016-01-21 at 12:12 +0000, Wei Liu wrote: > > 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.=A0=A0It is better no= w. > > > > = > > > > > +=3Ditem B I I > > > > However, I think you need to explictly state that the user may (and > > > > indeed, must) pass multiple settings as separate arguments.=A0=A0AF= AICT > > > > 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=3D00:16:3e:xx:yy:zz,bridge=3Dbr0 > > > 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=3D00: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. > = > If you are feeling particularly highly motivated then xl_cmdimpl.c contai= ns > quite a lot of adhoc parsing of various comma a space separated lists > (often dupped for a given instance into cmdline vs cfg file, which leads = to > unintentional behavioural differences like above) which could usefully be > consolidated into a series of helpers. > = Or rewrite every adhoc parser with bison/flex or Ocaml / Haskell parsec. Jokes aside. I will see what I can do. Consolidating them into helper functions is a good start. Wei. > Ian. > =