Re: [PATCH V8 3/7] libxl: add pvusb API

From: Olaf Hering <olaf@aepfle.de>
To: Chunyan Liu <cyliu@suse.com>
Cc: jgross@suse.com, wei.liu2@citrix.com, ian.campbell@citrix.com,
	george.dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com,
	xen-devel@lists.xen.org, jfehlig@suse.com,
	Simon Cao <caobosimon@gmail.com>
Subject: Re: [PATCH V8 3/7] libxl: add pvusb API
Date: Thu, 12 Nov 2015 12:32:00 +0100	[thread overview]
Message-ID: <20151112113200.GA302@aepfle.de> (raw)
In-Reply-To: <1445418510-19614-4-git-send-email-cyliu@suse.com>

On Wed, Oct 21, Chunyan Liu wrote:

> Add pvusb APIs, including:

Some comments below.

After a quick look I miss the proposed ctrl/device separation for pvscsi
(what handles "state" changes?). But, I have to read all the other
dozen+ threads about that topic first.

> +    flexarray_append_pair(back, "state", "1");

4.6+ has macros for "state" values, like
flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateInitialising));

> +    flexarray_append_pair(front, "state", "1");

4.6+ has macros for "state" values.

> +        LOG(DEBUG, "Adding new usb device to xenstore");

Which one? Perhaps print also details.

> +    LOG(DEBUG, "Removing USB device from xenstore");

Which one? Perhaps print also details.

> +        /* check if the USB interface is already bound to "usbbcak" */

Typo.

Olaf