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

On 07/09/2012 11:46 AM, Kevin Wolf wrote: > Am 09.07.2012 17:05, schrieb Corey Bryant: >> I'm not sure this is an issue with current design. I know things have >> changed a bit as the email threads evolved, so I'll paste the current >> design that I am working from. Please let me know if you still see any >> issues. >> >> FD passing: >> ----------- >> New monitor commands enable adding/removing an fd to/from a set. New >> monitor command query-fdsets enables querying of current monitor fdsets. >> The set of fds should all refer to the same file, with each fd having >> different access flags (ie. O_RDWR, O_RDONLY). qemu_open can then dup >> the fd that has the matching access mode flags. >> >> Design points: >> -------------- >> 1. add-fd >> -> fd is passed via SCM rights and qemu adds fd to first unused fdset >> (e.g. /dev/fdset/1) >> -> add-fd monitor function initializes the monitor inuse flag for the >> fdset to true >> -> add-fd monitor function initializes the remove flag for the fd to false >> -> add-fd returns fdset number and received fd number (e.g fd=3) to caller >> >> 2. drive_add file=/dev/fdset/1 >> -> qemu_open uses the first fd in fdset1 that has access flags matching >> the qemu_open action flags and has remove flag set to false >> -> qemu_open increments refcount for the fdset >> -> Need to make sure that if a command like 'device-add' fails that >> refcount is not incremented >> >> 3. add-fd fdset=1 >> -> fd is passed via SCM rights >> -> add-fd monitor function adds the received fd to the specified fdset >> (or fails if fdset doesn't exist) >> -> add-fd monitor function initializes the remove flag for the fd to false >> -> add-fd returns fdset number and received fd number (e.g fd=4) to caller >> >> 4. block-commit >> -> qemu_open performs "reopen" by using the first fd from the fdset that >> has access flags matching the qemu_open action flags and has remove flag >> set to false >> -> qemu_open increments refcount for the fdset >> -> Need to make sure that if a command like 'block-commit' fails that >> refcount is not incremented >> >> 5. remove-fd fdset=1 fd=4 >> -> remove-fd monitor function fails if fdset doesn't exist >> -> remove-fd monitor function turns on remove flag for fd=4 > > What was again the reason why we keep removed fds in the fdset at all? Because if refcount is > 0 for the fd set, then the fd could be in use by a block device. So we keep it around until refcount is decremented to zero, at which point it is safe to close. > > The removed flag would make sense for a fdset after a hypothetical > close-fdset call because the fdset needs to be kept around until the > last user closes it, but I think removed fds can be deleted immediately. fds in an fd set really need to be kept around until zero block devices reference them. At that point, if '(refcount == 0 && (!inuse || remove))' is true, then we'll officially close the fd.

> I think I might have confused remove-fd and close-fdset in earlier > emails in this thread, so I hope this isn't inconsistent with what I > said before. > Ok no problem. >> 6. qemu_close (need to replace all close calls in block layer with >> qemu_close) >> -> qemu_close decrements refcount for fdset >> -> qemu_close closes all fds that have (refcount == 0 && (!inuse || remove)) >> -> qemu_close frees the fdset if no fds remain in it >> >> 7. disconnecting the QMP monitor >> -> monitor disconnect visits all fdsets on monitor and turns off monitor >> in-use flag for fdset > > And close all fds with refcount == 0. > Yes, this makes sense. It also makes sense to close removed fds with refcount == 0 in the remove-fd function. Basically this will be the same thing we do in qemu_close. We'll close any fds that evaulate the following as true: (refcount == 0 && (!inuse || remove))