[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes,
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration |
Date: |
Tue, 20 Nov 2012 19:44:55 -0600 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Amit Shah <address@hidden> writes:
> Popping an elem from the vq just to find out its length causes problems
> with save/load later on. Use the new virtqueue_get_avail_bytes()
> function instead, saves us the complexity in the migration code, as well
> as makes the migration endian-safe.
>
> Signed-off-by: Amit Shah <address@hidden>
To be clear, migration is already endian safe but this is definitely a
very nice cleanup.
Reviewed-by: Anthony Liguori <address@hidden>
Regards,
Anthony Liguori
> ---
> hw/virtio-rng.c | 76
> +++++++++------------------------------------------------
> 1 file changed, 12 insertions(+), 64 deletions(-)
>
> diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
> index 3ca96c8..6c49bb2 100644
> --- a/hw/virtio-rng.c
> +++ b/hw/virtio-rng.c
> @@ -22,14 +22,10 @@ typedef struct VirtIORNG {
>
> /* Only one vq - guest puts buffer(s) on it when it needs entropy */
> VirtQueue *vq;
> - VirtQueueElement elem;
>
> /* Config data for the device -- currently only chardev */
> VirtIORNGConf *conf;
>
> - /* Whether we've popped a vq element into 'elem' above */
> - bool popped;
> -
> RngBackend *rng;
>
> /* We purposefully don't migrate this state. The quota will reset on the
> @@ -48,17 +44,12 @@ static bool is_guest_ready(VirtIORNG *vrng)
> return false;
> }
>
> -static size_t pop_an_elem(VirtIORNG *vrng)
> +static size_t get_request_size(VirtQueue *vq)
> {
> - size_t size;
> + unsigned int in, out;
>
> - if (!vrng->popped && !virtqueue_pop(vrng->vq, &vrng->elem)) {
> - return 0;
> - }
> - vrng->popped = true;
> -
> - size = iov_size(vrng->elem.in_sg, vrng->elem.in_num);
> - return size;
> + virtqueue_get_avail_bytes(vq, &in, &out);
> + return in;
> }
>
> static void virtio_rng_process(VirtIORNG *vrng);
> @@ -67,6 +58,7 @@ static void virtio_rng_process(VirtIORNG *vrng);
> static void chr_read(void *opaque, const void *buf, size_t size)
> {
> VirtIORNG *vrng = opaque;
> + VirtQueueElement elem;
> size_t len;
> int offset;
>
> @@ -78,15 +70,14 @@ static void chr_read(void *opaque, const void *buf,
> size_t size)
>
> offset = 0;
> while (offset < size) {
> - if (!pop_an_elem(vrng)) {
> + if (!virtqueue_pop(vrng->vq, &elem)) {
> break;
> }
> - len = iov_from_buf(vrng->elem.in_sg, vrng->elem.in_num,
> + len = iov_from_buf(elem.in_sg, elem.in_num,
> 0, buf + offset, size - offset);
> offset += len;
>
> - virtqueue_push(vrng->vq, &vrng->elem, len);
> - vrng->popped = false;
> + virtqueue_push(vrng->vq, &elem, len);
> }
> virtio_notify(&vrng->vdev, vrng->vq);
>
> @@ -100,21 +91,19 @@ static void chr_read(void *opaque, const void *buf,
> size_t size)
>
> static void virtio_rng_process(VirtIORNG *vrng)
> {
> - ssize_t size;
> + size_t size;
>
> if (!is_guest_ready(vrng)) {
> return;
> }
>
> - size = pop_an_elem(vrng);
> + size = get_request_size(vrng->vq);
> size = MIN(vrng->quota_remaining, size);
> -
> - if (size > 0) {
> + if (size) {
> rng_backend_request_entropy(vrng->rng, size, chr_read, vrng);
> }
> }
>
> -
> static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
> {
> VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
> @@ -131,23 +120,6 @@ static void virtio_rng_save(QEMUFile *f, void *opaque)
> VirtIORNG *vrng = opaque;
>
> virtio_save(&vrng->vdev, f);
> -
> - qemu_put_byte(f, vrng->popped);
> - if (vrng->popped) {
> - int i;
> -
> - qemu_put_be32(f, vrng->elem.index);
> -
> - qemu_put_be32(f, vrng->elem.in_num);
> - for (i = 0; i < vrng->elem.in_num; i++) {
> - qemu_put_be64(f, vrng->elem.in_addr[i]);
> - }
> -
> - qemu_put_be32(f, vrng->elem.out_num);
> - for (i = 0; i < vrng->elem.out_num; i++) {
> - qemu_put_be64(f, vrng->elem.out_addr[i]);
> - }
> - }
> }
>
> static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
> @@ -159,30 +131,6 @@ static int virtio_rng_load(QEMUFile *f, void *opaque,
> int version_id)
> }
> virtio_load(&vrng->vdev, f);
>
> - vrng->popped = qemu_get_byte(f);
> - if (vrng->popped) {
> - int i;
> -
> - vrng->elem.index = qemu_get_be32(f);
> -
> - vrng->elem.in_num = qemu_get_be32(f);
> - g_assert(vrng->elem.in_num < VIRTQUEUE_MAX_SIZE);
> - for (i = 0; i < vrng->elem.in_num; i++) {
> - vrng->elem.in_addr[i] = qemu_get_be64(f);
> - }
> -
> - vrng->elem.out_num = qemu_get_be32(f);
> - g_assert(vrng->elem.out_num < VIRTQUEUE_MAX_SIZE);
> - for (i = 0; i < vrng->elem.out_num; i++) {
> - vrng->elem.out_addr[i] = qemu_get_be64(f);
> - }
> -
> - virtqueue_map_sg(vrng->elem.in_sg, vrng->elem.in_addr,
> - vrng->elem.in_num, 1);
> - virtqueue_map_sg(vrng->elem.out_sg, vrng->elem.out_addr,
> - vrng->elem.out_num, 0);
> - }
> -
> /* We may have an element ready but couldn't process it due to a quota
> limit. Make sure to try again after live migration when the quota may
> have been reset.
> @@ -232,7 +180,7 @@ VirtIODevice *virtio_rng_init(DeviceState *dev,
> VirtIORNGConf *conf)
>
> vrng->qdev = dev;
> vrng->conf = conf;
> - vrng->popped = false;
> +
> vrng->quota_remaining = vrng->conf->max_bytes;
>
> g_assert_cmpint(vrng->conf->max_bytes, <=, INT64_MAX);
> --
> 1.8.0
- [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes, Amit Shah, 2012/11/20
- [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy, Amit Shah, 2012/11/20
- [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration, Amit Shah, 2012/11/20
- Re: [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration,
Anthony Liguori <=
- [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal, Amit Shah, 2012/11/20
- [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments, Amit Shah, 2012/11/20