From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH RFC V2 1/5] libxl: add pvusb definitions Date: Tue, 3 Mar 2015 16:45:08 +0000 Message-ID: <54F5E514.2010802@eu.citrix.com> References: <1421656131-19366-1-git-send-email-cyliu@suse.com> <1421656131-19366-2-git-send-email-cyliu@suse.com> <1425381019.24959.87.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1425381019.24959.87.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 , Chunyan Liu Cc: ian.jackson@citrix.com, lars.kurth@citrix.com, caobosimon@gmail.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 03/03/2015 11:10 AM, Ian Campbell wrote: > On Mon, 2015-01-19 at 16:28 +0800, Chunyan Liu wrote: > > Sorry for the long delay in replying. > >> To attach a usb device, a virtual usb controller should be created first. >> This patch defines usbctrl and usbdevice related structs. > > Per <54CA17DF0200006600095E3D@relay2.provo.novell.com> please could you > mention here that the HVM guest related parts (i.e. > LIBXL_USBCTRL_TYPE_DEVICEMODEL) and libxl_usb_type are placeholders for > emulated HVM support. > > In fact I wonder if it should just be omitted, we will need a LIBXL_HAVE > for HVM USB support anyway once it is implemented so we can add the enum > then. > > Or will we -- do we think returning an error for such an HVM guest with > USB devices configured now is acceptable and for it to silently start > working at some point in the future is OK? I think that was my idea back when I was writing the HVM hotplug patches. > >> >> Signed-off-by: Chunyan Liu >> Signed-off-by: Simon Cao >> --- >> tools/libxl/libxl_types.idl | 58 +++++++++++++++++++++++++++++++++++- >> tools/libxl/libxl_types_internal.idl | 1 + >> 2 files changed, 58 insertions(+), 1 deletion(-) >> >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> index 1214d2e..0639434 100644 >> --- a/tools/libxl/libxl_types.idl >> +++ b/tools/libxl/libxl_types.idl >> @@ -521,6 +531,27 @@ libxl_device_pci = Struct("device_pci", [ >> ("seize", bool), >> ]) >> >> +libxl_device_usbctrl = Struct("device_usbctrl", [ >> + ("name", string), >> + ("type", libxl_usbctrl_type), >> + ("backend_domid", libxl_domid), >> + ("backend_domname", string), >> + ("devid", libxl_devid), >> + ("usb_version", uint8), >> + ("num_ports", uint8), > > I think int would be fine for both of these last two (and is a bit > kinder to language bindings). > >> + ]) >> + >> +libxl_device_usb = Struct("device_usb", [ >> + ("ctrl", integer), > > Is this an index into something? If so what? > > There seems to be no usbctrl array added to the domain_config struct, so > I'm unsure how this is used. It looks like this is meant to specify which USB controller you're plugging your device into; and if you set it to -1, then it will automatically try to plug it into the best one (and also automatically create one for you if one doesn't exist). This should probably be a libxl_devid I'm guessing, then? -George