On 07/03/2012 10:25 AM, Corey Bryant wrote:
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.
Correct - since we will be adding a remove-fd, then that command needs to know both the fdset name and the individual fd within the set to be removed.
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?
Passing the flag parameters is not trivial, as that would mean the QMP code would have to define constants mapping to all of the O_* flags that qemu_open supports. It's easier to support closing by fd number.
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.
pass-fd (or add-fd, whatever name we give it) adds an fd to an fdset, with initial use count of 1 (the use is the monitor). qemu_open() increments the use count. A new qemu_close() wrapper would decrement the use count. And both calling 'remove-fd', or closing the QMP monitor of an fd that has not yet been passed through 'remove-fd', serves as a way to decrement the use count. You'd still have to track whether the monitor is using an fd (to avoid over-decrementing on QMP monitor close), but by having the monitor's use also tracked under the refcount, then refcount reaching 0 is sufficient to auto-close an fd. I think that also means that re-establishing the client QMP connection would increment For some examples: 1. client calls 'add-fd', qemu is now tracking fd=4 with refcount 1, in use by monitor, as member of fdset1 2. client crashes, so all tracked fds are visited; fd=4 had not yet been passed to 'remove-fd', so qemu decrements refcount; refcount of fd=4 is now 0 so qemu closes it 1. client calls 'add-fd', qemu is now tracking fd=4 with refcount 1, in use by monitor, as member of fdset1 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, so qemu_open() increments the refcount to 2 3. client crashes, so all tracked fds are visited; fd=4 had not yet been passed to 'remove-fd', so qemu decrements refcount to 1, but leaves fd=4 open because it is still in use by the block device 4. client re-establishes QMP connection, and 'query-fds' lets client learn about fd=4 still being open as part of fdset1, but also informs client that fd is not in use by the monitor 1. client calls 'add-fd', qemu is now tracking fd=4 with refcount 1, in use by monitor, as member of fdset1 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, so qemu_open() increments the refcount to 2 3. client calls 'remove-fd fdset=1 fd=4', so qemu marks fd=4 as no longer in use by the monitor, refcount decremented to 1 but still left open because it is in use by the block device 4. client crashes, so all tracked fds are visited; but fd=4 is already marked as not in use by the monitor, so its refcount is unchanged 1. client calls 'add-fd', qemu is now tracking fd=4 with refcount 1, in use by monitor, as member of fdset1 2. client calls 'device-add' with /dev/fdset/1 as the backing filename, but the command fails for some other reason, so the refcount is still 1 at the end of the command (although it may have been temporarily incremented then decremented during the command) 3. client calls 'remove-fd fdset=1 fd=4' to deal with the failure (or QMP connection is closed), so qemu marks fd=4 as no longer in use by the monitor, refcount is now decremented to 0 and fd=4 is closed I think that covers the idea; you need a bool in_use for tracking monitor state (the monitor is in use until either a remove-fd or a monitor connection closes), as well as a ref-count.
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.
Yes, I think a single query command is good enough here, something like: { "execute":"query-fdsets" } => { "return" : { "sets": [ { "name": "fdset1", "fds": [ { "fd": 4, "monitor": true, "refcount": 1 } ] }, { "name": "fdset2", "fds": [ { "fd": 5, "monitor": false, "refcount": 1 }, { "fd": 6, "monitor": true, "refcount": 2 } ] } ] } }
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.
I think the refcount being non-zero _is_ what defines an fd as being in use by qemu (including use by the monitor). Any place you have to close an fd before reopening it is dangerous; the safe way is always to open with the new permissions before closing the old permissions. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org