[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd

From: Corey Bryant
Subject: Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
Date: Tue, 03 Jul 2012 09:42:56 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1

On 07/02/2012 06:31 PM, Eric Blake wrote:
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.

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, 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.

     { '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.  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'?

Yes see the description of the close bool on pass-fds below which determines how the fds are closed.

It's not an atomic operation, but it does handle Dan's concern of preventing fd leaks.

If libvirt did crash after the pass-fd, then couldn't it just abandon the fd set (which will get closed) and libvirt could send a new fd set and work with that?

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)?

Yeah I see what you're saying.

I was also wondering if it would make sense for qemu to consume the passed fds and use those (ie. no dup()). This would prevent the issues of having to close the fds that linger on the monitor. But I don't know if it's realistic for libvirt to know how many open calls qemu will need to make (this would need 1 fd passed per open call).

     { 'command': 'close-fds',
       'data': {'fdsetname': 'str' }
      @fdsetname - the file descriptor set name
      @close - *if true QEMU closes the monitor fds when they expire.
               if false, fds remain on the monitor until close-fds
PRO: Supports reopen
PRO: All other commands work without impact by using qemu_open()
PRO: No fd leakage if close==true specified
CON: Determining when to close fds when close==true may be tricky
1. pass-fd FDSET={M} -> returns a string "/dev/fdset/1") that refers to
the passed set of fds.
2. drive_add file=/dev/fdset/1 -- qemu_open() uses the first fd from the
list that has access flags matching the qemu_open() action flags.
3. block-commit -- qemu_open() reopens "/dev/fdset/1" by using the first
fd from the list that has access flags matching the qemu_open() action
4. The monitor fds are closed:
    A) *If @close == true, qemu closes the set of fds when the timer
    B) If @close == false, libvirt must issue close-fds command to close
       the set of fds

*How to solve this has yet to be determined.  Kevin mentioned
potentially using a bottom-half or a timer in qemu_close().

Still, it is certainly an option worth thinking about, and I'm hoping we
can come to a solid design that everyone agrees provides the desired
security and flexibility without having to recode every single existing

I agree.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]