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

>> 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 > > Doesn't the refcount belong to an fdset rather than a single fd? As long > as the fdset has still a reference, it's possible to reach the other fds > in the set through a reopen. For example: > > 1. client calls 'add-fd', qemu is now tracking fd=4 (O_RDWR) as a member > of fdset1, in use by monitor, refcount 1 > 2. client calls 'add-fd', qemu is now tracing fd=5 (O_RDONLY) as a > member of fdset, in use by monitor, refcount is still 1 > 3. client calls 'device-add' with /dev/fdset/1 as the backing filename > and r/w, so qemu_open() dup()s fd 4 and increments the refcount to 2 > 4. client crashes, so all tracked fdsets are visited; fdset1 gets its > refcount decreased to 1, and both member fds 4 and 5 stay open. > > If you had refcounted a single fd, fd 5 would be closed now and qemu > couldn't switch to O_RDONLY any more. > If the O_RDONLY is for a future command, it would make more sense to add that fd before that future command. Or are you passing it at step 2 because it is needed for the probe re-open issue?

>>>> 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. > > There is one case I'm aware of where we need to be careful: Before > opening an image, qemu may probe the format. In this case, the image > gets opened twice, and the first close comes before the second open. I'm > not entirely sure how hard it would be to get rid of that behaviour. > > If we can't get rid of it, we have a small window that the refcount > doesn't really cover, and if we weren't careful it would become racy. > This is why I mentioned earlier that maybe we need to defer the refcount > decrease a bit. However, I can't see how the in-use flag would make a > difference there. If the refcount can't cover it, the in-use flag can't > either. Yeah this is a problem. Could we introduce another flag to cover this?