From: Paolo Bonzini <pbonzini@redhat.com>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: target-devel <target-devel@vger.kernel.org>,
lf-virt <virtualization@lists.linux-foundation.org>,
kvm-devel <kvm@vger.kernel.org>,
qemu-devel <qemu-devel@nongnu.org>,
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
Zhi Yong Wu <wuzhy@cn.ibm.com>,
Anthony Liguori <aliguori@linux.vnet.ibm.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
Hannes Reinecke <hare@suse.de>,
Anthony Liguori <aliguori@us.ibm.com>
Subject: Re: [RFC 1/9] notifier: add validity check and notify function
Date: Wed, 25 Jul 2012 08:53:29 +0200 [thread overview]
Message-ID: <500F97E9.4070508@redhat.com> (raw)
In-Reply-To: <1343169246-17636-2-git-send-email-nab@linux-iscsi.org>
Il 25/07/2012 00:33, Nicholas A. Bellinger ha scritto:
> +int event_notifier_notify(EventNotifier *e)
> +{
> + uint64_t value = 1;
> + int r;
> +
> + assert(event_notifier_valid(e));
> + r = write(e->fd, &value, sizeof(value));
> + if (r < 0) {
> + return -errno;
> + }
> + assert(r == sizeof(value));
> + return 0;
> +}
Note we now have event_notifier_set.
> +#define EVENT_NOTIFIER_INITIALIZER ((EventNotifier){ .fd = -1 })
This is problematic when your event notifier is inside a struct, because
it is extremely easy to forget the initializer. You have to initialize
them yourself. Also, the right thing to test is not whether the
notifier is initialized; it is whether the notifier is actually checked
in QEMU's select() loop.
So, I would prefer avoiding event_notifier_valid and just use a boolean
(in virtio_queue_set_host_notifier_fd_handler and
virtio_queue_set_guest_notifier_fd_handler) to track whether the
notifiers are in use.
Paolo
WARNING: multiple messages have this Message-ID (diff)
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: Jens Axboe <axboe@kernel.dk>,
Anthony Liguori <aliguori@us.ibm.com>,
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
kvm-devel <kvm@vger.kernel.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
qemu-devel <qemu-devel@nongnu.org>,
Zhi Yong Wu <wuzhy@cn.ibm.com>,
Anthony Liguori <aliguori@linux.vnet.ibm.com>,
target-devel <target-devel@vger.kernel.org>,
Hannes Reinecke <hare@suse.de>,
lf-virt <virtualization@lists.linux-foundation.org>,
Christoph Hellwig <hch@lst.de>
Subject: Re: [Qemu-devel] [RFC 1/9] notifier: add validity check and notify function
Date: Wed, 25 Jul 2012 08:53:29 +0200 [thread overview]
Message-ID: <500F97E9.4070508@redhat.com> (raw)
In-Reply-To: <1343169246-17636-2-git-send-email-nab@linux-iscsi.org>
Il 25/07/2012 00:33, Nicholas A. Bellinger ha scritto:
> +int event_notifier_notify(EventNotifier *e)
> +{
> + uint64_t value = 1;
> + int r;
> +
> + assert(event_notifier_valid(e));
> + r = write(e->fd, &value, sizeof(value));
> + if (r < 0) {
> + return -errno;
> + }
> + assert(r == sizeof(value));
> + return 0;
> +}
Note we now have event_notifier_set.
> +#define EVENT_NOTIFIER_INITIALIZER ((EventNotifier){ .fd = -1 })
This is problematic when your event notifier is inside a struct, because
it is extremely easy to forget the initializer. You have to initialize
them yourself. Also, the right thing to test is not whether the
notifier is initialized; it is whether the notifier is actually checked
in QEMU's select() loop.
So, I would prefer avoiding event_notifier_valid and just use a boolean
(in virtio_queue_set_host_notifier_fd_handler and
virtio_queue_set_guest_notifier_fd_handler) to track whether the
notifiers are in use.
Paolo
next prev parent reply other threads:[~2012-07-25 6:53 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-24 22:33 [RFC 0/9] vhost-scsi: Add support for host virtualized target Nicholas A. Bellinger
2012-07-24 22:33 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-24 22:33 ` [RFC 1/9] notifier: add validity check and notify function Nicholas A. Bellinger
2012-07-24 22:33 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-25 6:53 ` Paolo Bonzini [this message]
2012-07-25 6:53 ` Paolo Bonzini
2012-07-25 6:53 ` Paolo Bonzini
2012-07-24 22:33 ` Nicholas A. Bellinger
2012-07-24 22:33 ` [RFC 2/9] virtio-pci: support host notifiers in TCG mode Nicholas A. Bellinger
2012-07-24 22:33 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-25 6:50 ` Paolo Bonzini
2012-07-25 6:50 ` [Qemu-devel] " Paolo Bonzini
2012-07-24 22:33 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [RFC 3/9] virtio-pci: check that event notification worked Nicholas A. Bellinger
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-24 22:34 ` [RFC 4/9] vhost: Pass device path to vhost_dev_init() Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [RFC 5/9] virtio-scsi: Add wwpn and tgpt properties Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [RFC 6/9] virtio-scsi: Open and initialize /dev/vhost-scsi Nicholas A. Bellinger
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-25 7:05 ` Paolo Bonzini
2012-07-25 7:05 ` [Qemu-devel] " Paolo Bonzini
2012-07-24 22:34 ` [RFC 7/9] virtio-scsi: Start/stop vhost Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-25 7:01 ` Paolo Bonzini
2012-07-25 7:01 ` [Qemu-devel] " Paolo Bonzini
2012-07-25 7:03 ` Paolo Bonzini
2012-07-25 7:03 ` Paolo Bonzini
2012-07-25 7:03 ` [Qemu-devel] " Paolo Bonzini
2012-07-25 7:01 ` Paolo Bonzini
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [RFC 8/9] virtio-scsi: Set max_target=0 during vhost-scsi operation Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [RFC 9/9] vhost-scsi: add -vhost-scsi host device Nicholas A. Bellinger
2012-07-24 22:34 ` Nicholas A. Bellinger
2012-07-24 22:34 ` [Qemu-devel] " Nicholas A. Bellinger
2012-07-25 6:58 ` Paolo Bonzini
2012-07-25 6:58 ` [Qemu-devel] " Paolo Bonzini
2012-07-25 2:53 ` [RFC 0/9] vhost-scsi: Add support for host virtualized target Zhi Yong Wu
2012-07-25 2:53 ` Zhi Yong Wu
2012-07-25 2:53 ` [Qemu-devel] " Zhi Yong Wu
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=500F97E9.4070508@redhat.com \
--to=pbonzini@redhat.com \
--cc=aliguori@linux.vnet.ibm.com \
--cc=aliguori@us.ibm.com \
--cc=axboe@kernel.dk \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=kvm@vger.kernel.org \
--cc=mst@redhat.com \
--cc=nab@linux-iscsi.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@linux.vnet.ibm.com \
--cc=target-devel@vger.kernel.org \
--cc=virtualization@lists.linux-foundation.org \
--cc=wuzhy@cn.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.