|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V8 5/7] xl: add pvusb commands
>>> On 11/12/2015 at 07:38 PM, in message
<CAFLBxZb3LeZY1V78GdswCBB0B1_069-DbY-tuNVHnQ2h=ywT4A@xxxxxxxxxxxxxx>, George
Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:
> On Wed, Oct 21, 2015 at 10:08 AM, Chunyan Liu <cyliu@xxxxxxxx> wrote:
> > Add pvusb commands: usbctrl-attach, usbctrl-detach, usb-list,
> > usb-attach and usb-detach.
> >
> > To attach a usb device to guest through pvusb, one could follow
> > following example:
> >
> > #xl usbctrl-attach test_vm version=1 ports=8
> >
> > #xl usb-list test_vm
> > will show the usb controllers and port usage under the domain.
> >
> > #xl usbattach test_vm hostbus=1 hostaddr=2
>
> Nit: usb-attach (missing a '-')
Oh, yes.
>
> > will find the first usable controller:port, and attach usb
> > device whose busnum is 1 and devnum is 6.
> > One could also specify which <controller> and which <port>.
> >
> > #xl usb-detach test_vm 0 1
> > will detach USB device under controller 0 port 1.
> >
> > #xl usbctrl-detach test_vm dev_id
> > will destroy the controller with specified dev_id. Dev_id
> > can be traced in usb-list info.
> >
> > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> > Signed-off-by: Simon Cao <caobosimon@xxxxxxxxx>
> >
> > ---
> > Changes:
> > - change usb-attach parameter from hostbus.hostaddr to
> > hostbus=xx hostaddr=
> > - since we get rid of libxl_device_usb_getinfo, so adjust usb-list
> > information a little bit.
> > - parse_usb_config and parse_usbctrl_config following parse_nic_config
> > way, put in this patch, and shared domcreate routine.
> >
> > docs/man/xl.pod.1 | 40 ++++++++
> > tools/libxl/xl.h | 5 +
> > tools/libxl/xl_cmdimpl.c | 250
> ++++++++++++++++++++++++++++++++++++++++++++++
> > tools/libxl/xl_cmdtable.c | 25 +++++
> > 4 files changed, 320 insertions(+)
> >
> > diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> > index d0cd612..f09a872 100644
> > --- a/docs/man/xl.pod.1
> > +++ b/docs/man/xl.pod.1
> > @@ -1345,6 +1345,46 @@ List pass-through pci devices for a domain.
> >
> > =back
> >
> > +=head1 USB PASS-THROUGH
> > +
> > +=over 4
> > +
> > +=item B<usbctrl-attach> I<domain-id> I[<type=val>] [I<version=val>]
> [I<ports=number>]
> > +
> > +Create a new USB controller for the specified domain.
> > +B<type=val> is the usb controller type, currently only support 'pv'.
>
> "B<type=val> is the usb controller type. Currently only 'pv' and
> 'auto' are supported."
>
> Note the period rather than the comma.
>
> (See below on how to make this true.)
>
> > +B<version=val> is the usb controller version, could be 1 (USB1.1) or 2
> (USB2.0).
>
> "<version=val> is the usb controller version. Possible values include
> 1 (USB1.1) and 2 (USB2.0)."
>
> (Same thing wrt the period.)
Thanks. Will check all.
>
> > +B<ports=number> is the total ports of the usb controller.
>
> Is there a maximum number of ports?
Yes, according to Juergen, it should be no more than 31.
>
> > +By default, it will create a USB2.0 controller with 8 ports.
> > +
> > +=item B<usbctrl-detach> I<domain-id> I<devid>
> > +
> > +Destroy a USB controller from the specified domain.
> > +B<devid> is devid of the USB controller.
> > +
> > +If B<-f> is specified, B<xl> is going to forcefully remove the device even
> > +without guest's collaboration.
>
> "If B<-f> is specified, B<xl> will forcefully remove removal (i.e.,
> without the guest's cooperation)."
Thanks.
>
> > +
> > +=item B<usb-attach> I<domain-id> I<hostbus=busnum> I<hostaddr=devnum>
> [I<controller=devid> [I<port=number>]]
> > +
> > +Hot-plug a new pass-through USB device to the specified domain.
> > +B<bus.addr> is the busnum.devnum of the physical USB device to
> > pass-through.
>
> "hostbus and hostaddr are the bus and device number of the physical
> USB device to pass through."
Thanks. Forgot to update here.
>
> > +B<controller=devid> B<port=number> is the USB controller:port to hotplug
> > the
> > +USB device to. By default, it will find the first available
> controller:port
> > +and use it; if there is no controller, it will create one.
> > +
> > +=item B<usb-detach> I<domain-id> I<controller=devid> I<port=number>
> > +
> > +Hot-unplug a previously assigned USB device from a domain.
> > +B<controller=devid> and B<port=number> is USB controller:port in guest
> > where
> the
> > +USB device is attached to.
> > +
> > +=item B<usb-list> I<domain-id>
> > +
> > +List pass-through usb devices for a domain.
> > +
> > +=back
> > +
> > =head1 TMEM
> >
> > =over 4
> > diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> > index 0021112..ddd9690 100644
> > --- a/tools/libxl/xl.h
> > +++ b/tools/libxl/xl.h
> > @@ -85,6 +85,11 @@ int main_blockdetach(int argc, char **argv);
> > int main_vtpmattach(int argc, char **argv);
> > int main_vtpmlist(int argc, char **argv);
> > int main_vtpmdetach(int argc, char **argv);
> > +int main_usbctrl_attach(int argc, char **argv);
> > +int main_usbctrl_detach(int argc, char **argv);
> > +int main_usbattach(int argc, char **argv);
> > +int main_usbdetach(int argc, char **argv);
> > +int main_usblist(int argc, char **argv);
> > int main_uptime(int argc, char **argv);
> > int main_claims(int argc, char **argv);
> > int main_tmem_list(int argc, char **argv);
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 365798b..e6ff6f4 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -1255,6 +1255,64 @@ static void parse_vnuma_config(const XLU_Config
> *config,
> > free(vcpu_parsed);
> > }
> >
> > +/* Parses usbctrl data and adds info into usbctrl
> > + * Returns 1 if the input token does not match one of the keys
> > + * or parsed values are not correct. Successful parse returns 0 */
> > +static int parse_usbctrl_config(libxl_device_usbctrl *usbctrl, char
> *token)
> > +{
> > + char *oparg;
> > +
> > + if (MATCH_OPTION("type", token, oparg)) {
> > + if (!strcmp("pv", oparg)) {
> > + usbctrl->type = LIBXL_USBCTRL_TYPE_PV;
> > + } else {
> > + fprintf(stderr,
> > + "Unsupported USB controller type '%s'\n", oparg);
> > + return 1;
> > + }
>
> One of the reasons for making the enum types in the IDL is that the
> IDL then automatically generates functions for converting to/from
> strings.
>
> So here I would do something like:
>
> if(libxl_usbctrl_type_from_string(optarg, &usbctrl->type)) {
> fprintf(stderr, "Invalid usbctrl type: %s\n", optarg);
> exit(1);
> }
>
> (Just to emphasize -- you don't need to write the above function; it's
> generated automatically.)
Yes, I know. I'll adjust codes.
>
>
> > +/* Parses usb data and adds info into usb
> > + * Returns 1 if the input token does not match one of the keys
> > + * or parsed values are not correct. Successful parse returns 0 */
> > +static int parse_usb_config(libxl_device_usb *usb, char *token)
> > +{
> > + char *oparg;
> > +
> > + if (MATCH_OPTION("devtype", token, oparg)) {
> > + if (!strcmp("hostdev", oparg)) {
> > + usb->devtype = LIBXL_USBDEV_TYPE_HOSTDEV;
>
> Same thing here, with libxl_usbdev_type_to_string().
Got it.
>
>
> > +int main_usbctrl_attach(int argc, char **argv)
> > +{
> > + uint32_t domid;
> > + int opt, rc = 0;
> > + libxl_device_usbctrl usbctrl;
> > +
> > + SWITCH_FOREACH_OPT(opt, "", NULL, "usbctrl-attach", 1) {
> > + /* No options */
> > + }
> > +
> > + domid = find_domain(argv[optind++]);
> > +
> > + libxl_device_usbctrl_init(&usbctrl);
> > +
> > + for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
> > + if (parse_usbctrl_config(&usbctrl, *argv))
> > + return 1;
> > + }
> > +
> > + rc = libxl_device_usbctrl_add(ctx, domid, &usbctrl, 0);
> > + if (rc) {
> > + fprintf(stderr, "libxl_device_usbctrl_add failed.\n");
> > + rc = 1;
> > + }
> > +
> > + libxl_device_usbctrl_dispose(&usbctrl);
> > + return rc;
> > +}
> > +
> > +int main_usbctrl_detach(int argc, char **argv)
> > +{
> > + uint32_t domid;
> > + int opt, devid, rc;
> > + libxl_device_usbctrl usbctrl;
> > +
> > + SWITCH_FOREACH_OPT(opt, "", NULL, "usbctrl-detach", 2) {
> > + /* No options */
> > + }
> > +
> > + domid = find_domain(argv[optind]);
> > + devid = atoi(argv[optind+1]);
> > +
> > + libxl_device_usbctrl_init(&usbctrl);
> > + if (libxl_devid_to_device_usbctrl(ctx, domid, devid, &usbctrl)) {
> > + fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
> > + return 1;
> > + }
> > +
> > + rc = libxl_device_usbctrl_remove(ctx, domid, &usbctrl, 0);
> > + if (rc) {
> > + fprintf(stderr, "libxl_device_usbctrl_remove failed.\n");
> > + rc = 1;
> > + }
> > +
> > + libxl_device_usbctrl_dispose(&usbctrl);
> > + return rc;
> > +
> > +}
> > +
> > +int main_usbattach(int argc, char **argv)
> > +{
> > + uint32_t domid;
> > + int opt, rc;
> > + libxl_device_usb usb;
> > +
> > + SWITCH_FOREACH_OPT(opt, "", NULL, "usb-attach", 2) {
> > + /* No options */
> > + }
> > +
> > + libxl_device_usb_init(&usb);
> > +
> > + domid = find_domain(argv[optind++]);
> > +
> > + for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
> > + if (parse_usb_config(&usb, *argv))
> > + return 1;
> > + }
> > +
> > + rc = libxl_device_usb_add(ctx, domid, &usb, 0);
> > + if (rc) {
> > + fprintf(stderr, "libxl_device_usb_add failed.\n");
> > + rc = 1;
> > + }
> > +
> > + libxl_device_usb_dispose(&usb);
> > + return rc;
> > +}
> > +
> > +int main_usbdetach(int argc, char **argv)
> > +{
> > + uint32_t domid;
> > + int ctrl, port;
> > + int opt, rc = 1;
> > + libxl_device_usb usb;
> > +
> > + SWITCH_FOREACH_OPT(opt, "", NULL, "usb-detach", 3) {
> > + /* No options */
> > + }
> > +
> > + domid = find_domain(argv[optind]);
> > + ctrl = atoi(argv[optind+1]);
> > + port = atoi(argv[optind+2]);
> > +
> > + if (argc - optind > 3) {
> > + fprintf(stderr, "Invalid arguments.\n");
> > + return 1;
> > + }
> > +
> > + libxl_device_usb_init(&usb);
> > + if (libxl_ctrlport_to_device_usb(ctx, domid, ctrl, port, &usb)) {
> > + fprintf(stderr, "Unknown device at controller %d port %d.\n",
> > + ctrl, port);
> > + return 1;
> > + }
> > +
> > + rc = libxl_device_usb_remove(ctx, domid, &usb, 0);
> > + if (rc) {
> > + fprintf(stderr, "libxl_device_usb_remove failed.\n");
> > + rc = 1;
> > + }
> > +
> > + libxl_device_usb_dispose(&usb);
> > + return rc;
> > +}
> > +
> > +int main_usblist(int argc, char **argv)
> > +{
> > + uint32_t domid;
> > + libxl_device_usbctrl *usbctrls;
> > + libxl_usbctrlinfo usbctrlinfo;
> > + int numctrl, i, j, opt;
> > +
> > + SWITCH_FOREACH_OPT(opt, "", NULL, "usb-list", 1) {
> > + /* No options */
> > + }
> > +
> > + domid = find_domain(argv[optind++]);
> > +
> > + if (argc > optind) {
> > + fprintf(stderr, "Invalid arguments.\n");
> > + exit(-1);
> > + }
> > +
> > + usbctrls = libxl_device_usbctrl_list(ctx, domid, &numctrl);
> > + if (!usbctrls) {
> > + return 0;
> > + }
> > +
> > + for (i = 0; i < numctrl; ++i) {
> > + printf("%-6s %-6s %-3s %-5s %-7s %-5s %-30s\n",
> > + "Devid", "Type", "BE", "state", "usb-ver", "ports",
> "BE-path");
> > +
> > + libxl_usbctrlinfo_init(&usbctrlinfo);
> > +
> > + if (!libxl_device_usbctrl_getinfo(ctx, domid,
> > + &usbctrls[i], &usbctrlinfo)) {
> > + printf("%-6d %-6s %-3d %-5d %-7d %-5d %-30s\n",
> > + usbctrlinfo.devid,
> > + libxl_usbctrl_type_to_string(usbctrlinfo.type),
>
> Oh, I see you already know about the "type_to_string()" functions. :-)
>
> > + usbctrlinfo.backend_id, usbctrlinfo.state,
> > + usbctrlinfo.version, usbctrlinfo.ports,
> > + usbctrlinfo.backend);
> > +
> > + for (j = 1; j <= usbctrlinfo.ports; j++) {
> > + libxl_device_usb usb;
> > + libxl_usbinfo usbinfo;
> > +
> > + libxl_device_usb_init(&usb);
> > + libxl_usbinfo_init(&usbinfo);
> > +
> > + printf(" Port %d:", j);
> > +
> > + if (!libxl_ctrlport_to_device_usb(ctx, domid,
> > + usbctrlinfo.devid, j,
> &usb)) {
> > + printf(" Bus %03x Device %03x\n",
> > + usbinfo.busnum, usbinfo.devnum);
> > + } else {
> > + printf("\n");
> > + }
> > +
> > + libxl_usbinfo_dispose(&usbinfo);
>
> Er, what's going on with the usbinfo? Did you take the usbinfo stuff
> out of libxl_pvusb.c, but forget to take it out of libxl_types.idl and
> here?
>
> I guess you really want usb.u.hostdev.* here.
Yes, you're totally right. I'll adjust codes.
Thanks.
- Chunyan
>
> Everything else looks good, thanks.
>
> -George
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |