* [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes
@ 2012-11-20 14:26 Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration Amit Shah
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Amit Shah @ 2012-11-20 14:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list
Hello,
This series fixes a few things, especially the migration support,
making it endian-safe.
New in this submission: delete the timer on device remove, else
hot-unplug of an rng device could result in a bad access.
The backend should be decoupled on hot-unplug of the virtio-rng device
as well, but that's for later.
Please review and apply.
Amit Shah (4):
virtio-rng: use virtqueue_get_avail_bytes, fix migration
virtio-rng: remove extra request for entropy
virtio-rng: disable timer on device removal
virtio-rng: fix typos, comments
hw/virtio-rng.c | 92 ++++++++++--------------------------------------------
include/qemu/rng.h | 6 ++--
2 files changed, 20 insertions(+), 78 deletions(-)
--
1.8.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration
2012-11-20 14:26 [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes Amit Shah
@ 2012-11-20 14:26 ` Amit Shah
2012-11-21 1:44 ` Anthony Liguori
2012-11-20 14:26 ` [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy Amit Shah
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Amit Shah @ 2012-11-20 14:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list
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 <amit.shah@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy
2012-11-20 14:26 [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration Amit Shah
@ 2012-11-20 14:26 ` Amit Shah
2012-11-21 1:41 ` Anthony Liguori
2012-11-20 14:26 ` [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments Amit Shah
3 siblings, 1 reply; 11+ messages in thread
From: Amit Shah @ 2012-11-20 14:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list
If we got fewer bytes from the backend than requested, don't poke the
backend for more bytes; the guest will ask for more (or if the guest has
already asked for more, the backend knows about it via handle_input()).
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/virtio-rng.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
index 6c49bb2..cf5a8ff 100644
--- a/hw/virtio-rng.c
+++ b/hw/virtio-rng.c
@@ -80,13 +80,6 @@ static void chr_read(void *opaque, const void *buf, size_t size)
virtqueue_push(vrng->vq, &elem, len);
}
virtio_notify(&vrng->vdev, vrng->vq);
-
- /*
- * Lastly, if we had multiple elems queued by the guest, and we
- * didn't have enough data to fill them all, indicate we want more
- * data.
- */
- virtio_rng_process(vrng);
}
static void virtio_rng_process(VirtIORNG *vrng)
--
1.8.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal
2012-11-20 14:26 [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy Amit Shah
@ 2012-11-20 14:26 ` Amit Shah
2012-11-21 1:41 ` Anthony Liguori
2012-11-20 14:26 ` [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments Amit Shah
3 siblings, 1 reply; 11+ messages in thread
From: Amit Shah @ 2012-11-20 14:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list
Disable the rate-limit timer on device remove (e.g. hot-unplug).
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/virtio-rng.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
index cf5a8ff..c8a6da7 100644
--- a/hw/virtio-rng.c
+++ b/hw/virtio-rng.c
@@ -194,6 +194,8 @@ void virtio_rng_exit(VirtIODevice *vdev)
{
VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
+ qemu_del_timer(vrng->rate_limit_timer);
+ qemu_free_timer(vrng->rate_limit_timer);
unregister_savevm(vrng->qdev, "virtio-rng", vrng);
virtio_cleanup(vdev);
}
--
1.8.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments
2012-11-20 14:26 [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes Amit Shah
` (2 preceding siblings ...)
2012-11-20 14:26 ` [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal Amit Shah
@ 2012-11-20 14:26 ` Amit Shah
2012-11-21 1:43 ` Anthony Liguori
3 siblings, 1 reply; 11+ messages in thread
From: Amit Shah @ 2012-11-20 14:26 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Amit Shah, Paolo Bonzini, qemu list
Fix typos, whitespace and update comments to match current
implementation.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/virtio-rng.c | 7 +++----
include/qemu/rng.h | 6 +++---
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
index c8a6da7..f4ed9cf 100644
--- a/hw/virtio-rng.c
+++ b/hw/virtio-rng.c
@@ -23,7 +23,6 @@ typedef struct VirtIORNG {
/* Only one vq - guest puts buffer(s) on it when it needs entropy */
VirtQueue *vq;
- /* Config data for the device -- currently only chardev */
VirtIORNGConf *conf;
RngBackend *rng;
@@ -125,9 +124,9 @@ static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
virtio_load(&vrng->vdev, f);
/* 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.
- */
+ * limit. Make sure to try again after live migration when the quota may
+ * have been reset.
+ */
virtio_rng_process(vrng);
return 0;
diff --git a/include/qemu/rng.h b/include/qemu/rng.h
index 7e9d672..9836463 100644
--- a/include/qemu/rng.h
+++ b/include/qemu/rng.h
@@ -61,10 +61,10 @@ struct RngBackend
* This function is used by the front-end to request entropy from an entropy
* source. This function can be called multiple times before @receive_entropy
* is invoked with different values of @receive_entropy and @opaque. The
- * backend will queue each request and handle appropriate.
+ * backend will queue each request and handle appropriately.
*
* The backend does not need to pass the full amount of data to @receive_entropy
- * but will pass at a value greater than 0.
+ * but will pass a a value greater than 0.
*/
void rng_backend_request_entropy(RngBackend *s, size_t size,
EntropyReceiveFunc *receive_entropy,
@@ -87,7 +87,7 @@ void rng_backend_cancel_requests(RngBackend *s);
*
* This function will open the backend if it is not already open. Calling this
* function on an already opened backend will not result in an error.
- */
+ */
void rng_backend_open(RngBackend *s, Error **errp);
#endif
--
1.8.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy
2012-11-20 14:26 ` [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy Amit Shah
@ 2012-11-21 1:41 ` Anthony Liguori
0 siblings, 0 replies; 11+ messages in thread
From: Anthony Liguori @ 2012-11-21 1:41 UTC (permalink / raw)
To: Amit Shah; +Cc: Paolo Bonzini, qemu list
Amit Shah <amit.shah@redhat.com> writes:
> If we got fewer bytes from the backend than requested, don't poke the
> backend for more bytes; the guest will ask for more (or if the guest has
> already asked for more, the backend knows about it via handle_input()).
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> hw/virtio-rng.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
> index 6c49bb2..cf5a8ff 100644
> --- a/hw/virtio-rng.c
> +++ b/hw/virtio-rng.c
> @@ -80,13 +80,6 @@ static void chr_read(void *opaque, const void *buf, size_t size)
> virtqueue_push(vrng->vq, &elem, len);
> }
> virtio_notify(&vrng->vdev, vrng->vq);
> -
> - /*
> - * Lastly, if we had multiple elems queued by the guest, and we
> - * didn't have enough data to fill them all, indicate we want more
> - * data.
> - */
> - virtio_rng_process(vrng);
> }
>
> static void virtio_rng_process(VirtIORNG *vrng)
> --
> 1.8.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal
2012-11-20 14:26 ` [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal Amit Shah
@ 2012-11-21 1:41 ` Anthony Liguori
0 siblings, 0 replies; 11+ messages in thread
From: Anthony Liguori @ 2012-11-21 1:41 UTC (permalink / raw)
To: Amit Shah; +Cc: Paolo Bonzini, qemu list
Amit Shah <amit.shah@redhat.com> writes:
> Disable the rate-limit timer on device remove (e.g. hot-unplug).
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
Nice catch.
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> hw/virtio-rng.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
> index cf5a8ff..c8a6da7 100644
> --- a/hw/virtio-rng.c
> +++ b/hw/virtio-rng.c
> @@ -194,6 +194,8 @@ void virtio_rng_exit(VirtIODevice *vdev)
> {
> VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
>
> + qemu_del_timer(vrng->rate_limit_timer);
> + qemu_free_timer(vrng->rate_limit_timer);
> unregister_savevm(vrng->qdev, "virtio-rng", vrng);
> virtio_cleanup(vdev);
> }
> --
> 1.8.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments
2012-11-20 14:26 ` [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments Amit Shah
@ 2012-11-21 1:43 ` Anthony Liguori
2012-11-21 5:51 ` Amit Shah
0 siblings, 1 reply; 11+ messages in thread
From: Anthony Liguori @ 2012-11-21 1:43 UTC (permalink / raw)
To: Amit Shah; +Cc: Paolo Bonzini, qemu list
Amit Shah <amit.shah@redhat.com> writes:
> Fix typos, whitespace and update comments to match current
> implementation.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> hw/virtio-rng.c | 7 +++----
> include/qemu/rng.h | 6 +++---
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
> index c8a6da7..f4ed9cf 100644
> --- a/hw/virtio-rng.c
> +++ b/hw/virtio-rng.c
> @@ -23,7 +23,6 @@ typedef struct VirtIORNG {
> /* Only one vq - guest puts buffer(s) on it when it needs entropy */
> VirtQueue *vq;
>
> - /* Config data for the device -- currently only chardev */
> VirtIORNGConf *conf;
>
> RngBackend *rng;
> @@ -125,9 +124,9 @@ static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
> virtio_load(&vrng->vdev, f);
>
> /* 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.
> - */
> + * limit. Make sure to try again after live migration when the quota may
> + * have been reset.
> + */
> virtio_rng_process(vrng);
>
> return 0;
> diff --git a/include/qemu/rng.h b/include/qemu/rng.h
> index 7e9d672..9836463 100644
> --- a/include/qemu/rng.h
> +++ b/include/qemu/rng.h
> @@ -61,10 +61,10 @@ struct RngBackend
> * This function is used by the front-end to request entropy from an entropy
> * source. This function can be called multiple times before @receive_entropy
> * is invoked with different values of @receive_entropy and @opaque. The
> - * backend will queue each request and handle appropriate.
> + * backend will queue each request and handle appropriately.
> *
> * The backend does not need to pass the full amount of data to @receive_entropy
> - * but will pass at a value greater than 0.
> + * but will pass a a value greater than 0.
Your typo fix has a typo.
Regards,
Anthony Liguori
> */
> void rng_backend_request_entropy(RngBackend *s, size_t size,
> EntropyReceiveFunc *receive_entropy,
> @@ -87,7 +87,7 @@ void rng_backend_cancel_requests(RngBackend *s);
> *
> * This function will open the backend if it is not already open. Calling this
> * function on an already opened backend will not result in an error.
> - */
> + */
> void rng_backend_open(RngBackend *s, Error **errp);
>
> #endif
> --
> 1.8.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration
2012-11-20 14:26 ` [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration Amit Shah
@ 2012-11-21 1:44 ` Anthony Liguori
2012-11-21 5:53 ` Amit Shah
0 siblings, 1 reply; 11+ messages in thread
From: Anthony Liguori @ 2012-11-21 1:44 UTC (permalink / raw)
To: Amit Shah; +Cc: Paolo Bonzini, qemu list
Amit Shah <amit.shah@redhat.com> 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 <amit.shah@redhat.com>
To be clear, migration is already endian safe but this is definitely a
very nice cleanup.
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments
2012-11-21 1:43 ` Anthony Liguori
@ 2012-11-21 5:51 ` Amit Shah
0 siblings, 0 replies; 11+ messages in thread
From: Amit Shah @ 2012-11-21 5:51 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu list
On (Tue) 20 Nov 2012 [19:43:16], Anthony Liguori wrote:
> > * The backend does not need to pass the full amount of data to @receive_entropy
> > - * but will pass at a value greater than 0.
> > + * but will pass a a value greater than 0.
>
> Your typo fix has a typo.
Woops, series re-sent.
Amit
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration
2012-11-21 1:44 ` Anthony Liguori
@ 2012-11-21 5:53 ` Amit Shah
0 siblings, 0 replies; 11+ messages in thread
From: Amit Shah @ 2012-11-21 5:53 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu list
On (Tue) 20 Nov 2012 [19:44:55], Anthony Liguori wrote:
> Amit Shah <amit.shah@redhat.com> 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 <amit.shah@redhat.com>
>
> To be clear, migration is already endian safe but this is definitely a
> very nice cleanup.
The in_addr and out_addr fields have pointers -- so endian- plus
word-length safety isn't guaranteed if migrating across architectures.
Amit
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-11-21 5:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-20 14:26 [Qemu-devel] [PATCH 0/4] [For 0.13] virtio-rng: Fixes Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 1/4] virtio-rng: use virtqueue_get_avail_bytes, fix migration Amit Shah
2012-11-21 1:44 ` Anthony Liguori
2012-11-21 5:53 ` Amit Shah
2012-11-20 14:26 ` [Qemu-devel] [PATCH 2/4] virtio-rng: remove extra request for entropy Amit Shah
2012-11-21 1:41 ` Anthony Liguori
2012-11-20 14:26 ` [Qemu-devel] [PATCH 3/4] virtio-rng: disable timer on device removal Amit Shah
2012-11-21 1:41 ` Anthony Liguori
2012-11-20 14:26 ` [Qemu-devel] [PATCH 4/4] virtio-rng: fix typos, comments Amit Shah
2012-11-21 1:43 ` Anthony Liguori
2012-11-21 5:51 ` Amit Shah
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).