When looking for a dirty bitmap to share, we should handle filters by
just including them in the search (so they do not break backing chains).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 nbd/server.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index aeca3893fe..0d51d46b81 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1508,13 +1508,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
     if (bitmap) {
         BdrvDirtyBitmap *bm = NULL;
 
-        while (true) {
+        while (bs) {
             bm = bdrv_find_dirty_bitmap(bs, bitmap);
-            if (bm != NULL || bs->backing == NULL) {
+            if (bm != NULL) {
                 break;
             }
 
-            bs = bs->backing->bs;
+            bs = bdrv_filtered_bs(bs);
         }
 
         if (bm == NULL) {
-- 
2.21.0

13.06.2019 1:09, Max Reitz wrote:
> When looking for a dirty bitmap to share, we should handle filters by
> just including them in the search (so they do not break backing chains).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   nbd/server.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index aeca3893fe..0d51d46b81 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1508,13 +1508,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>       if (bitmap) {
>           BdrvDirtyBitmap *bm = NULL;
>   
> -        while (true) {
> +        while (bs) {
>               bm = bdrv_find_dirty_bitmap(bs, bitmap);
> -            if (bm != NULL || bs->backing == NULL) {
> +            if (bm != NULL) {
>                   break;
>               }
>   
> -            bs = bs->backing->bs;
> +            bs = bdrv_filtered_bs(bs);
>           }
>   
>           if (bm == NULL) {
> 

Hmm, I'm a bit confused by the fact that we reuse bs for the other purpose (it was my idea, but bad one),
it seems safe here, as the only following usage of bs seems want entirely bs containing the bitmap, so it's
OK.. It may be worth adding near "BdrvDirtyBitmap *bm" and additional "BlockDriverState *bm_bs = bs" and
operate on it.


Anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>



-- 
Best regards,
Vladimir

On 6/12/19 5:09 PM, Max Reitz wrote:
> When looking for a dirty bitmap to share, we should handle filters by
> just including them in the search (so they do not break backing chains).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  nbd/server.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/nbd/server.c b/nbd/server.c
> index aeca3893fe..0d51d46b81 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1508,13 +1508,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>      if (bitmap) {
>          BdrvDirtyBitmap *bm = NULL;
>  
> -        while (true) {
> +        while (bs) {
>              bm = bdrv_find_dirty_bitmap(bs, bitmap);
> -            if (bm != NULL || bs->backing == NULL) {
> +            if (bm != NULL) {
>                  break;
>              }
>  
> -            bs = bs->backing->bs;
> +            bs = bdrv_filtered_bs(bs);
>          }
>  
>          if (bm == NULL) {
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org