Re: [PATCH 2/3] qemu: Implement virtio-pstore device
From: Stefan Hajnoczi
Date: Thu Sep 22 2016 - 08:24:27 EST
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.
> + }
> +
> + 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).
> +
> + 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()?
> +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.
Attachment:
signature.asc
Description: PGP signature