qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Corey Bryant <coreyb@linux.vnet.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com,
	libvir-list@redhat.com, qemu-devel@nongnu.org,
	lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets
Date: Thu, 26 Jul 2012 07:13:02 -0600	[thread overview]
Message-ID: <5011425E.8030201@redhat.com> (raw)
In-Reply-To: <5010B7D3.6090309@linux.vnet.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

On 07/25/2012 09:21 PM, Corey Bryant wrote:
> 
> 
> On 07/25/2012 03:25 PM, Eric Blake wrote:
>> On 07/25/2012 02:22 AM, Kevin Wolf wrote:
>>>>> Hm, not a nice interface where qemu_close() needs the filename and
>>>>> (worse) could be given a wrong filename. Maybe it would be better to
>>>>> maintain a list of fd -> fdset mappings in qemu_open/close?
>>>>>
>>>>
>>>> I agree, I don't really like it either.
>>>>
>>>> We already have a list of fd -> fdset mappings (mon_fdset_fd_t ->
>>>> mon_fdset_t).  Would it be too costly to loop through all the
>>>> fdsets/fds
>>>> at the beginning of every qemu_close()?
>>>
>>> I don't think so. qemu_close() is not a fast path and happens almost
>>> never, and the list is short enough that searching it isn't a problem
>>> anyway.
>>
>> I agree - just do the loop to do the reverse lookup yourself, rather
>> than making qemu_close() have a different signature than close().
>>
> 
> Great, I'll do this then.

You may want an optimization of using a bitset for tracking which fds
are tracked by fdset in the first place, so that the fast path of
qemu_close() will be a check against the bitset to see if you even have
to waste time on the reverse lookup in the first place.  The bitset will
typically be small (bounded not only by the maximum possible fd, but
further by the fact that we don't usually open that many fds in the
first place), but I'm not sure if you can get away with static sizing.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

  reply	other threads:[~2012-07-26 13:13 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-23 13:07 [Qemu-devel] [PATCH v5 0/6] file descriptor passing using fd sets Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg Corey Bryant
2012-07-23 22:50   ` Eric Blake
2012-07-24  2:19     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets Corey Bryant
2012-07-25 18:16   ` Eric Blake
2012-07-26  2:55     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 3/6] monitor: Clean up fd sets on monitor disconnect Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 4/6] block: Convert open calls to qemu_open Corey Bryant
2012-07-25 19:22   ` Eric Blake
2012-07-26  3:11     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 5/6] block: Convert close calls to qemu_close Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets Corey Bryant
2012-07-23 13:14   ` Corey Bryant
2012-08-02 22:21     ` Corey Bryant
2012-08-06  9:15       ` Kevin Wolf
2012-08-06 13:32         ` Corey Bryant
2012-08-06 13:51           ` Kevin Wolf
2012-08-06 14:15             ` Corey Bryant
2012-08-07 16:43               ` Corey Bryant
2012-07-24 12:07   ` Kevin Wolf
2012-07-25  3:41     ` Corey Bryant
2012-07-25  8:22       ` Kevin Wolf
2012-07-25 19:25         ` Eric Blake
2012-07-26  3:21           ` Corey Bryant
2012-07-26 13:13             ` Eric Blake [this message]
2012-07-26 13:16               ` Kevin Wolf
2012-07-27  4:07                 ` Corey Bryant
2012-07-25 19:43   ` Eric Blake
2012-07-26  3:57     ` Corey Bryant
2012-07-26  9:07       ` Kevin Wolf
2012-07-27  3:59         ` Corey Bryant
2012-07-27  4:03         ` Corey Bryant
2012-08-02 15:08       ` Corey Bryant
2012-07-24 12:09 ` [Qemu-devel] [PATCH v5 0/6] file descriptor passing using " Kevin Wolf
2012-07-25  3:42   ` Corey Bryant

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=5011425E.8030201@redhat.com \
    --to=eblake@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=coreyb@linux.vnet.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=libvir-list@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).