On Fri, Jan 30, 2015 at 05:17:56PM -0700, Eric Blake wrote:
On 11/20/2014 08:08 AM, Stefan Berger wrote:
Wow, I've been horribly slow at reviewing this. Do feel free to ping on list if no one seems to notice a patch, to widen the chances of anyone taking a glance at it.
Pass the TPM file descriptor to QEMU via command line. Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional parameters -add-fd set=10,fd=20.
This addresses the use case when QEMU is started with non-root privileges and QEMU cannot open /dev/tpm0 for example.
One problem is that for the passing of the file descriptor set to work, virCommandReorderFDs must not be called on the virCommand. This is prevented by setting a flag in the virCommandPassFDGetFDIndex that is checked to be clear when virCommandReorderFDs is run.
I'm still trying to figure out how virCommandReorderFDs() got into the picture (I didn't write that section of the code); when I originally worked on virCommand, the only way to pass fds to the child was in direct positions (same fd in child as in parent), not by remapping one fd in the parent to another position in the child, except for the three standard descriptors. It looks like virCommandReorderFDs was added to allow remapping other fds and populates the LISTEN_FDS environment variable with how many fds were thus mapped. So the two approaches don't really mix. Do we ever use virCommandPassListenFDs() on a virCommand that will also do raw fd passthrough? Maybe the real thing to do is to track at virCommand build-up time that only one of the two passthrough methods can be used, and fail loudly if a programmer tries to do both methods at once.
Or instead of virCommandPassFD() which only takes a source FD number, we could have added a virComandPassFDRemap which takes a source and target FD number. That way we don't have a global "reorder FDs" concept that can break in unexpected ways - we would only ever re-order FDs for which an explicitly target FD was requested.
Or maybe we should ALWAYS prepare to remap fds in the child, so that the child receives fds in contiguous order with no gaps. It might simplify the code base to always reorder things, and have the mapping done up front. That is, change the virCommandPassFD() function to return an integer of which next consecutive fd the child will see, or -1 on failure. Callers that then need to alter the command line of the child will have to pay attention to the return value (something a bit different than most virCommand build-up, which intentionally defer error checking to the very end), but it might be worth it.
This might be simplest way to go - I'm just wondering if it will cause us any other type of fun problems Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|