Re: [Libvir] [PATCH] Enable USB device setting information handling on virsh.
On Tue, Mar 13, 2007 at 05:50:38PM +0900, Masayuki Sunou wrote:
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>
okay that looks fine to me
> 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.
okay, fair enough.
+ offset = strtok((char *)tmp, ":");
Sorry you can't use strtok there, it's not reentrant, uses a global
variable and as a result multiple concurent threads accessing libvirt for
different connections may get their data randomly corrupted, this is a
big problem. I prefer parsing to be done explicitely computing indexes
from the string being parsed (and then you can use strndup() to generate
copies if needed).
+ if ((offset = strtok(NULL, ":")) != NULL) {
+ virBufferAdd(&buf, " <disk>\n", 13 );
+ virBufferVSprintf(&buf, " <source
dev='%s'/>\n", offset);
+ virBufferAdd(&buf, " </disk>\n", 14 );
+ }
plus offset would not been freed here, so that would be a memory leak too,
+ } 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
the patch for xml.c looks good to me, but I would rather apply both at the
same time. Could you regenerate a patch doing the same but avoiding strtok
(I'm not too fond of strtok_r() either).
thanks in advance !
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/