Re: [Libvir] [PATCH] Enable USB device setting information handling on virsh.

13 Mar 2007

Hi Daniel

I examined your suggestions, and corrected the patch as follows. 
 ・ only (usb 1)
    <usb>
        <bus/>
    </usb>

 ・ (usbdevice mouse)
    <usb>
        <mouse/>
    </usb>

 ・ (usbdevice tablet)
    <usb>
        <tablet/>
    </usb>

 ・ (usbdevice disk:xxx)
    <usb>
        <disk>
          <source dev='xxx'/>
        </disk>
    </usb>

 ・ (usbdevice host:xxx.yyy)
    <usb>
        <host bus='xxx' addr='yyy'/>
    </usb>

 ・ (usbdevice host:xxx:yyy)
    <usb>
        <host vendor='xxx' product='yyy'/>
    </usb>
...
Can we just share parsing or is the fact that a disk hooked on USB bus such
a real difference ?
I think that dividing usual disk and disk of USB is necessary. 
Because USB enables a dynamic detaching of disk though usual disk
cannot do it by domHVM.

Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>

Thanks

--------------------------------------------------------------------------------
Index: src/xend_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.101
diff -u -p -r1.101 xend_internal.c

--- src/xend_internal.c	8 Mar 2007 14:12:06 -0000	1.101
+++ src/xend_internal.c	13 Mar 2007 05:33:33 -0000
@@ -1668,6 +1668,49 @@ xend_parse_sexp_desc(virConnectPtr conn,
         free(tty);
     }
 
+    tmp = sexpr_node(root, "domain/image/hvm/usbdevice");
+    if ((tmp != NULL) && (tmp[0] != 0)) {
+        char *offset;
+        virBufferAdd(&buf, "    <usb>\n", 10 );
+        if (strncmp(tmp, "disk:", 5) == 0) {
+            offset = strtok((char *)tmp, ":");
+            if ((offset = strtok(NULL, ":")) != NULL) {
+                virBufferAdd(&buf, "      <disk>\n", 13 );
+                virBufferVSprintf(&buf, "        <source dev='%s'/>\n", offset);
+                virBufferAdd(&buf, "      </disk>\n", 14 );
+            }
+        } else if (strncmp(tmp, "host:", 5) == 0) {
+            if (strrchr(tmp, ':') > (tmp + 4)) {
+                offset = strtok((char *)tmp, ":");
+                if ((offset = strtok(NULL, ":")) != NULL) {
+                    virBufferVSprintf(&buf, "      <host vendor='%s'", offset);
+                    if ((offset = strtok(NULL, ":")) != NULL) {
+                        virBufferVSprintf(&buf, " product='%s'", offset);
+                    }
+                    virBufferAdd(&buf, "/>\n", 3 );
+                }
+            } else {
+                offset = strtok((char *)tmp, ":.");
+                if ((offset = strtok(NULL, ":.")) != NULL) {
+                    virBufferVSprintf(&buf, "      <host bus='%s'", offset);
+                    if ((offset = strtok(NULL, ":.")) != NULL) {
+                        virBufferVSprintf(&buf, " addr='%s'", offset);
+                    }
+                    virBufferAdd(&buf, "/>\n", 3 );
+                }
+            }
+        } else if (strncmp(tmp, "mouse", 5) == 0) {
+            virBufferAdd(&buf, "      <mouse/>\n", 15 );
+        } else if (strncmp(tmp, "tablet", 6) == 0) {
+            virBufferAdd(&buf, "      <tablet/>\n", 16 );
+        }
+        virBufferAdd(&buf, "    </usb>\n", 11 );
+    } else if (sexpr_int(root, "domain/image/hvm/usb")) {
+        virBufferAdd(&buf, "    <usb>\n", 10 );
+        virBufferAdd(&buf, "      <bus/>\n", 13 );
+        virBufferAdd(&buf, "    </usb>\n", 11 );
+    }
+
     virBufferAdd(&buf, "  </devices>\n", 13);
     virBufferAdd(&buf, "</domain>\n", 10);
 
Index: src/xml.c
===================================================================
RCS file: /data/cvs/libvirt/src/xml.c,v
retrieving revision 1.64
diff -u -p -r1.64 xml.c
--- src/xml.c	8 Mar 2007 14:12:06 -0000	1.64
+++ src/xml.c	13 Mar 2007 05:33:37 -0000
@@ -524,6 +524,66 @@ virDomainParseXMLOSDescHVM(virConnectPtr
     }
     xmlXPathFreeObject(obj);
 
