Re: [libvirt] [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
Am 03.07.2012 00:31, schrieb Eric Blake:
On 07/02/2012 04:02 PM, Corey Bryant wrote:
> Here's another option that Kevin and I discussed today on IRC. I've
> modified a few minor details since the discussion. And Kevin please
> correct me if anything is wrong.
>
> Proposal Four: Pass a set of fds via 'pass-fds'. The group of fds
> should all refer to the same file, but may have different access flags
> (ie. O_RDWR, O_RDONLY). qemu_open can then dup the fd that has the
> matching access mode flags.
But this means that libvirt has to open a file O_RDWR up front for any
file that it _might_ need qemu to reopen later, and that qemu is now
hanging on to 2 fds per fdset instead of 1 fd for the life of any client
of the fdset.
It doesn't have to be up front. There's no reason not to have monitor
commands that add or remove fds from a given fdset later.
I see no reason why libvirt can't pass in an O_RDWR fd when qemu
only
needs to use an O_RDONLY fd; making qemu insist on an O_RDONLY fd makes
life harder for libvirt. On the other hand, I can see from a safety
standpoint that passing in an O_RDWR fd opens up more potential for
errors than passing in an O_RDONLY fd,
Yes, this is exactly my consideration. Having a read-only file opened as
O_RDWR gives us the chance to make stupid mistakes.
but if you don't know up front
whether you will ever need to write into a file, then it would be better
to pass in O_RDONLY. At least I don't see it as a security risk:
passing in O_RDWR but setting SELinux permissions on the fd to only
allow read() but not write() via the labeling of the fd may be possible,
so that libvirt could still prevent accidental writes into an O_RDWR
file that starts life only needing to service reads.
But this would assume that libvirt knows exactly when a reopen happens,
for each current and future qemu version. I wouldn't want to tie qemu's
internals so closely to the management application, even if libvirt
could probably get it reasonably right (allow rw before sending a
monitor command; revoke rw when receiving a QMP event that the commit
has completed). It wouldn't work if we had a qemu-initiated ro->rw
transition, but I don't think we have one at the moment.
> pass-fds:
> { 'command': 'pass-fds',
> 'data': {'fdsetname': 'str', '*close':
'bool'},
> 'returns': 'string' }
This still doesn't solve Dan's complaint that things are not atomic; if
the monitor dies between the pass-fds and the later use of the fdset, we
would need a counterpart monitor command to query what fdsets are
currently known by qemu.
If you want a query-fdsets, that should be easy enough.
Actually, we might be able to even make the command transactionable.
This would of course require blockdev-add to be transactionable as well
to be of any use.
And like you pointed out, you didn't make it
clear how a timeout mechanism would be implemented to auto-close fds
that are not consumed in a fixed amount of time - would that be another
optional parameter to 'pass-fds'?
Do you think a timeout is helpful? Can't we just drop libvirt's
reference automatically if the monitor connection gets closed?
The BH/timer thing Corey mentioned is more about the qemu internal
problem that during a reopen there may be a short period where the old
fd is closed, but the new one not yet opened, so the fdset might need to
survive a short period with refcount 0.
Or do we need a way to initially create a set with only one O_RDONLY
fd,
but later pass in a new O_RDWR fd but associate it with the existing set
rather than creating a new set (akin to the 'force' option of 'pass-fd'
in proposal two)?
As I said above, yes, I think this makes a lot sense.
Kevin