On 01/26/2013 08:44 PM, Stefan Berger wrote:
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?
Ok, so likely this was a bad example and fd= does not need to be
replaced with the fd sets. There are five occurrences of path= in the
code, which could be handled similarly as described above with an
additional open() and virCommandTransferFD(). My guess is that you want
to eliminate the syscall open() usage within QEMU. Is that right ?
Regards,
Stefan