[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Add support for QEMU file descriptor sets



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]