From: Namhyung Kim <namhyung@kernel.org>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: virtio-dev@lists.oasis-open.org,
"Anton Vorontsov" <anton@enomsg.org>,
"Daniel P . Berrange" <berrange@redhat.com>,
"Kees Cook" <keescook@chromium.org>,
kvm@vger.kernel.org, "Radim Krčmář" <rkrcmar@redhat.com>,
qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
"Steven Rostedt" <rostedt@goodmis.org>,
virtualization@lists.linux-foundation.org,
"Minchan Kim" <minchan@kernel.org>,
"Tony Luck" <tony.luck@intel.com>,
"Anthony Liguori" <aliguori@amazon.com>,
"Colin Cross" <ccross@android.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Ingo Molnar" <mingo@kernel.org>
Subject: Re: [PATCH 2/3] qemu: Implement virtio-pstore device
Date: Fri, 23 Sep 2016 14:52:05 +0900 [thread overview]
Message-ID: <20160923055205.GC16119@danjae.aot.lge.com> (raw)
In-Reply-To: <20160922122316.GB8221@stefanha-x1.localdomain>
On Thu, Sep 22, 2016 at 01:23:16PM +0100, Stefan Hajnoczi wrote:
> On Sun, Sep 04, 2016 at 11:38:59PM +0900, Namhyung Kim wrote:
> > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq)
> > +{
> > + VirtIOPstore *s = VIRTIO_PSTORE(vdev);
> > + VirtQueueElement *elem;
> > + struct virtio_pstore_req req;
> > + struct virtio_pstore_res res;
> > + ssize_t len = 0;
> > + int ret;
> > +
> > + for (;;) {
> > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> > + if (!elem) {
> > + return;
> > + }
> > +
> > + if (elem->out_num < 1 || elem->in_num < 1) {
> > + error_report("request or response buffer is missing");
> > + exit(1);
>
> The new virtio_error() function might be available, depending on when
> this patch series is merged. virtio_error() should be used instead of
> exit(1). See "[PATCH v5 0/9] virtio: avoid exit() when device enters
> invalid states" on qemu-devel.
Thanks for the info, will take a look.
>
> > + }
> > +
> > + if (elem->out_num > 2 || elem->in_num > 3) {
> > + error_report("invalid number of input/output buffer");
> > + exit(1);
> > + }
>
> The VIRTIO specification requires that flexible framing is supported.
> The device cannot make assumptions about the scatter-gather list. It
> must support any layout (e.g. even multiple 1-byte iovecs making up the
> buffer).
Ok.
>
> > +
> > + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> > + if (len != (ssize_t)sizeof(req)) {
> > + error_report("invalid request size: %ld", (long)len);
> > + exit(1);
> > + }
> > + res.cmd = req.cmd;
> > + res.type = req.type;
> > +
> > + switch (le16_to_cpu(req.cmd)) {
> > + case VIRTIO_PSTORE_CMD_OPEN:
> > + ret = virtio_pstore_do_open(s);
> > + break;
> > + case VIRTIO_PSTORE_CMD_CLOSE:
> > + ret = virtio_pstore_do_close(s);
> > + break;
> > + case VIRTIO_PSTORE_CMD_ERASE:
> > + ret = virtio_pstore_do_erase(s, &req);
> > + break;
> > + case VIRTIO_PSTORE_CMD_READ:
> > + ret = virtio_pstore_do_read(s, elem);
> > + if (ret == 1) {
> > + /* async channel io */
> > + continue;
> > + }
> > + break;
> > + case VIRTIO_PSTORE_CMD_WRITE:
> > + ret = virtio_pstore_do_write(s, elem, &req);
> > + if (ret == 1) {
> > + /* async channel io */
> > + continue;
> > + }
> > + break;
> > + default:
> > + ret = -1;
> > + break;
> > + }
> > +
> > + res.ret = ret;
>
> Missing cpu_to_le()?
Right!
>
> > +static void pstore_set_bufsize(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + VirtIOPstore *s = opaque;
> > + Error *error = NULL;
> > + uint64_t value;
> > +
> > + visit_type_size(v, name, &value, &error);
> > + if (error) {
> > + error_propagate(errp, error);
> > + return;
> > + }
> > +
> > + if (value < 4096) {
> > + error_setg(&error, "Warning: too small buffer size: %"PRIu64, value);
>
> This is an error, not a warning. Please remove "Warning:" so it's clear
> to the user that this message caused QEMU to fail.
Will do.
Thanks,
Namhyung
next prev parent reply other threads:[~2016-09-23 5:52 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-04 14:38 [RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v5) Namhyung Kim
2016-09-04 14:38 ` [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Namhyung Kim
2016-09-08 20:49 ` Kees Cook
2016-09-22 11:57 ` Stefan Hajnoczi
2016-09-23 5:48 ` Namhyung Kim
2016-09-04 14:38 ` [PATCH 2/3] qemu: Implement virtio-pstore device Namhyung Kim
2016-09-22 12:23 ` Stefan Hajnoczi
2016-09-23 5:52 ` Namhyung Kim [this message]
2016-09-04 14:39 ` [PATCH 3/3] kvmtool: " Namhyung Kim
-- strict thread matches above, loose matches on Subject: below --
2016-08-31 8:07 [RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v4) Namhyung Kim
2016-08-31 8:08 ` [PATCH 2/3] qemu: Implement virtio-pstore device Namhyung Kim
2016-08-20 8:07 [RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v3) Namhyung Kim
2016-08-20 8:07 ` [PATCH 2/3] qemu: Implement virtio-pstore device Namhyung Kim
2016-08-24 22:00 ` Daniel P. Berrange
2016-08-26 4:48 ` Namhyung Kim
2016-08-26 12:27 ` Daniel P. Berrange
2016-09-13 15:57 ` Michael S. Tsirkin
2016-09-16 10:05 ` Namhyung Kim
2016-11-10 22:50 ` Michael S. Tsirkin
2016-11-15 6:23 ` Namhyung Kim
2016-11-15 14:38 ` Michael S. Tsirkin
2016-07-18 4:37 [RFC/PATCHSET 0/3] virtio-pstore: Implement virtio pstore device Namhyung Kim
2016-07-18 4:37 ` [PATCH 2/3] qemu: Implement virtio-pstore device Namhyung Kim
2016-07-18 7:28 ` Christian Borntraeger
2016-07-18 8:33 ` Namhyung Kim
2016-07-18 10:03 ` Stefan Hajnoczi
2016-07-18 14:21 ` Namhyung Kim
2016-07-20 8:29 ` Stefan Hajnoczi
2016-07-20 12:46 ` Namhyung Kim
2016-07-19 15:48 ` Namhyung Kim
2016-07-20 8:21 ` Stefan Hajnoczi
2016-07-20 12:30 ` Namhyung Kim
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=20160923055205.GC16119@danjae.aot.lge.com \
--to=namhyung@kernel.org \
--cc=aliguori@amazon.com \
--cc=anton@enomsg.org \
--cc=berrange@redhat.com \
--cc=ccross@android.com \
--cc=keescook@chromium.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=minchan@kernel.org \
--cc=mingo@kernel.org \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rkrcmar@redhat.com \
--cc=rostedt@goodmis.org \
--cc=stefanha@gmail.com \
--cc=tony.luck@intel.com \
--cc=virtio-dev@lists.oasis-open.org \
--cc=virtualization@lists.linux-foundation.org \
/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).