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: kwolf@redhat.com, aliguori@us.ibm.com,
	stefanha@linux.vnet.ibm.com, libvir-list@redhat.com,
	qemu-devel@nongnu.org, lcapitulino@redhat.com,
	pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets
Date: Mon, 13 Aug 2012 10:16:54 -0600	[thread overview]
Message-ID: <50292876.60808@redhat.com> (raw)
In-Reply-To: <502904C6.6050903@linux.vnet.ibm.com>

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

On 08/13/2012 07:44 AM, Corey Bryant wrote:
> I'll send a new version shortly with these updates also.
> 

>>> +
>>> +        ret = monitor_fdset_dup_fd_add(fdset_id, dupfd);
>>> +        if (ret == -1) {
>>> +            close(dupfd);
>>> +            return -1;
>>
>> This function appears to promise a reasonable errno on failure.

Actually, looking at that function again,


+int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+{
+    MonFdset *mon_fdset;
+    MonFdsetFd *mon_fdset_fd_dup;
+
+    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+        if (mon_fdset->id != fdset_id) {
+            continue;
+        }
+        QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
+            if (mon_fdset_fd_dup->fd == dup_fd) {
+                return -1;
+            }
+        }
+        mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
+        mon_fdset_fd_dup->fd = dup_fd;
+        QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next);
+        return 0;
+    }
+    return -1;
+}

The only way it could fail is if we are trying to add an fd that is
already in the set, or if we don't find mon_fdset; both of which would
indicate logic bugs earlier in our program.  Would it be worth asserting
that these conditions are impossible, and making this function return
void (the addition is always successful if it returns, since g_malloc0
aborts rather than failing with ENOMEM)?

And the more I think about it, the more I think that qemu_open MUST
provide a sane errno value on exit, so you need to make sure that all
exit paths out of qemu_open have a sensible errno (whether or not the
helper functions also have to leave errno sane is a matter of taste).

-- 
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-08-13 16:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-11 13:14 [Qemu-devel] [PATCH v9 0/7] file descriptor passing using fd sets Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 1/7] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets Corey Bryant
2012-08-11 14:16   ` Eric Blake
2012-08-13 13:44     ` Corey Bryant
2012-08-13 13:44     ` Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 3/7] block: Prevent detection of /dev/fdset/ as floppy Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 4/7] block: Convert open calls to qemu_open Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 5/7] block: Convert close calls to qemu_close Corey Bryant
2012-08-11 13:22   ` Blue Swirl
2012-08-13 13:43     ` Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets Corey Bryant
2012-08-11 14:28   ` Eric Blake
2012-08-13 13:44     ` Corey Bryant
2012-08-13 16:16       ` Eric Blake [this message]
2012-08-13 16:33         ` Corey Bryant
2012-08-13 17:13           ` Eric Blake
2012-08-13 17:32             ` Corey Bryant
2012-08-11 13:14 ` [Qemu-devel] [PATCH v9 7/7] monitor: Clean up fd sets on monitor disconnect 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=50292876.60808@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=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).