From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH V13 5/5] xl: add pvusb commands
Date: Wed, 17 Feb 2016 11:43:02 +0000
Message-ID: <20160217114302.GA3510@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>
	<22211.25102.91003.42436@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <22211.25102.91003.42436@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: jgross@suse.com, wei.liu2@citrix.com, ian.campbell@citrix.com, george.dunlap@eu.citrix.com, Chunyan Liu <cyliu@suse.com>, xen-devel@lists.xen.org, Jim Fehlig <jfehlig@suse.com>, Simon Cao <caobosimon@gmail.com>
List-Id: xen-devel@lists.xenproject.org

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.