Re: [Xen-devel] [PATCH V12 5/5] domcreate: support pvusb in configuration file [and 1 more messages]

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Chunyan Liu writes ("[PATCH V12 4/5] xl: add pvusb commands"):
> Add pvusb commands: usbctrl-attach, usbctrl-detach, usb-list,
> usbdev-attach and usbdev-detach.

The way you have documented usbctrl-attach and the config file syntax
in this patch and the next one means that each usb config setting
appears twice in the documentation.

Can you please follow the example of disk devices ?  (Except that you
probably don't want to refer to a separate file, but to a separate
part of the manpage.)

This will be easier if you swap the order of these two patches,
because otherwise one patch has to refer to docs which are introduced
in a later patch.

> +        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),
> +                    usbctrlinfo.backend_id, usbctrlinfo.state,
> +                    usbctrlinfo.version, usbctrlinfo.ports,
> +                    usbctrlinfo.backend);

I think the backend xenstore path should not normally be printed out
by xl.  That some of the other xl subcommands do that for some devices
is a mistake, I'm afraid.

Chunyan Liu writes ("[PATCH V12 5/5] domcreate: support pvusb in configuration 
file"):
> 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.
...
> +    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 have to say that I am very uncomfortable with the level of code
duplication here, but the existing code is very bad for this so I
don't feel I can ask you to refactor it.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel