[Qemu-devel] [PATCH] block: Consider all child nodes in bdrv_requests_pending()

[Qemu-devel] [PATCH] block: Consider all child nodes in bdrv_requests_pending()

[Kevin Wolf]

The function manually recursed into bs->file and bs->backing to check
whether there were any requests pending, but it ignored other children.

There's no need to special case file and backing here, so just replace
these two explicit recursions by a loop recursing for all child nodes.

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/io.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/block/io.c b/block/io.c
index 5ac6256..8dcad3b 100644
--- a/block/io.c
+++ b/block/io.c
@@ -216,6 +216,8 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
 /* Check if any requests are in-flight (including throttled requests) */
 bool bdrv_requests_pending(BlockDriverState *bs)
 {
+    BdrvChild *child;
+
     if (!QLIST_EMPTY(&bs->tracked_requests)) {
         return true;
     }
@@ -225,12 +227,13 @@ bool bdrv_requests_pending(BlockDriverState *bs)
     if (!qemu_co_queue_empty(&bs->throttled_reqs[1])) {
         return true;
     }
-    if (bs->file && bdrv_requests_pending(bs->file->bs)) {
-        return true;
-    }
-    if (bs->backing && bdrv_requests_pending(bs->backing->bs)) {
-        return true;
+
+    QLIST_FOREACH(child, &bs->children, next) {
+        if (bdrv_requests_pending(child->bs)) {
+            return true;
+        }
     }
+
     return false;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-block] [PATCH] block: Consider all child nodes in bdrv_requests_pending()

[Alberto Garcia]

On Wed 28 Oct 2015 11:46:51 AM CET, Kevin Wolf wrote:
> The function manually recursed into bs->file and bs->backing to check
> whether there were any requests pending, but it ignored other children.
>
> There's no need to special case file and backing here, so just replace
> these two explicit recursions by a loop recursing for all child nodes.
>
> Reported-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH] block: Consider all child nodes in bdrv_requests_pending()

[Jeff Cody]

On Wed, Oct 28, 2015 at 11:46:51AM +0100, Kevin Wolf wrote:
> The function manually recursed into bs->file and bs->backing to check
> whether there were any requests pending, but it ignored other children.
> 
> There's no need to special case file and backing here, so just replace
> these two explicit recursions by a loop recursing for all child nodes.
> 
> Reported-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/io.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 5ac6256..8dcad3b 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -216,6 +216,8 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
>  /* Check if any requests are in-flight (including throttled requests) */
>  bool bdrv_requests_pending(BlockDriverState *bs)
>  {
> +    BdrvChild *child;
> +
>      if (!QLIST_EMPTY(&bs->tracked_requests)) {
>          return true;
>      }
> @@ -225,12 +227,13 @@ bool bdrv_requests_pending(BlockDriverState *bs)
>      if (!qemu_co_queue_empty(&bs->throttled_reqs[1])) {
>          return true;
>      }
> -    if (bs->file && bdrv_requests_pending(bs->file->bs)) {
> -        return true;
> -    }
> -    if (bs->backing && bdrv_requests_pending(bs->backing->bs)) {
> -        return true;
> +
> +    QLIST_FOREACH(child, &bs->children, next) {
> +        if (bdrv_requests_pending(child->bs)) {
> +            return true;
> +        }
>      }
> +
>      return false;
>  }
>  
> -- 
> 1.8.3.1
> 
> 

Reviewed-by: Jeff Cody <jcody@redhat.com>

Re: [Qemu-devel] [PATCH] block: Consider all child nodes in bdrv_requests_pending()

[Stefan Hajnoczi]

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

On Wed, Oct 28, 2015 at 11:46:51AM +0100, Kevin Wolf wrote:
> The function manually recursed into bs->file and bs->backing to check
> whether there were any requests pending, but it ignored other children.
> 
> There's no need to special case file and backing here, so just replace
> these two explicit recursions by a loop recursing for all child nodes.
> 
> Reported-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/io.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 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 --]