[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V8 7/7] domcreate: support pvusb in configuration file
>>> On 11/13/2015 at 12:10 AM, in message <CAFLBxZZ_KN0vMr77+dfRTgGEWW3LSbv8m__AWfvEeCH1WBRrYw@xxxxxxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote: > On Wed, Oct 21, 2015 at 10:08 AM, Chunyan Liu <cyliu@xxxxxxxx> wrote: > > Add code to support pvusb in domain config file. One could specify > > usbctrl and usb in domain's configuration file and create domain, > > then usb controllers will be created and usb device would be attached > > to guest automatically. > > > > One could specify usb controllers and usb devices in config file > > like this: > > usbctrl=['version=2,ports=4', 'version=1, ports=4', ] > > usbdev=['hostbus=2, hostaddr=1, controller=0,port=1', ] > > > > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx> > > Signed-off-by: Simon Cao <caobosimon@xxxxxxxxx> > > > > --- > > changes: > > - change parse_usb_config and parse_usbctrl_config, following > > parse_nic_config way, and move to previous patch > > - update user interface to specify hostbus, hostaddr instead of > > hostbus.hostaddr for future extension > > > > docs/man/xl.cfg.pod.5 | 81 > ++++++++++++++++++++++++++++++++++++++++++++ > > tools/libxl/libxl_create.c | 73 +++++++++++++++++++++++++++++++++++++-- > > tools/libxl/libxl_device.c | 4 +++ > > tools/libxl/libxl_internal.h | 8 +++++ > > tools/libxl/xl_cmdimpl.c | 54 ++++++++++++++++++++++++++++- > > 5 files changed, 216 insertions(+), 4 deletions(-) > > > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > > index b63846a..f24c008 100644 > > --- a/docs/man/xl.cfg.pod.5 > > +++ b/docs/man/xl.cfg.pod.5 > > @@ -722,6 +722,87 @@ Note this may be overridden by rdm_policy option in > > PCI > device configuration. > > > > =back > > > > +=item B<usbctrl=[ "USBCTRL_SPEC_STRING", "USBCTRL_SPEC_STRING", ... ]> > > + > > +Specifies the USB controllers created for this guest. Each > > +B<USB_SPEC_STRING> has the form C<KEY=VALUE,KEY=VALUE,...> where: > > + > > +=over 4 > > + > > +=item B<KEY=VALUE> > > + > > +Possible B<KEY>s are: > > + > > +=over 4 > > + > > +=item B<type=TYPE> > > + > > +Specifies the protocol to implement USB controller, could be "pv" > (indicates > > +PVUSB) or "qemu" (indicates QEMU emulated). Currently only "pv" is > supported. > > I think most of these should basically be copied from the descriptions > of the arguments in patch 5. I'll check. > > > +=item B<version=VERSION> > > + > > +Specifies version of the USB controller, could be 1 (USB1.1) or 2 > (USB2.0). > > +Default is 2 (USB2.0). > > + > > +=item B<ports=PORTS> > > + > > +Specifies port number of the USB controller. Default is 8. > > + > > +Each USB controller will have an index starting from 0. On the same > > +controller, each port will have an index starting from 1. > > I'd say: > > "USB controler ids start from 0. In line with the USB spec, however, > ports on a controller start from 1." Much better. Thanks! > > > + > > +E.g. > > +usbctrl=["version=1,ports=4", "version=2,ports=8",] > > +The first controller has: > > +controller index = 0, and port 1,2,3,4. > > +The second controller has: > > +controller index = 1, and port 1,2,3,4,5,6,7,8. > > The example is good, but I'd use "controller id" rather than "controller > index". Take it. > > > +=back > > + > > +=back > > + > > +=item B<usbdev=[ "USB_SPEC_STRING", "USB_SPEC_STRING", ... ]> > > + > > +Specifies the host USB devices to passthrough to this guest. Each > > +B<USB_SPEC_STRING> has the form C<KEY=VALUE,KEY=VALUE,...> where: > > "Specifiec the USB devices to be attached to the guest at boot." > (i.e., don't assume all devices are hostdev.) Thanks, I'll update. > > > +=over 4 > > + > > +=item B<KEY=VALUE> > > + > > +Possible B<KEY>s are: > > + > > +=over 4 > > + > > +=item B<devtype=hostdev> > > + > > +Specifies USB device type. Currently only support 'hostdev'. > > + > > +=item B<hostbus=busnum> > > + > > +Specifies busnum of the USB device from the host perspective. > > + > > +=item B<hostaddr=devnum> > > + > > +Specifies devnum of the USB device from the host perspective. > > + > > +=item B<controller=CONTROLLER> > > + > > +Specifies USB controller index, to which controller the USB device is > attached. > > + > > +=item B<port=PORT> > > + > > +Specifies USB port index, to which port the USB device is attached. > B<port=PORT> > > +is valid only when B<controller=CONTROLLER> is specified. Without > > +B<controller=CONTROLLER>, it will find the first available USB > controller:port > > +and use it. If there is no controller at all, it will create one. > > I think the last sentence should be a separate paragraph, probably > after the "=back". And a more idiomatic way to write it might be: > > "If no controller is specified, an available controller:port > combination will be used. If there are no available controller:port > options, a new controller will be created." Thanks very much! Much better. > > Only one other minor comment from me: > > > + if (!xlu_cfg_get_list(config, "usbctrl", &usbctrls, 0, 0)) { > > + d_config->num_usbctrls = 0; > > + d_config->usbctrls = NULL; > > + while ((buf = xlu_cfg_get_listitem(usbctrls, > d_config->num_usbctrls)) > > + != NULL) { > > I don't know what the other maintainers think, but particularly given > that this is spanning a line, I would personally take out the > comparison, and just make it > > while ((buf = ...)) { > ... > } > > Other than that, this one looks good to me. > > -George > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |