On Fri, Dec 18, 2009 at 11:44:54AM +0000, Mark McLoughlin wrote:
Hey,
This looks good, except we're encoding a lot more knowledge of how qemu
works than I'd like.
The plan we had to future proof this was that libvirt would assign an id
(using -device foo,id=bar) and 'info pci' would include the device id in
its output.
Once you have that, you no longer need to make assumptions about what
product:vendor is used for each device or what order addresses are
allocated. It's a simple addition to qemu and a simple addition to your
'info pci' parsing code, but I think we'd rest easier wrt. future
compatibility.
That would be preferable, but the trouble is that will only work with
QEMU >= 0.12. Using the 'info pci' output works for all QEMUs. By
'works'
I mean it lets us handle unplug even of devices present at startup. It
obviously doesn't allow for static PCI addresing, since that requires
the new -device arg.
Random thought - how well does this patch work if e.g. you don't
specify
a NIC model? I don't see where we hard-code the knowledge about what
model qemu uses by default ...
The other patch series switching over to -device deals with that by
explicitly setting rtl8139 NIC if no model is provided, so we don't
rely on the unstable QEMU default anymore. Of course there is no
concept of defaults when using -device syntax.
Also, it'd be good to have unit tests for this - inputs would be
a
domain XML lacking addresses, the corresponding 'info pci' output and it
would check addresses against an expected resulting domain XML.
Agreed.
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 :|