[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 contribute some corrected patches. 

1. strtok() --> strtok_r()
   --> Add tok_buf for strtok_r()
2. Correct the first argument of strtok_r()
3. Add a version of XML 
   --> So that the format of XML may change in the future 

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

Thanks

-------------------------------------------------------------------------------
Index: libvirt/include/libvirt/libvirt.h
===================================================================
RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h,v
retrieving revision 1.40
diff -u -p -r1.40 libvirt.h
--- libvirt/include/libvirt/libvirt.h	8 Mar 2007 08:31:07 -0000	1.40
+++ libvirt/include/libvirt/libvirt.h	15 Mar 2007 06:09:16 -0000
@@ -221,6 +221,12 @@ int			virGetVersion		(unsigned long *lib
 						 unsigned long *typeVer);
 
 /*
+ * Macro providing the version of the XML
+ */
+#define XML_VERSION_NUMBER 1
+
+
+/*
  * Connection and disconnections to the Hypervisor
  */
 int			virInitialize		(void);
Index: libvirt/src/xend_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.102
diff -u -p -r1.102 xend_internal.c
--- libvirt/src/xend_internal.c	14 Mar 2007 13:14:51 -0000	1.102
+++ libvirt/src/xend_internal.c	15 Mar 2007 06:09:23 -0000
@@ -1344,7 +1344,7 @@ xend_parse_sexp_desc(virConnectPtr conn,
         domid = sexpr_int(root, "domain/domid");
     else
         domid = -1;
-    virBufferVSprintf(&buf, "<domain type='xen' id='%d'>\n", domid);
+    virBufferVSprintf(&buf, "<domain type='xen' id='%d' version='%d'>\n", domid, XML_VERSION_NUMBER);
 
     tmp = sexpr_node(root, "domain/name");
     if (tmp == NULL) {
@@ -1668,6 +1668,57 @@ xend_parse_sexp_desc(virConnectPtr conn,
         free(tty);
     }
 
+    tmp = sexpr_node(root, "domain/image/hvm/usbdevice");
+    if ((tmp != NULL) && (tmp[0] != 0)) {
+        char *offset;
+        char *tmp_buf;
+        char *tok_buf;
+        virBufferAdd(&buf, "    <usb>\n", 10 );
+        if (strncmp(tmp, "disk:", 5) == 0) {
+            tmp_buf = strdup(tmp);
+            offset = strtok_r(tmp_buf, ":", &tok_buf);
+            if ((offset = strtok_r(NULL, ":", &tok_buf)) != NULL) {
+                virBufferAdd(&buf, "      <disk>\n", 13 );
+                virBufferVSprintf(&buf, "        <source dev='%s'/>\n", offset);
+                virBufferAdd(&buf, "      </disk>\n", 14 );
+            }
+            free(tmp_buf);
+        } else if (strncmp(tmp, "host:", 5) == 0) {
+            if (strrchr(tmp, ':') > (tmp + 4)) {
+                tmp_buf = strdup(tmp);
+                offset = strtok_r(tmp_buf, ":", &tok_buf);
+                if ((offset = strtok_r(NULL, ":", &tok_buf)) != NULL) {
+                    virBufferVSprintf(&buf, "      <host vendor='%s'", offset);
+                    if ((offset = strtok_r(NULL, ":", &tok_buf)) != NULL) {
+                        virBufferVSprintf(&buf, " product='%s'", offset);
+                    }
+                    virBufferAdd(&buf, "/>\n", 3 );
+                }
+                free(tmp_buf);
+            } else {
+                tmp_buf = strdup(tmp);
+                offset = strtok_r(tmp_buf, ":.", &tok_buf);
+                if ((offset = strtok_r(NULL, ":.", &tok_buf)) != NULL) {
+                    virBufferVSprintf(&buf, "      <host bus='%s'", offset);
+                    if ((offset = strtok_r(NULL, ":.", &tok_buf)) != NULL) {
+                        virBufferVSprintf(&buf, " addr='%s'", offset);
+                    }
+                    virBufferAdd(&buf, "/>\n", 3 );
+                }
+                free(tmp_buf);
+            }
+        } 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: libvirt/src/xml.c
===================================================================
RCS file: /data/cvs/libvirt/src/xml.c,v
retrieving revision 1.64
diff -u -p -r1.64 xml.c
--- libvirt/src/xml.c	8 Mar 2007 14:12:06 -0000	1.64
+++ libvirt/src/xml.c	15 Mar 2007 06:09:27 -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 <20070314155441 GI3127 redhat com>
   "Re: [Libvir] [PATCH] Enable USB device setting information handling on virsh."
   "Daniel Veillard <veillard redhat com>" wrote:

> On Wed, Mar 14, 2007 at 03:38:02AM +0000, Daniel P. Berrange wrote:
> > I'm not so sure about this to be honest. This is very verbose syntax and having
> > the empty <usb> wrapper around every element isn't adding all that much useful
> > info in relation to its verbosity.
> 
>   I based my review on the structure, this was looking flexible enough to
> not raise big problem, but I didn't tried to make a global analysis, thanks
> for doing this !
> 
> > I've been thinking about how the contents of the <devices> section relates to
> > the way Linux models devices in sysfs. Thus far we have 'interface', 'disk'
> > 'console' and 'graphics'. 
> > 
> >   - The 'disk' element maps onto /sys/block or /sys/class/scsi_disk
> >   - The 'interface' element maps onto /sys/class/net
> >   - The 'graphics' element maps onto /sys/class/graphics
> >   - The 'console' element maps onto /sys/class/misc
> > 
> > I tend to think of /sys/block as really being a class too (ie /sys/class/block).
> > 
> > With the way we've been thinking about USB in this thread so far though, we're
> > approaching a model that is much more like /sys/bus rather than /sys/class. ie
> > we're grouping everything under a bus type at the top level, rather than the
> > rest of the nodes which are grouped by class (function).
> > 
> > Listing again at what things we need to represent here:
> > 
> >   - Input devices - mouse, tablet
> >   - Disk devices
> >   - Presence of USB or not (aka whether a bus is emulated)
> >   - Pass through of devices from the host bus
> > 
> > Now consider a couple of non-USB related things we don't have representations
> > for yet
> > 
> >   - VMWare mouse type
> >   - Summa graphics tablet - a serial device tablet
> >   - PS/2  mouse - implicitly represented if we don't ask for any other mouse
> >   - PCI device pass through from host
> >   - Serial ports
> >   - Parallel ports
> >   - Sound cards
> > 
> > Basically we end up with two sorts of things we need to represent:
> > 
> >   - Emulation of particular classes of devices grouped by their function
> >   - Pass through of real hardware, irrespective of function
> > 
> > Based on this observation I think we need to actually treat the Xen USB configs in
> > two different ways according to the above split. 
> 
>   yes that look nicer in the long term
> 
> > So USB disks might look like
> > 
> >    <disk type='file' device='disk'>
> >      <source file='/some/path.img'/>
> >      <target dev='usb'/>
> >    </disk>
> > 
> > NB, I'm using a generic 'usb' tag as the guest device name, since USB devices are
> > fundamentally hotplug oriented it is very unsual to have persistent USB bus IDs
> > and thus you'll never have a persistent device node. Maybe its better to have a
> > separate named attribute instead of re-using the 'dev' attribute in <target>:
> > 
> >    <disk type='file' device='disk'>
> >      <source file='/some/path.img'/>
> >      <target bus='usb'/>
> >    </disk>
> > 
> > Or a separate bus type ?
> > 
> >    <disk type='file' device='disk'>
> >      <source file='/some/path.img'/>
> >      <bus type='usb'/>
> >    </disk>
> 
>   I'm not sure this really adds much over <target dev='usb'/>, we can still
> add extra attributes if needed onto target in the usb case.
> 
> > Anyway, now onto input devices. It would seem like a reasonable idea to have a
> > top level '<input>' device element.
> 
>   Yes, but when you mean top level you really mean as child of <devices>, I
> see why an input would be any different, right ?
> 
> > So a generic USB mouse could be
> > 
> >     <input type='usbmouse'/>
> > 
> > While a tablet would be
> > 
> >     <input type='usbtablet'/>
> > 
> > Alternatively we could be explicit about the particular type of device / named
> > model being emulated, eg for the USB tablet:
> > 
> >     <input type='evtouch'/>
> > 
> > The downside with the latter is that its probably different device name for
> > every different virt system. If we just kept a generic 'usbmouse' it would
> > represent 'whatever particular usb mouse the virt system supports'.
> > 
> > My final thought is that we could have 'typoe' be compulsory representing the
> > type of pointer. And then 'bus' and 'model' could be optional atttributes to
> > give more specific info if needed, eg for tablets these are equivalent:
> > 
> >     <input type='tablet'/>
> >     <input bus='serial' type='tablet'/>
> >     <input bus='serial' type='tablet' model='summagraphics'/>
> >     <input type='tablet' model='summagraphics'/>
> > 
> > And these are equivalent:
> > 
> >     <input type='tablet' model='evtouch'/>
> >     <input bus='usb' type='tablet'/>
> >     <input bus='usb' type='tablet' model='evtouch'/>
> 
>   looks good to me
> 
> > When creating a domain, the caller could leave out 'bus' or 'model' if they
> > don't care about specific details. When asking libvirt for an XML dump of
> > a running domain though, we'd fill in the most specific info we have.
> > 
> > Finally, for the mapping of host devices through to the guest. In this case we
> > probably do just want to group stuff under either a bus specific node, eg
> > 
> >    <usb>
> >      <host vendor='0234' model='2342'/>
> >      <host vendor='1248' model='4228'/>
> >      <host bus='03' device='02'/>
> >    </usb>
> > 
> >    <pci>
> >      <host id='0000:00:1f.1'/>
> >    </pci>
> > 
> > Or a generic bus node
> > 
> >     <bus type='usb'>
> >        <host vendor='0234' model='2342'/>
> >        <host vendor='1248' model='4228'/>
> >        <host bus='03' device='02'/>
> >     </bus>
> > 
> >     <bus type='pci'>
> >        <host id='0000:00:1f.1'/>
> >     </bus>
> 
>    I prefer the later, it's a bit more structured.
> 
> > This email was *significantly* longer than I thought it would be. I guess
> > that's good because it shows there's some non-trivial decisions we should
> > make before committing to a particular style of implementation. I want to
> > ask do a little research of the way USB / PCI devices are expressed with
> > VMWare config files, and with the XenAPI  XML-RPC format in case there's
> > any useful stuff we can learn from the way they model devices.
> 
>   yes thanks for getting deeper on this !
> 
> 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]