[libvirt] [PATCH] PCI passthrough docs, tests and minor change
The following add the documentation for the PCI passthrough, extends the RNG and add an extra test for the QEmu parsing to args and serialization back to XML. I also noticed that we parse (and save) and extra PCI domain argument, but it's not actually used when calling qemu, so I assume it's a missing feature for QEmu and just decided to patch the code to not save the extra value when not defined (i.e. 0). Whether it should just be removed at this point is IMHO a separate decision, which is why I took the way with minimal changes. Note that the domain attribute is not documented nor part or the RNG either at this point. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, 2009-01-12 at 11:30 +0100, Daniel Veillard wrote:
+ For PCI devices the element carries 3 attributes allowing to designate + the device as can be found with the <code>lspci</code> command, the + <code>bus</code> attribute allows the hexadecimal values 0 to ff, the + <code>slot</code> attribute allows the hexadecimal values 0 to 1f, and + the <code>function</code> attribute allows the hexadecimal values 0 to + 7.</dd></dl>
Looks good, but probably want to recommend "virsh nodedev-list" rather than lspci? Cheers, Mark.
On Mon, Jan 12, 2009 at 11:17:52AM +0000, Mark McLoughlin wrote:
On Mon, 2009-01-12 at 11:30 +0100, Daniel Veillard wrote:
+ For PCI devices the element carries 3 attributes allowing to designate + the device as can be found with the <code>lspci</code> command, the + <code>bus</code> attribute allows the hexadecimal values 0 to ff, the + <code>slot</code> attribute allows the hexadecimal values 0 to 1f, and + the <code>function</code> attribute allows the hexadecimal values 0 to + 7.</dd></dl>
Looks good, but probably want to recommend "virsh nodedev-list" rather than lspci?
Definitely - this was the primary motivation for adding the node device APIs 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 :|
On Mon, Jan 12, 2009 at 11:23:51AM +0000, Daniel P. Berrange wrote:
On Mon, Jan 12, 2009 at 11:17:52AM +0000, Mark McLoughlin wrote:
On Mon, 2009-01-12 at 11:30 +0100, Daniel Veillard wrote:
+ For PCI devices the element carries 3 attributes allowing to designate + the device as can be found with the <code>lspci</code> command, the + <code>bus</code> attribute allows the hexadecimal values 0 to ff, the + <code>slot</code> attribute allows the hexadecimal values 0 to 1f, and + the <code>function</code> attribute allows the hexadecimal values 0 to + 7.</dd></dl>
Looks good, but probably want to recommend "virsh nodedev-list" rather than lspci?
Definitely - this was the primary motivation for adding the node device APIs
I agree that it allows to work with remote URIs, but ... except it doesn't seems to indicate what devices are actually there. IMHO the current PCI pasthrough limitation is that it doesn't work by name or by capability of the device, so you have to go fishing to get what you look for, lspci allows that. Unless you go though all the nodedev-list results and run virsh nodedev-dumpxml on each. Is there a way to dump in one command all informations about the PCI devices on the host. Oh and a way to list the capabilities available, to avoid guessing. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Jan 12, 2009 at 11:17:52AM +0000, Mark McLoughlin wrote:
On Mon, 2009-01-12 at 11:30 +0100, Daniel Veillard wrote:
+ For PCI devices the element carries 3 attributes allowing to designate + the device as can be found with the <code>lspci</code> command, the + <code>bus</code> attribute allows the hexadecimal values 0 to ff, the + <code>slot</code> attribute allows the hexadecimal values 0 to 1f, and + the <code>function</code> attribute allows the hexadecimal values 0 to + 7.</dd></dl>
Looks good, but probably want to recommend "virsh nodedev-list" rather than lspci?
Okay I commited the docs with a reference to both, as I think this covers slight different users. I also documented the domain optional argument and removed the code to avoid saving it, thanks for the fast review :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Jan 12, 2009 at 11:30:51AM +0100, Daniel Veillard wrote:
The following add the documentation for the PCI passthrough, extends the RNG and add an extra test for the QEmu parsing to args and serialization back to XML. I also noticed that we parse (and save) and extra PCI domain argument, but it's not actually used when calling qemu, so I assume it's a missing feature for QEmu and just decided to patch the code to not save the extra value when not defined (i.e. 0).
Index: src/domain_conf.c =================================================================== RCS file: /data/cvs/libxen/src/domain_conf.c,v retrieving revision 1.52 diff -u -r1.52 domain_conf.c --- src/domain_conf.c 8 Jan 2009 13:54:20 -0000 1.52 +++ src/domain_conf.c 12 Jan 2009 10:23:03 -0000 @@ -3137,11 +3137,20 @@ } } if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - virBufferVSprintf(buf, " <address domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", - def->source.subsys.u.pci.domain, - def->source.subsys.u.pci.bus, - def->source.subsys.u.pci.slot, - def->source.subsys.u.pci.function); + if (def->source.subsys.u.pci.domain != 0) { + virBufferVSprintf(buf, + " <address domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", + def->source.subsys.u.pci.domain, + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function); + } else { + virBufferVSprintf(buf, + " <address bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", + def->source.subsys.u.pci.bus, + def->source.subsys.u.pci.slot, + def->source.subsys.u.pci.function); + } }
virBufferAddLit(buf, " </source>\n");
NACK to this chunk - we should always output the 'domain' attribute even when it is zero - it should only be optional when parsing the XML. The fact that QEMU doesn't use it is just an impl artifact of QEMU. 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 :|
On Mon, Jan 12, 2009 at 11:46:38AM +0000, Daniel P. Berrange wrote:
I also noticed that we parse (and save) and extra PCI domain argument, but it's not actually used when calling qemu, so I assume it's a missing feature for QEmu and just decided to patch the code to not save the extra value when not defined (i.e. 0). NACK to this chunk - we should always output the 'domain' attribute even when it is zero - it should only be optional when parsing the XML. The fact that QEMU doesn't use it is just an impl artifact of QEMU.
Then it need to be documented too, no way around since it will always show up in dumps. And the fact it's not actually used must be documented too. IMHO this just increase the long maintainance for something you don't have any garantee to ever use. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Jan 12, 2009 at 12:55:14PM +0100, Daniel Veillard wrote:
On Mon, Jan 12, 2009 at 11:46:38AM +0000, Daniel P. Berrange wrote:
I also noticed that we parse (and save) and extra PCI domain argument, but it's not actually used when calling qemu, so I assume it's a missing feature for QEmu and just decided to patch the code to not save the extra value when not defined (i.e. 0). NACK to this chunk - we should always output the 'domain' attribute even when it is zero - it should only be optional when parsing the XML. The fact that QEMU doesn't use it is just an impl artifact of QEMU.
Then it need to be documented too, no way around since it will always show up in dumps. And the fact it's not actually used must be documented too. IMHO this just increase the long maintainance for something you don't have any garantee to ever use.
IMHO it is a bug that QEMU just hardcodes domain ID as 0000 because this is assuming a machine only ever has one PCI domain, which is just a bogus assumption. 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 :|
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Mark McLoughlin