On Sun, Aug 03, 2008 at 01:41:28AM +0200, Guido G?nther wrote:
Hi,
attached is a second version. Changes are:
* s/bus/subsystem/
* support hexadecimal and decimal attributes
* introduce device and source elements.
I decided to not drop vendor and product id into their own elements
since the structure would then become very nested for no good reason.
The reason is that I want it to match the host device enumeration
XML format. This will be using explicit <product> and <vendor>
tags, with an 'id' attribute, because there will be text content
in the body giving the human readable name.
Some examples:
<hostdev mode="subsys" type='usb'>
<source>
<device vendor="0x0204" product="0x6025"/>
So this needs to be changed to
<vendor id="0x0204"/>
<product id="0x6025"/>
</source>
</hostdev>
<hostdev mode="subsys" type='usb'>
<source>
<address bus="001" device="003"/>
</source>
</hostdev>
This one is fine.
Support for <target> will be coming once I got around to adjust
qemu/kvm.
Ok, that's not critical, but the other things we need before we
can commit this are
- Code to format XML for output, so that the devices are included when
dumping the XML description of a guest.
- Update to the RNG schema in docs/libvirt.rng
- Example XML data files in tests/qemuxml2argvdata/, along with the
corresponding CLI args for QEMU. THis is then hooked into the
qemuxml2argvtest.c and qemuxml2xmltest.c files
The latter test suite requires point the XML formatting & RNG updates.
The currrent code you have for parsing XML & attaching device/creating
CLI flags is basically correct with only very minor bugs
+static int
+virDomainHostdevSubsysUsbDefParseXML(virConnectPtr conn,
+ const xmlNodePtr node,
+ virDomainHostdevDefPtr def) {
Minor indentation bug.
+
+ int ret = -1;
+ xmlNodePtr cur;
+
+ def->source.subsys.usb.vendor = 0;
+ def->source.subsys.usb.product = 0;
+ def->source.subsys.usb.bus = 0;
+ def->source.subsys.usb.device = 0;
This isn't needed, since the VIR_ALLOC contract says that the memory
range has been memset() to all zero.
+static int qemudDomainAttachHostDevice(virDomainPtr dom,
virDomainDeviceDefPtr dev)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
+ int ret;
+ char *cmd, *reply;
+
+ if (dev->data.hostdev->source.subsys.usb.vendor) {
+ ret = asprintf(&cmd, "usb_add host:%.4x:%.4x",
+ dev->data.hostdev->source.subsys.usb.vendor,
+ dev->data.hostdev->source.subsys.usb.product);
+ } else {
+ ret = asprintf(&cmd, "usb_add host:%.3d.%.3d",
+ dev->data.hostdev->source.subsys.usb.bus,
+ dev->data.hostdev->source.subsys.usb.device);
+ }
+ if (ret == -1) {
+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ "%s", _("out of memory"));
+ return -1;
This should be reporting OOM, and not pass the 'dom' parameter, and
no need for a string message, eg
qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|