On 07/03/2012 11:59 AM, Kevin Wolf wrote:
Am 03.07.2012 17:40, schrieb Corey Bryant:
Thanks again for taking time to discuss this at today's QEMU community call.
Here's the proposal we discussed at the call. Please let me know if I
missed anything or if there are any issues with this design.
Proposal Five: New monitor commands enable adding/removing an fd
to/from a set. The set 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.
PRO: Supports reopen
PRO: All other commands work without impact by using qemu_open()
PRO: No fd leakage (fds are associated with monitor connection and, if
not in use, closed when monitor disconnects)
PRO: Security-wise this is ok since libvirt can manage the set of fd's
(ie. it can add/remove an O_RDWR fd to/from the set as needed).
CON: Not atomic (e.g. doesn't add an fd with single drive_add command).
USAGE:
1. add-fd /dev/fdset/1 FDSET={M} -> qemu adds fd to set named
"/dev/fdset/1" - command returns qemu fd (e.g fd=4) to caller. libvirt
in-use flag turned on for fd.
I thought qemu would rather return the number of the fdset (which it
also assigns if none it passed, i.e. for fdset creation). Does libvirt
need the number of an individual fd?
If libvirt prefers to assign fdset numbers itself, I'm not against it,
it's just something that wasn't clear to me yet.
That's fine. QEMU can return the fdset number or a string
(/dev/fdset/1) if none is specified. And an fdset will need to be
specified if adding to an existing set.
I think libvirt will need the fd returned by add-fd so that it can
evaluate fds returned by query-fd. It's also useful for remove-fd.
2. drive_add file=/dev/fdset/1 -> qemu_open uses the first fd from the
set that has access flags matching the qemu_open action flags.
qemu_open increments refcount for this fd.
3. add-fd /dev/fdset/1 FDSET={M} -> qemu adds fd to set named
"/dev/fdset/1" - command returns qemu fd to caller (e.g fd=5). libvirt
in-use flag turned on for fd.
3. block-commit -> qemu_open reopens "/dev/fdset/1" by using the first
fd from the set that has access flags matching the qemu_open action
flags. qemu_open increments refcount for this fd.
4. remove-fd /dev/fdset/1 5 -> caller requests fd==5 be removed from the
set. turns libvirt in-use flag off marking the fd ready to be closed
when qemu is done with it.
If we decided to not return the individual fd numbers to libvirt, file
descriptors would be uniquely identified by an fdset/flags pair here.
Are you saying we'd pass the fdset name and flags parameters on
remove-fd to somehow identify the fds to remove?
5. qemu_close decrements refcount for fd, and closes fd when refcount is
zero and libvirt in use flag is off.
The monitor could just hold another reference, then we save the
additional flag. But that's a qemu implementation detail.
I'm not sure I understand what you mean.
More functional details:
-If libvirt crashes it can call "query-fd /dev/fdset/1" to determine
which fds are open in the set.
We also need a query-fdsets command that lists all fdsets that exist. If
we add information about single fds to the return value of it, we
probably don't need a separate query-fd that operates on a single fdset.
Yes, good point. And maybe we don't need 2 commands. query-fdsets
could return all the sets and all the fds that are in those sets.
-If monitor connection closes, qemu will close fds that have a refcount
of zero. Do we also need a qemu in-use flag in case refcount is zero
and fd is still in use?
In use by whom? If it's still in use in qemu (as in "in-use flag would
be set") and we have a refcount of zero, then that's a bug.
In use by qemu. I don't think it's a bug. I think there are situations
where refcount gets to zero but qemu is still using the fd.
-This support requires introduction of qemu_close function that will be
called everywhere in block layer that close is currently called.
Notes:
-Patch series 1 will include support for all of the above. This will be
my initial focus.
-Patch series 2 will include command line support that enables
association of command line fd with a monitor set. This will be my
secondary focus, most likely after patch series 1 is applied.
Thanks, this is a good and as far as I can tell complete summary of what
we discussed.
Kevin
Definitely! Thank you for all the input.
--
Regards,
Corey
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list