[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 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. > +=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." > + > +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". > +=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.) > +=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." 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |