[virt-tools-list] [virt-manager PATCH 0/3] Add support for TPM passthrough

[Cole Robinson]

On 06/24/2013 05:58 PM, Stefan Berger wrote:
> From: root <root at k-d941f-5.watson.ibm.com>
> 

Hmm, might want to set .gitconfig on that machine :)

> Add support for the QEMU TPM passthrough device that is supported by libvirt:
> 
> http://libvirt.org/formatdomain.html#elementsTpm
> 
> I added the choice of the backend device even though only one backend
> type (passthrough) is supported right now, but we are working on getting
> another TPM backend into QEMU.
> 
> Since only one device model (tpm-tis) is supported and we do not currently
> have plans to implemented for example a virtio model, I did not provide
> the choice for the device model.
> 

Thanks for the patches Stefan! Code itself looks fine. A few recommendations.

- Split patch 1 into two patches.

** First patch is just the TPM device file. Please also add a unit test for
this, easiest is to edit tests/xmlparse.py, follow the example of
testAlterRedirdev. See HACKING for info on running the test suite.

** Second patch adds the command line handling. Please also add a CLI unit
test for this: edit tests/clitest.py, look for add_category("smartcard" and do
something similar, though you probably only need to add 2-3 tests max. Also
stick a --tpm example onto the many-devices test. Finally add a man page
section: man/virt-install.pod, follow the example of --smartcard or
--redirdev, it should be very simple.

- Reorganize the virt-manager patches by combining the UI and logical changes
in the same patch, but have the first patch be the details page bits, second
patch is the add hardware bits.

- In add hardware, stick all the fields in a table, this will give consistent
spacing between labels and interactable elements. Check the addhw watchdog
page as an example.

- On the first virt-manager patch, stick a couple tpm examples in the
test-many-devices guest in tests/testdriver.xml. This makes it quite easy to
test details UI without needing an actual guest. The test driver is also
useful testing the addhardware bits. See HACKING for more details.

- I'm getting some weird GTK warnings when running virt-manager --debug, this
might be a side effect of editing the glade files by hand. For the
virt-manager patches, I'd recommend resaving the ui files with glade to make
sure they are properly formatted.

None of that stuff should be too tedious so don't let the length of the
suggestions scare you :)

Thanks,
Cole