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

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



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/
> 


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