From: Corey Bryant <coreyb@linux.vnet.ibm.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
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
Subject: Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
Date: Tue, 03 Jul 2012 14:21:17 -0400 [thread overview]
Message-ID: <4FF3381D.40101@linux.vnet.ibm.com> (raw)
In-Reply-To: <4FF33349.10404@redhat.com>
On 07/03/2012 02:00 PM, Eric Blake wrote:
> On 07/03/2012 11:46 AM, Corey Bryant wrote:
>
>>
>> Yes, I think adding a +1 to the refcount for the monitor makes sense.
>>
>> I'm a bit unsure how to increment the refcount when a monitor reconnects
>> though. Maybe it is as simple as adding a +1 to each fd's refcount when
>> the next QMP monitor connects.
>
> Or maybe delay a +1 until after a 'query-fds' - it is not until the
> monitor has reconnected and learned what fds it should be aware of that
> incrementing the refcount again makes sense. But that would mean making
> 'query-fds' track whether this is the first call since the monitor
> reconnected, as it shouldn't normally increase refcounts.
This doesn't sound ideal.
>
> The other alternative is that the monitor never re-increments a
> refcount. Once a monitor disconnects, that fd is lost to the monitor,
> and a reconnected monitor must pass in a new fd to be re-associated with
> the fdset. In other words, the monitor's use of an fd is a one-way
> operation, starting life in use but ending at the first disconnect or
> remove-fd.
>
>
I would vote for this 2nd alternative. As long as we're not introducing
an fd leak. And I don't think we are if we decrement the refcount on
remove-fd or on QMP disconnect.
>>> 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
>>
>> And in step 4 the QMP connection will increment the refcount +1 for all
>> fds that persisted through the QMP disconnect. (?)
>
> I'm not sure we need the refcount increment on reconnect. 'query-fds'
> should provide enough information for the new monitor to know what
> fdsets are still in use by qemu, even though they are no longer
> available to 'remove-fd' from the monitor, and if the monitor is worried
> about keeping the fd set alive it can call 'add-fd' to again associate a
> new fd with the set. The lifetime of a set is thus as long as any of
> its associated fds have a non-zero refcount.
>
This sounds good to me.
And qemu_open will need to make sure the monitor in_use flag is true
before dup'ing an fd.
--
Regards,
Corey
next prev parent reply other threads:[~2012-07-03 18:22 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 [this message]
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
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=4FF3381D.40101@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.