From: Corey Bryant <coreyb@linux.vnet.ibm.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com,
libvir-list@redhat.com, qemu-devel@nongnu.org,
Luiz Capitulino <lcapitulino@redhat.com>,
pbonzini@redhat.com, Eric Blake <eblake@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
Date: Fri, 06 Jul 2012 13:40:59 -0400 [thread overview]
Message-ID: <4FF7232B.4040101@linux.vnet.ibm.com> (raw)
In-Reply-To: <4FF6ABD9.4040502@redhat.com>
On 07/06/2012 05:11 AM, Kevin Wolf wrote:
> Am 05.07.2012 19:00, schrieb Eric Blake:
>> On 07/05/2012 10:35 AM, Corey Bryant wrote:
>>> 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
>>> refcount of 0; fd=4's in-use flag is turned on
>>> 2. client calls 'device-add' with /dev/fdset/1 as the backing filename,
>>> so qemu_open() increments the refcount of fdset1 to 1
>>> 3. client calls 'remove-fd fdset=1 fd=4', so qemu marks fd=4 as no
>>> longer in-use by the monitor, and is left open because it is in use by
>>> the block device (refcount is 1)
>>> 4. client crashes, so all tracked fds are visited; fd=4 is not in-use
>>> but refcount is 1 so it is not closed
>> 5. client re-establishes QMP connection, so all tracked fds are visited.
>> What happens to the fd=4 in-use flag?
>>
>> ...but what are the semantics here?
>>
>> If we _always_ turn the in-use flag back on, then that says that even
>> though libvirt successfully asked to close the fd, the reconnection
>> means that libvirt once again has to ask to close things.
>>
>> If we _never_ turn the in-use flag back on, then we've broken the first
>> case above where we want an in-use fd to come back into use after a crash.
>>
>> Maybe that argues for two flags per fd: an in-use flag (there is
>> currently a monitor connection that can manipulate the fd, either
>> because it passed in the fd or because it reconnected) and a removed
>> flag (a monitor called remove-fd, and no longer wants to know about the
>> fd, even if it crashes and reconnects).
>
> I was in fact just going to suggest a removed flag as well, however
> combined with including the monitor connections in the refcount instead
> of an additional flag. This would also allow to have (the currently
> mostly hypothetical case of) multiple QMP monitors without interesting
> things happening.
>
> Maybe I'm missing some point that the inuse flag would allow and
> including monitors in the refcount doesn't. Is there one?
>
> Kevin
>
Ok let me try this again. I was going through some of the examples and I
think we need a separate in-use flag. Otherwise, when refcount gets to
1, we don't know if it is because of a monitor reference or a block
device reference. I think it would cause fds to sit on the monitor
until refcount gets to zero (monitor disconnects). Here's an example
without the in-use flag:
1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
refcount of 1 (incremented because of monitor reference); fd=4's remove
flag is initialized to off
2. client calls 'device-add' with /dev/fdset/1 as the backing filename;
qemu_open() increments the refcount of fdset1 to 2
3. client crashes, so all fdsets are visited; fd=4 had not yet been
passed to 'remove-fd', so it's remove flag is off; refcount for fdset1
is decremented to 1; fd=4 is left open because it is still in use by the
block device (refcount is 1)
4. client re-establishes QMP connection, refcount for fdset1 is
incremented to 2; 'query-fds' lets client learn about fd=4 still being
open as part of fdset1
5. client calls 'remove-fd fdset=1 fd=4'; qemu turns on remove flag for
fd=4; but fd=4 remains open because refcount of fdset1 is 2
6. qemu_close is called for fd=4; refcount for fdset1 is decremented to
1; fd=4 remains open because monitor still references fdset1 (refcount
of fdset1 is 1)
7. Sometime later.. QMP disconnects; refcount for fdset is decremented
to zero; fd=4 is closed
In the following case, we have an in-use and remove flag per fd and we
only increment/decrement refcount on qemu_open/qemu_close:
1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
refcount of 0; fd=4's remove flag is initialized to off and in-use flag
is initialized to on
2. client calls 'device-add' with /dev/fdset/1 as the backing filename;
qemu_open() increments the refcount of fdset1 to 1
3. client crashes, so all fdsets are visited; fd=4 had not yet been
passed to 'remove-fd', so it's remove flag is off; fd=4's in-use flag is
turned off; fd=4 is left open because it is still in-use by the block
device (refcount is still 1)
4. client re-establishes QMP connection, refcount for fdset1 is still 1;
fd=4's in-use flag is turned on; 'query-fds' lets client learn about
fd=4 still being open as part of fdset1
5. client calls 'remove-fd fdset=1 fd=4'; qemu turns on remove flag for
fd=4; but fd=4 remains open because refcount of fdset1 is 1
6. qemu_close is called for fd=4; refcount for fdset1 is decremented to
0; fd=4 is closed because (refcount == 0 && (!inuse || removed)) is true
7. Sometime later.. QMP disconnects
--
Regards,
Corey
next prev parent reply other threads:[~2012-07-06 17:41 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-22 18:36 [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd Corey Bryant
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 1/7] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg Corey Bryant
2012-06-22 19:31 ` Eric Blake
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 2/7] qapi: Convert getfd and closefd Corey Bryant
2012-07-11 18:51 ` Luiz Capitulino
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 3/7] qapi: Add pass-fd QMP command Corey Bryant
2012-06-22 20:24 ` Eric Blake
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 4/7] qapi: Re-arrange monitor.c functions Corey Bryant
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 5/7] block: Prevent /dev/fd/X filename from being detected as floppy Corey Bryant
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 6/7] block: Convert open calls to qemu_open Corey Bryant
2012-06-22 18:36 ` [Qemu-devel] [PATCH v4 7/7] osdep: Enable qemu_open to dup pre-opened fd Corey Bryant
2012-06-22 19:58 ` Eric Blake
[not found] ` <20120626091004.GA14451@redhat.com>
[not found] ` <4FE9A0F0.2050809@redhat.com>
[not found] ` <20120626175045.2c7011b3@doriath.home>
[not found] ` <4FEA37A9.10707@linux.vnet.ibm.com>
[not found] ` <4FEA3D9C.8080205@redhat.com>
2012-07-02 22:02 ` [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd Corey Bryant
2012-07-02 22:31 ` Eric Blake
2012-07-03 9:07 ` Daniel P. Berrange
2012-07-03 9:40 ` Kevin Wolf
2012-07-03 13:42 ` Corey Bryant
2012-07-03 15:40 ` Corey Bryant
2012-07-03 15:59 ` Kevin Wolf
2012-07-03 16:25 ` Corey Bryant
2012-07-03 17:03 ` Eric Blake
2012-07-03 17:46 ` Corey Bryant
2012-07-03 18:00 ` Eric Blake
2012-07-03 18:21 ` Corey Bryant
2012-07-04 8:09 ` Kevin Wolf
2012-07-05 15:06 ` Corey Bryant
2012-07-09 14:05 ` Luiz Capitulino
2012-07-09 15:05 ` Corey Bryant
2012-07-09 15:46 ` Kevin Wolf
2012-07-09 16:18 ` Luiz Capitulino
2012-07-09 17:59 ` Corey Bryant
2012-07-09 17:35 ` Corey Bryant
2012-07-09 17:48 ` Luiz Capitulino
2012-07-09 18:02 ` Corey Bryant
2012-07-10 7:53 ` Kevin Wolf
2012-07-09 18:20 ` Corey Bryant
2012-07-04 8:00 ` Kevin Wolf
2012-07-05 14:22 ` Corey Bryant
2012-07-05 14:51 ` Kevin Wolf
2012-07-05 16:35 ` Corey Bryant
2012-07-05 16:37 ` Corey Bryant
2012-07-06 9:06 ` Kevin Wolf
2012-07-05 17:00 ` Eric Blake
2012-07-05 17:36 ` Corey Bryant
2012-07-06 9:11 ` Kevin Wolf
2012-07-06 17:14 ` Corey Bryant
2012-07-06 17:15 ` Corey Bryant
2012-07-06 17:40 ` Corey Bryant [this message]
2012-07-06 18:19 ` [Qemu-devel] [libvirt] " Corey Bryant
2012-07-09 14:04 ` [Qemu-devel] " Kevin Wolf
2012-07-09 15:23 ` Corey Bryant
2012-07-09 15:30 ` Kevin Wolf
2012-07-09 18:40 ` Anthony Liguori
2012-07-09 19:00 ` Luiz Capitulino
2012-07-10 8:54 ` Daniel P. Berrange
2012-07-10 7:58 ` Kevin Wolf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FF7232B.4040101@linux.vnet.ibm.com \
--to=coreyb@linux.vnet.ibm.com \
--cc=aliguori@us.ibm.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=lcapitulino@redhat.com \
--cc=libvir-list@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).