[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V7 3/7] libxl: add pvusb API





On 10/13/2015 09:15 PM, George Dunlap wrote:
On 13/10/15 02:46, Chun Yan Liu wrote:

On 10/12/2015 09:46 PM, George Dunlap wrote:
On 12/10/15 08:19, Chun Yan Liu wrote:
+
+    usbinfo->devnum = usb->u.hostdev.hostaddr;
+    usbinfo->busnum = usb->u.hostdev.hostbus;
+
+    busid = usb_busaddr_to_busid(gc, usb->u.hostdev.hostbus,
+                                 usb->u.hostdev.hostaddr);
+    if (!busid) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    filename = GCSPRINTF(SYSFS_USB_DEV"/%s/idVendor", busid);
+    if (!libxl_read_sysfs_file_contents(ctx, filename, &buf, NULL))
+        sscanf(buf, "%" SCNx16, &usbinfo->idVendor);
+
+    filename = GCSPRINTF(SYSFS_USB_DEV"/%s/idProduct", busid);
+    if (!libxl_read_sysfs_file_contents(ctx, filename, &buf, NULL))
+        sscanf(buf, "%" SCNx16, &usbinfo->idProduct);
+
+    filename = GCSPRINTF(SYSFS_USB_DEV"/%s/manufacturer", busid);
+    if (!libxl_read_sysfs_file_contents(ctx, filename, &buf,
&buflen) &&
+        buflen > 0) {
+        /* replace \n to \0 */
+        if (((char *)buf)[buflen - 1] == '\n')
+            ((char *)buf)[buflen - 1] = '\0';
+        usbinfo->manuf = libxl__strdup(NOGC, buf);
+   }
+
+    filename = GCSPRINTF(SYSFS_USB_DEV"/%s/product", busid);
+    if (!libxl_read_sysfs_file_contents(ctx, filename, &buf,
&buflen) &&
+        buflen > 0) {
+        /* replace \n to \0 */
+        if (((char *)buf)[buflen - 1] == '\n')
+            ((char *)buf)[buflen - 1] = '\0';
+        usbinfo->prod = libxl__strdup(NOGC, buf);
+    }
   Basically, starting here, we have information which won't be
available
if we're using a pvusb driver domain.
   This information is nice-to-have, but I don't think this
information is
directly relevant to libxl or xl; the funcitonality to get this
information is available from other libraries like libusb.  I'm
inclined
to say that if we want to have pvusb driver domains (and I think we
do),
we should just get rid of this information.
For command 'xl usb-list', those information should be reported to user.
Do you mean we could call libusb to get thoes information directly in
xl toolstack and get rid of this function?

I think we can keep the function, since every other device type has the
function XXX_getinfo. But here we could check backend_domid, for
backend=dom0, doing above work; for backend!=dom0 (e.g. pvusb
driver domain, no matter how, it should also be able to let users
getting
those information. Can add code  in future.)
Does xl need that information?  Can't the user get that information from
lsusb?

In any case, I can see why it would be *useful* to have in xl.  But
about having it in libxl, I think this question sort of goes along with
the question about the next patch -- whether libxl should be in the
business of providing general information about the USB devices it's
handling, or whether it should limit itself to doing what is absolutely
necessary to talk to usbback.

There's a part of me that sees the point of it: it's not actually that
much extra code (at least for Linux), and it makes it easy to add some
very useful features to xl.

On the other hand, it's not portable to other OSes.  At the moment of
course pvusb isn't portable either, but once we have qemu USB (providing
either emulated or PV usb) then I *think* most of the other
functionality will Just Work on any platform that can compile qemu
(incl. FreeBSD, NetBSD, &c), won't it?  The code you're introducing here
would have to be re-implented for those platforms, and for every new
platform that wanted to include this functionality, wouldn't it?
So, about the portability problem, I think it's back to: do need to
update code to call libusb instead of reading sysfs now? Except
for this function, still have places reading sysfs to get hostbus,
hostaddr, vendorId, deviceId. Those are not portable for other
platform.
I realize I didn't give you very clear guidance; I guess I was hoping to
get an opinion from the tools maintainers.  Or perhaps, I was hoping to
let them be the "bad guys" and say, "You can't have this feature in
libxl", so I wouldn't have to. :-)

In the absence of guidance to the contrary, I suggest that patch series
should focus on getting the core pvusb functionality in, without the
extra usb-querying bits.  Then we can discuss a further series which
either adds the usb querying functionality to libxl, or implement it in
xl using libusb.
OK. Got it. Thanks.

-Chunyan


  -George



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.