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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list