[Qemu-devel] [PATCH] virtio-blk: Move resetting of req->mr_next to virtio_blk_handle_rw_error

[Qemu-devel] [PATCH] virtio-blk: Move resetting of req->mr_next to virtio_blk_handle_rw_error

[Fam Zheng]

"werror=report" would free the req in virtio_blk_handle_rw_error, we
mustn't write to it in that case.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/virtio-blk.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 848f3fe..756ae5c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -72,6 +72,9 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     VirtIOBlock *s = req->dev;
 
     if (action == BLOCK_ERROR_ACTION_STOP) {
+        /* Break the link as the next request is going to be parsed from the
+         * ring again. Otherwise we may end up doing a double completion! */
+        req->mr_next = NULL;
         req->next = s->rq;
         s->rq = req;
     } else if (action == BLOCK_ERROR_ACTION_REPORT) {
@@ -112,10 +115,6 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
              * happen on the other side of the migration).
              */
             if (virtio_blk_handle_rw_error(req, -ret, is_read)) {
-                /* Break the link in case the next request is added to the
-                 * restart queue and is going to be parsed from the ring again.
-                 */
-                req->mr_next = NULL;
                 continue;
             }
         }
-- 
2.4.3

Re: [Qemu-devel] [PATCH] virtio-blk: Move resetting of req->mr_next to virtio_blk_handle_rw_error

[Paolo Bonzini]

On 23/11/2015 01:41, Fam Zheng wrote:
> "werror=report" would free the req in virtio_blk_handle_rw_error, we
> mustn't write to it in that case.
> 
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/block/virtio-blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 848f3fe..756ae5c 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -72,6 +72,9 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
>      VirtIOBlock *s = req->dev;
>  
>      if (action == BLOCK_ERROR_ACTION_STOP) {
> +        /* Break the link as the next request is going to be parsed from the
> +         * ring again. Otherwise we may end up doing a double completion! */
> +        req->mr_next = NULL;
>          req->next = s->rq;
>          s->rq = req;
>      } else if (action == BLOCK_ERROR_ACTION_REPORT) {
> @@ -112,10 +115,6 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
>               * happen on the other side of the migration).
>               */
>              if (virtio_blk_handle_rw_error(req, -ret, is_read)) {
> -                /* Break the link in case the next request is added to the
> -                 * restart queue and is going to be parsed from the ring again.
> -                 */
> -                req->mr_next = NULL;
>                  continue;
>              }
>          }
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH] virtio-blk: Move resetting of req->mr_next to virtio_blk_handle_rw_error

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

On Mon, Nov 23, 2015 at 08:41:20AM +0800, Fam Zheng wrote:
> "werror=report" would free the req in virtio_blk_handle_rw_error, we
> mustn't write to it in that case.
> 
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/block/virtio-blk.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]