[libvirt] Add support for QEMU file descriptor sets

[Stefan Berger]

On 01/25/2013 06:35 PM, Eric Blake wrote:
> On 01/22/2013 12:07 PM, Stefan Berger wrote:
>> On 01/22/2013 01:29 PM, Daniel P. Berrange wrote:
>>> On Tue, Jan 22, 2013 at 01:09:50PM -0500, Stefan Berger wrote:
>>> Thanks for starting on this; it is something I have also been 
>>> planning to work on.

Glad to be able to help. When doing this my main concern was getting my 
stack of TPM patches for libvirt going again to be able to test, so the 
approach may have been a lot more 'naive' than your goal of supporting 
the new functionality as a replacement for all existing file 
descriptors. Considering that TPM would appear after -add-fd appeared, 
things are a bit easier...

>
> First, we need more than just moving it into qemu_command.c; we also

I did this in the meantime.

> need a qemu_capabilities change to add a capability to know when the
> feature is available.  Or even two: 'add-fd' monitor command was added

A new capability for the command line support should be fairly easy.

> in 1.2, but -add-fd command line option wasn't until 1.3.  Then again,
> maybe it's just simpler to state that if you are targeting qemu 1.2,
> there is nothing worth using the new add-fd QMP command for that cannot
> already be done with older fallbacks, and it is only 1.3 and later where
> the new fd passing becomes essential.
>
> Then, based on the capability being present or absent, any code in
> qemu_command that wants to pass an fd to qemu needs to call into a
> helper routine that will either use the new -add-fd argument, fall back
> to older ad-hoc processing, or error out if there is no fallback for
> that particular usage.

Let's take this command line parameter here as an example.

tap,fd=21,id=hostnet1,vhost=on,vhostfd=23

We could write a function where we pass "fd"  and the '21' as parameters 
and  it creates "fd=21", which we then append. We could do this as one 
patch with all of them, so also with "vhostfd" and '23'. Not many 
changes here, just one more VIR_FREE() call needed for each file 
descriptor string.

In a subsequent patch we now test inside that function whether the new 
command line parameter is available using the capability (from another 
patch) and create fd=/dev/set/10 and vhostfd=/dev/set/20 respectively 
*and* already add "-add-fd set=10,fd=21" and "-add-fd set=20,fd=23" to 
virCommand using the functions I already posted (as the lowest layer).

Would it be that simple, or do we need to add more parameters to -add-fd 
set=10,fd=21,xyz under certain circumstances?

>
> Then we also need to use the QMP commands to update the fdsets after
> qemu is started; depending on what the fd is used for, we can close the
> fd once qemu has dup'd it into internal use, so that qemu doesn't hang
> on to the fd that libvirt passed in forever even when it's internal use
> is hot-unplugged.

Hm, isn't that already solved by the virCommandTransferFD command plus 
subsequent closing of the fd post forking? I thought the -add-fd way of 
passing fd's was a straight forward replacement for example for the 
command line above, but maybe there's more to it... I quickly checked 
the qemu_command.c sources and it looks like all fd's used (searched by 
'fd=') are virCommandTransferFD'ed except for 'ioeventfd=%s' which looks 
like it's Type-to-string conversion.

>
> We also need to figure out how to use the QMP commands during hotplug
> operations, instead of the command line additions at qemu startup.
>
> And there's still the nagging issue that even as of qemu 1.4, there is
> still no way to specify block device backing chains via fds; so even if
> we can pass in an fd for some use cases (like TPM), we are still waiting
> for qemu to give us a way to do it in all use cases (like NFS security
> for qcow2 backing chains across snapshots, block copy, and block
> commit).  But one thing at a time.
>
> How much of this work are you planning on attempting, and how much do
> you need me to do?

Initially my plan was to only support TPM (which doesn't hotplug for 
example) ... :-/

    Stefan