Re: [PATCH 2/3] qemu: Implement virtio-pstore device
flat view
From: Stefan Hajnoczi <hidden>
Date: 2016-09-22 12:24:24
Also in:
kvm, lkml, qemu-devel
On Sun, Sep 04, 2016 at 11:38:59PM +0900, Namhyung Kim wrote:
quoted hunk ↗ jump to hunk
+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.
quoted hunk ↗ jump to hunk
+ } + + 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).
quoted hunk ↗ jump to hunk
+ + 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()?
quoted hunk ↗ jump to hunk
+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.