+    obj = xmlXPathEval(BAD_CAST "/domain/devices/usb", ctxt);
+    if ((obj != NULL) && (obj->type == XPATH_NODESET) &&
+        (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) {
+        virBufferAdd(buf, "(usb 1)", 7);
+        xmlXPathFreeObject(obj);
+        obj = xmlXPathEval(BAD_CAST "string(/domain/devices/usb/disk/source/@dev)", ctxt);
+        if ((obj != NULL) && (obj->type == XPATH_STRING) &&
+            (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
+            virBufferVSprintf(buf, "(usbdevice 'disk:%s')", obj->stringval);
+        }
+        if (obj)
+            xmlXPathFreeObject(obj);
+
+        obj = xmlXPathEval(BAD_CAST "string(/domain/devices/usb/host/@vendor)", ctxt);
+        if ((obj != NULL) && (obj->type == XPATH_STRING) &&
+            (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
+            virBufferVSprintf(buf, "(usbdevice 'host:%s", obj->stringval);
+            xmlXPathFreeObject(obj);
+            obj = xmlXPathEval(BAD_CAST "string(/domain/devices/usb/host/@product)", ctxt);
+            if ((obj != NULL) && (obj->type == XPATH_STRING) &&
+            (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
+                virBufferVSprintf(buf, ":%s", obj->stringval);
+            }
+            virBufferAdd(buf, "')", 2);
+        }
+        if (obj)
+            xmlXPathFreeObject(obj);
+
+        obj = xmlXPathEval(BAD_CAST "string(/domain/devices/usb/host/@bus)", ctxt);
+        if ((obj != NULL) && (obj->type == XPATH_STRING) &&
+            (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
+            virBufferVSprintf(buf, "(usbdevice 'host:%s", obj->stringval);
+            xmlXPathFreeObject(obj);
+            obj = xmlXPathEval(BAD_CAST "string(/domain/devices/usb/host/@addr)", ctxt);
+            if ((obj != NULL) && (obj->type == XPATH_STRING) &&
+            (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
+                virBufferVSprintf(buf, ".%s", obj->stringval);
+            }
+            virBufferAdd(buf, "')", 2);
+        }
+        if (obj)
+            xmlXPathFreeObject(obj);
+
+        obj = xmlXPathEval(BAD_CAST "/domain/devices/usb/mouse", ctxt);
+        if ((obj != NULL) && (obj->type == XPATH_NODESET) &&
+            (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) {
+            virBufferAdd(buf, "(usbdevice 'mouse')", 19);
+        }
+        if (obj)
+            xmlXPathFreeObject(obj);
+
+        obj = xmlXPathEval(BAD_CAST "/domain/devices/usb/tablet", ctxt);
+        if ((obj != NULL) && (obj->type == XPATH_NODESET) &&
+            (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) {
+            virBufferAdd(buf, "(usbdevice 'tablet')", 20);
+        }
+    }
+    if (obj)
+        xmlXPathFreeObject(obj);
+
     virBufferAdd(buf, "))", 2);
 
     if (boot_dev)
--------------------------------------------------------------------------------


In message <20070308142704.GB26337@redhat.com>
   "Re: [Libvir] [PATCH] Enable USB device setting information handling on virsh."
   "Daniel Veillard <veillard@redhat.com>" wrote:
...
On Thu, Mar 08, 2007 at 12:59:30PM +0000, Daniel P. Berrange wrote:
...
On Thu, Mar 08, 2007 at 04:52:43PM +0900, Masayuki Sunou wrote:
...
Hi
When domHVM started by virsh create, 
the information of USB setting is not saved by vish dumpxml.
The reason is that USB setting is defined by Xen itself, not virsh.
This patch enables USB device setting information handling 
on virsh create/virsh dumpxml.
I've been wondering about how we'll represent USB devices in the
libvirt XML. The 'usbdevice' attribute in teh XenD SEXPR is just
a straight pass-through to QEMU's -usbdevice command line arg.
This arg can accept values of the form:
- 'mouse'
  - 'tablet'
  - 'disk:file'    eg 'disk:/var/lib/xen/images/usbdrive.img'
  - 'host:bus.addr'  eg 'host:01.02'  for BUS 01, device 02
  - 'host:vendor:product'   eg 'host:0324:01a4'
http://fabrice.bellard.free.fr/qemu/qemu-doc.html#SEC34
The patch you've got just puts all this into a single 'usbdevice'
attribute on a <usb> element. I see there is also a single empty
<usb> tag to identify whether the virtual USB bus is enabled at
all. So we have
<usb/>
    <usb usbdevice='mouse'/>
    <usb usbdevice='tablet/>
    <usb usbdevice='disk:/var/lib/xen/images/usbdrive.img'/>
    <usb usbdevice='host:01.02'/>
    <usb usbdevice='host:0324:01a4'/>
I'm wondering if we'd be better offnormalizing the attributes somewhat.
<usb bus='1'/>
   <usb hid='mouse'/>   (hid is USB speak for Human Input Device)
   <usb hid='tablet'/>
   <usb disk='/var/lib/xen/images/usbdrive.img'/>
   <usb bus='01' addr='02'/>
   <usb vendor='0324' product='01a4'/>
Or alternatively, multiplex off a 'type' attribute
<usb type='bus'/>
   <usb type='mouse'/>
   <usb type='tablet'/>
   <usb type='disk' path='/var/lib/xen/images/usbdrive.img'/>
   <usb type='host' bus='01' addr='02'/>
   <usb type='host' vendor='0324' product='01a4'>
What do people think ?
That I prefer we first discuss a bit more the format before applying such a
patch. I assume the usb description need to go in the <devices> block, right ?
I prefer the last version of the 3, you just check for a first attribute
and if found derive the processing code according to the value. It's IMHO
better than the second structurally speaking, and I really dislike the first
because it keeps the structure outside of XML which requires additional 
parsing and makes things harder.
  Now thinking out loud a bit, would <usb> need to be a structure, physically
it is structured around busses, do we need to reflect that, do we need
to make provision for this. Also in what respect is USB specific, I mean
  <devices>
    <disk path="..." />
  </devices>
and 
  <devices>
    <usb>
      <disk path="..." />
    </usb>
  </devices>
Can we just share parsing or is the fact that a disk hooked on USB bus such
a real difference ?
Daniel
-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard@redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


    