Re: [Qemu-block] [PATCH 2/4] error: Remove NULL checks on error_propagate() calls

[Laurent Vivier]

On 23/03/2018 21:50, Eric Blake wrote:
> On 03/22/2018 11:12 AM, Laurent Vivier wrote:
>> Re-run Coccinelle patch
>> scripts/coccinelle/error_propagate_null.cocci
>>
>> Signed-off-by: Laurent Vivier 
>> ---
>>   io/channel-websock.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
> 
> Misses an offender in numa.c, why?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06382.html
> 

No, error_report_err() doesn't check if  err is NULL, we must check
before calling it.

Thanks,
Laurent

Re: [Qemu-block] [PATCH] block: remove bdrv_dirty_bitmap_make_anon

[Vladimir Sementsov-Ogievskiy]

23.03.2018 19:42, Paolo Bonzini wrote:

All this function is doing will be repeated by
bdrv_do_release_matching_dirty_bitmap_locked, except
resetting bm->persistent.  But even that does not matter
because the bitmap will be freed.

Signed-off-by: Paolo Bonzini 


Reviewed-by: Vladimir Sementsov-Ogievskiy 


--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH for-2.12 v2 00/12] block: Follow-up for .bdrv_co_create (part 1)

[Kevin Wolf]

Am 21.03.2018 um 18:37 hat Kevin Wolf geschrieben:
> This series adds qemu-iotests for a few more block drivers (yet more to
> come in another series) and fixes a few things that previous review and
> these tests brought up.
> 
> The only major design change is that I converted the vdi block driver
> from a boolean 'static' create option to the standard 'preallocation'
> one that other drivers are using. This seems like a good move to make
> while the interface isn't stable yet.
> 
> v2:
> - Patch 1: Mention allowed values for 'preallocation' [Eric]
> - Patch 3: Fixed comments, removed 2^63-512 case [Eric]
> - Patch 6: Added missing reference output change [Eric]
> - Patch 7: s/UINT64_MAX/INT64_MAX/ [Eric]
> - Patches 8 and 12: Fixed comments [Eric]

Applied to the block branch.

Kevin

Re: [Qemu-block] [PATCH] block: simplify code around releasing bitmaps

[Vladimir Sementsov-Ogievskiy]

23.03.2018 19:42, Paolo Bonzini wrote:

QLIST_REMOVE does not require walking the list, and once the "bitmap"
argument is removed from bdrv_do_release_matching_dirty_bitmap_locked
the code simplifies a lot and it is worth inlining everything in the
callers of bdrv_do_release_matching_dirty_bitmap.

Signed-off-by: Paolo Bonzini 
---
  block/dirty-bitmap.c | 81 +++-
  1 file changed, 30 insertions(+), 51 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 13bb5066a4..c0fb49bf7b 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -250,48 +250,15 @@ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap 
*bitmap)
  }
  
  /* Called within bdrv_dirty_bitmap_lock..unlock */


Worth adding "Called with BQL taken" to the comment?


-static void bdrv_do_release_matching_dirty_bitmap_locked(
-BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
-bool (*cond)(BdrvDirtyBitmap *bitmap))
+static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
  {
-BdrvDirtyBitmap *bm, *next;
-
-QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
-if ((!bitmap || bm == bitmap) && (!cond || cond(bm))) {
-assert(!bm->active_iterators);
-assert(!bdrv_dirty_bitmap_frozen(bm));
-assert(!bm->meta);
-QLIST_REMOVE(bm, list);
-hbitmap_free(bm->bitmap);
-g_free(bm->name);
-g_free(bm);
-
-if (bitmap) {
-return;
-}
-}
-}
-
-if (bitmap) {
-abort();
-}
-}
-
-/* Called with BQL taken.  */
-static void bdrv_do_release_matching_dirty_bitmap(
-BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
-bool (*cond)(BdrvDirtyBitmap *bitmap))
-{
-bdrv_dirty_bitmaps_lock(bs);
-bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, cond);
-bdrv_dirty_bitmaps_unlock(bs);
-}
-
-/* Called within bdrv_dirty_bitmap_lock..unlock */
-static void bdrv_release_dirty_bitmap_locked(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap)
-{
-bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, NULL);
+assert(!bitmap->active_iterators);
+assert(!bdrv_dirty_bitmap_frozen(bitmap));
+assert(!bitmap->meta);
+QLIST_REMOVE(bitmap, list);
+hbitmap_free(bitmap->bitmap);
+g_free(bitmap->name);
+g_free(bitmap);
  }
  
  /**

@@ -344,7 +311,7 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
  error_setg(errp, "Merging of parent and successor bitmap failed");
  return NULL;
  }
-bdrv_release_dirty_bitmap_locked(bs, successor);
+bdrv_release_dirty_bitmap_locked(successor);
  parent->successor = NULL;
  
  return parent;

@@ -382,15 +349,12 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, 
int64_t bytes)
  bdrv_dirty_bitmaps_unlock(bs);
  }
  
-static bool bdrv_dirty_bitmap_has_name(BdrvDirtyBitmap *bitmap)

-{
-return !!bdrv_dirty_bitmap_name(bitmap);
-}
-
  /* Called with BQL taken.  */
  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
  {
-bdrv_do_release_matching_dirty_bitmap(bs, bitmap, NULL);
+bdrv_dirty_bitmaps_lock(bs);
+bdrv_release_dirty_bitmap_locked(bitmap);
+bdrv_dirty_bitmaps_unlock(bs);
  }
  
  /**

@@ -401,7 +365,15 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
   */
  void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
  {
-bdrv_do_release_matching_dirty_bitmap(bs, NULL, 
bdrv_dirty_bitmap_has_name);
+BdrvDirtyBitmap *bm, *next;
+
+bdrv_dirty_bitmaps_lock(bs);
+QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+if (bdrv_dirty_bitmap_name(bm)) {
+bdrv_release_dirty_bitmap_locked(bm);
+}
+}
+bdrv_dirty_bitmaps_unlock(bs);
  }
  
  /**

@@ -412,8 +384,15 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
   */


worth adding "Called with BQL taken." to the comment? 
bdrv_release_named_dirty_bitmaps functions has it.



  void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs)
  {
-bdrv_do_release_matching_dirty_bitmap(bs, NULL,
-  bdrv_dirty_bitmap_get_persistance);
+BdrvDirtyBitmap *bm, *next;
+
+bdrv_dirty_bitmaps_lock(bs);
+QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+if (bdrv_dirty_bitmap_get_persistance(bm)) {
+bdrv_release_dirty_bitmap_locked(bm);
+}
+}
+bdrv_dirty_bitmaps_unlock(bs);
  }
  
  /**


anyway,

Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH v2 1/1] iotests: fix test case 185

[Kevin Wolf]

Am 23.03.2018 um 04:43 hat QingFeng Hao geschrieben:
> Test case 185 failed since commit 4486e89c219 --- "vl: introduce 
> vm_shutdown()".
> It's because of the newly introduced function vm_shutdown calls 
> bdrv_drain_all,
> which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs
> that doubles the speed and offset is doubled.
> Some jobs' status are changed as well.
> 
> The fix is to not resume the jobs that are already yielded and also change
> 185.out accordingly.
> 
> Suggested-by: Stefan Hajnoczi 
> Signed-off-by: QingFeng Hao 

Stefan already commented on the fix itself, but I want to add two more
points:

Please change your subject line. "iotests: fix test case 185" means that
you are fixing the test case, not qemu code that makes the test case
fail. The subject line should describe the actual change. In all
likelihood it will start with "blockjob:" rather than "iotests:".

> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index fc645dac68..f8f208bbcf 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -99,6 +99,11 @@ typedef struct BlockJob {
>  bool ready;
>  
>  /**
> + * Set to true when the job is yielded.
> + */
> +bool yielded;

This is the same as !busy, so we don't need a new field for this.

Kevin

Re: [Qemu-block] [PATCH] block: simplify code around releasing bitmaps

[Paolo Bonzini]

On 26/03/2018 12:24, Vladimir Sementsov-Ogievskiy wrote:
> 23.03.2018 19:42, Paolo Bonzini wrote:
>> QLIST_REMOVE does not require walking the list, and once the "bitmap"
>> argument is removed from bdrv_do_release_matching_dirty_bitmap_locked
>> the code simplifies a lot and it is worth inlining everything in the
>> callers of bdrv_do_release_matching_dirty_bitmap.
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>   block/dirty-bitmap.c | 81
>> +++-
>>   1 file changed, 30 insertions(+), 51 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 13bb5066a4..c0fb49bf7b 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -250,48 +250,15 @@ void
>> bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
>>   }
>>     /* Called within bdrv_dirty_bitmap_lock..unlock */
> 
> Worth adding "Called with BQL taken" to the comment?

Yes, good idea.

>> +static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)

> worth adding "Called with BQL taken." to the comment?
> bdrv_release_named_dirty_bitmaps functions has it.
> 
>>   void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs)

This is pre-existing, but it's also a good idea.

Paolo

>> -    bdrv_do_release_matching_dirty_bitmap(bs, NULL,
>> - 
>> bdrv_dirty_bitmap_get_persistance);
>> +    BdrvDirtyBitmap *bm, *next;
>> +
>> +    bdrv_dirty_bitmaps_lock(bs);
>> +    QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
>> +    if (bdrv_dirty_bitmap_get_persistance(bm)) {
>> +    bdrv_release_dirty_bitmap_locked(bm);
>> +    }
>> +    }
>> +    bdrv_dirty_bitmaps_unlock(bs);
>>   }
>>     /**
> 
> anyway,
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> 

[Qemu-block] [PATCH v2] block: simplify code around releasing bitmaps

[Paolo Bonzini]

QLIST_REMOVE does not require walking the list, and once the "bitmap"
argument is removed from bdrv_do_release_matching_dirty_bitmap_locked
the code simplifies a lot and it is worth inlining everything in the
callers of bdrv_do_release_matching_dirty_bitmap.

Reviewed-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Paolo Bonzini 
---

Notes:
v1->v2: improve locking comments [Vladimir]

 block/dirty-bitmap.c | 84 
 1 file changed, 32 insertions(+), 52 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index ea82c06f07..a7eaa1051d 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -249,49 +249,16 @@ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap 
*bitmap)
 qemu_mutex_unlock(bitmap->mutex);
 }
 
-/* Called within bdrv_dirty_bitmap_lock..unlock */
-static void bdrv_do_release_matching_dirty_bitmap_locked(
-BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
-bool (*cond)(BdrvDirtyBitmap *bitmap))
+/* Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken.  */
+static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
 {
-BdrvDirtyBitmap *bm, *next;
-
-QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
-if ((!bitmap || bm == bitmap) && (!cond || cond(bm))) {
-assert(!bm->active_iterators);
-assert(!bdrv_dirty_bitmap_frozen(bm));
-assert(!bm->meta);
-QLIST_REMOVE(bm, list);
-hbitmap_free(bm->bitmap);
-g_free(bm->name);
-g_free(bm);
-
-if (bitmap) {
-return;
-}
-}
-}
-
-if (bitmap) {
-abort();
-}
-}
-
-/* Called with BQL taken.  */
-static void bdrv_do_release_matching_dirty_bitmap(
-BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
-bool (*cond)(BdrvDirtyBitmap *bitmap))
-{
-bdrv_dirty_bitmaps_lock(bs);
-bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, cond);
-bdrv_dirty_bitmaps_unlock(bs);
-}
-
-/* Called within bdrv_dirty_bitmap_lock..unlock */
-static void bdrv_release_dirty_bitmap_locked(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap)
-{
-bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, NULL);
+assert(!bitmap->active_iterators);
+assert(!bdrv_dirty_bitmap_frozen(bitmap));
+assert(!bitmap->meta);
+QLIST_REMOVE(bitmap, list);
+hbitmap_free(bitmap->bitmap);
+g_free(bitmap->name);
+g_free(bitmap);
 }
 
 /**
@@ -344,7 +311,7 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
 error_setg(errp, "Merging of parent and successor bitmap failed");
 return NULL;
 }
-bdrv_release_dirty_bitmap_locked(bs, successor);
+bdrv_release_dirty_bitmap_locked(successor);
 parent->successor = NULL;
 
 return parent;
@@ -382,15 +349,12 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, 
int64_t bytes)
 bdrv_dirty_bitmaps_unlock(bs);
 }
 
-static bool bdrv_dirty_bitmap_has_name(BdrvDirtyBitmap *bitmap)
-{
-return !!bdrv_dirty_bitmap_name(bitmap);
-}
-
 /* Called with BQL taken.  */
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
-bdrv_do_release_matching_dirty_bitmap(bs, bitmap, NULL);
+bdrv_dirty_bitmaps_lock(bs);
+bdrv_release_dirty_bitmap_locked(bitmap);
+bdrv_dirty_bitmaps_unlock(bs);
 }
 
 /**
@@ -401,7 +365,15 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
  */
 void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
 {
-bdrv_do_release_matching_dirty_bitmap(bs, NULL, 
bdrv_dirty_bitmap_has_name);
+BdrvDirtyBitmap *bm, *next;
+
+bdrv_dirty_bitmaps_lock(bs);
+QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+if (bdrv_dirty_bitmap_name(bm)) {
+bdrv_release_dirty_bitmap_locked(bm);
+}
+}
+bdrv_dirty_bitmaps_unlock(bs);
 }
 
 /**
@@ -409,11 +381,19 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState 
*bs)
  * bdrv_inactivate_recurse()).
  * There must not be any frozen bitmaps attached.
  * This function does not remove persistent bitmaps from the storage.
+ * Called with BQL taken.
  */
 void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs)
 {
-bdrv_do_release_matching_dirty_bitmap(bs, NULL,
-  bdrv_dirty_bitmap_get_persistance);
+BdrvDirtyBitmap *bm, *next;
+
+bdrv_dirty_bitmaps_lock(bs);
+QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+if (bdrv_dirty_bitmap_get_persistance(bm)) {
+bdrv_release_dirty_bitmap_locked(bm);
+}
+}
+bdrv_dirty_bitmaps_unlock(bs);
 }
 
 /**
-- 
2.16.2

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12] qemu-pr-helper: Actually allow users to specify pidfile

[Paolo Bonzini]

On 24/03/2018 12:46, Eric Blake wrote:
> On 03/24/2018 12:14 AM, Michal Privoznik wrote:
>> Due to wrong specification of arguments to getopt_long() any
>> attempt to set pidfile resulted in:
>>
>> 1) the default to be leaked
>> 2) the @pidfile variable to be set to NULL (because optarg is
>> NULL without this patch).
> 
> Broken since the introduction in commit b855f8d, in 2.11 (documentation
> has always mentioned the argument, code has never supported it).

Queued, thanks.

Paolo

>>
>> Signed-off-by: Michal Privoznik 
> 
> CC: qemu-sta...@nongnu.org
> Reviewed-by: Eric Blake 
> 

[Qemu-block] [PATCH 7/7] blockdev: unify block-dirty-bitmap-clear command and transaction action

[Vladimir Sementsov-Ogievskiy]

 - use error messages from qmp command, as they are more descriptive
 - we need not check bs, as block_dirty_bitmap_lookup never returns
   bs = NULL on success (and if user set bs to be not NULL pointer),
   so, let's just assert it.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c | 53 +
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 54e1ba8f44..3f0979298e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2114,18 +2114,26 @@ static void 
block_dirty_bitmap_clear_prepare(BlkActionState *common,
 if (!state->bitmap) {
 return;
 }
+assert(state->bs != NULL);
 
 if (bdrv_dirty_bitmap_frozen(state->bitmap)) {
-error_setg(errp, "Cannot modify a frozen bitmap");
+error_setg(errp,
+   "Bitmap '%s' is currently frozen and cannot be modified",
+   action->name);
 return;
 } else if (bdrv_dirty_bitmap_qmp_locked(state->bitmap)) {
-error_setg(errp, "Cannot modify a locked bitmap");
+error_setg(errp,
+   "Bitmap '%s' is currently locked and cannot be modified",
+   action->name);
 return;
 } else if (!bdrv_dirty_bitmap_enabled(state->bitmap)) {
-error_setg(errp, "Cannot clear a disabled bitmap");
+error_setg(errp,
+   "Bitmap '%s' is currently disabled and cannot be cleared",
+   action->name);
 return;
 } else if (bdrv_dirty_bitmap_readonly(state->bitmap)) {
-error_setg(errp, "Cannot clear a readonly bitmap");
+error_setg(errp, "Bitmap '%s' is readonly and cannot be cleared",
+   action->name);
 return;
 }
 }
@@ -2878,35 +2886,16 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
const char *name,
 void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
   Error **errp)
 {
-BdrvDirtyBitmap *bitmap;
-BlockDriverState *bs;
-
-bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
-if (!bitmap || !bs) {
-return;
-}
-
-if (bdrv_dirty_bitmap_frozen(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently frozen and cannot be modified",
-   name);
-return;
-} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently locked and cannot be modified",
-   name);
-return;
-} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
-error_setg(errp,
-   "Bitmap '%s' is currently disabled and cannot be cleared",
-   name);
-return;
-} else if (bdrv_dirty_bitmap_readonly(bitmap)) {
-error_setg(errp, "Bitmap '%s' is readonly and cannot be cleared", 
name);
-return;
-}
+BlockDirtyBitmap data = {
+.node = (char *)node,
+.name = (char *)name
+};
+TransactionAction action = {
+.type = TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR,
+.u.block_dirty_bitmap_clear.data = &data,
+};
 
-bdrv_clear_dirty_bitmap(bitmap);
+blockdev_do_action(&action, errp);
 }
 
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
-- 
2.11.1

[Qemu-block] [PATCH 1/7] block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

Functions write to BdrvDirtyBitmap field, so the should take the lock.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/dirty-bitmap.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 967159479d..6c00288fd7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -445,15 +445,19 @@ void bdrv_remove_persistent_dirty_bitmap(BlockDriverState 
*bs,
 /* Called with BQL taken.  */
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
+bdrv_dirty_bitmap_lock(bitmap);
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 bitmap->disabled = true;
+bdrv_dirty_bitmap_unlock(bitmap);
 }
 
 /* Called with BQL taken.  */
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
+bdrv_dirty_bitmap_lock(bitmap);
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 bitmap->disabled = false;
+bdrv_dirty_bitmap_unlock(bitmap);
 }
 
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
-- 
2.11.1

[Qemu-block] [PATCH 6/7] block/dirty-bitmap: bdrv_clear_dirty_bitmap: drop unused parameter

[Vladimir Sementsov-Ogievskiy]

Drop parameter "HBitmap **out" which is unused now, all callers set
it to NULL.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block_int.h |  2 +-
 block/dirty-bitmap.c  | 14 +-
 blockdev.c|  4 ++--
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index e5af7820ff..b2ef0f472d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1078,7 +1078,7 @@ bool blk_dev_is_medium_locked(BlockBackend *blk);
 
 void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes);
 
-void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
 
 void bdrv_inc_in_flight(BlockDriverState *bs);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 1812e17549..139102f24e 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -591,19 +591,15 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
 bdrv_dirty_bitmap_unlock(bitmap);
 }
 
-void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
 assert(bdrv_dirty_bitmap_enabled(bitmap));
 assert(!bdrv_dirty_bitmap_readonly(bitmap));
+
 bdrv_dirty_bitmap_lock(bitmap);
-if (!out) {
-hbitmap_reset_all(bitmap->bitmap);
-} else {
-HBitmap *backup = bitmap->bitmap;
-bitmap->bitmap = hbitmap_alloc(bitmap->size,
-   hbitmap_granularity(backup));
-*out = backup;
-}
+
+hbitmap_reset_all(bitmap->bitmap);
+
 bdrv_dirty_bitmap_unlock(bitmap);
 }
 
diff --git a/blockdev.c b/blockdev.c
index 88eae60c1c..54e1ba8f44 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2135,7 +2135,7 @@ static void 
block_dirty_bitmap_clear_commit(BlkActionState *common)
 BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
  common, common);
 
-bdrv_clear_dirty_bitmap(state->bitmap, NULL);
+bdrv_clear_dirty_bitmap(state->bitmap);
 }
 
 static void abort_prepare(BlkActionState *common, Error **errp)
@@ -2906,7 +2906,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const 
char *name,
 return;
 }
 
-bdrv_clear_dirty_bitmap(bitmap, NULL);
+bdrv_clear_dirty_bitmap(bitmap);
 }
 
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
-- 
2.11.1

[Qemu-block] [PATCH 5/7] blockdev: refactor block-dirty-bitmap-clear transaction

[Vladimir Sementsov-Ogievskiy]

bdrv_clear_dirty_bitmap do not fail, so we can call it in transaction
commit, avoiding any rollback.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index c31bf3d98d..88eae60c1c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2050,7 +2050,6 @@ typedef struct BlockDirtyBitmapState {
 BlkActionState common;
 BdrvDirtyBitmap *bitmap;
 BlockDriverState *bs;
-HBitmap *backup;
 bool prepared;
 } BlockDirtyBitmapState;
 
@@ -2129,18 +2128,6 @@ static void 
block_dirty_bitmap_clear_prepare(BlkActionState *common,
 error_setg(errp, "Cannot clear a readonly bitmap");
 return;
 }
-
-bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
-}
-
-static void block_dirty_bitmap_clear_abort(BlkActionState *common)
-{
-BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
- common, common);
-
-if (state->backup) {
-bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
-}
 }
 
 static void block_dirty_bitmap_clear_commit(BlkActionState *common)
@@ -2148,7 +2135,7 @@ static void 
block_dirty_bitmap_clear_commit(BlkActionState *common)
 BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
  common, common);
 
-hbitmap_free(state->backup);
+bdrv_clear_dirty_bitmap(state->bitmap, NULL);
 }
 
 static void abort_prepare(BlkActionState *common, Error **errp)
@@ -2210,7 +2197,6 @@ static const BlkActionOps actions[] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_clear_prepare,
 .commit = block_dirty_bitmap_clear_commit,
-.abort = block_dirty_bitmap_clear_abort,
 }
 };
 
-- 
2.11.1

[Qemu-block] [PATCH 3/7] dirty-bitmap: remove missed bdrv_dirty_bitmap_get_autoload header

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/dirty-bitmap.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 1ff8949b1b..c7e910016d 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -88,7 +88,6 @@ int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
 bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
 bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
-bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap);
 bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
-- 
2.11.1

[Qemu-block] [PATCH 0/7] Dirty bitmaps fixing and refactoring

[Vladimir Sementsov-Ogievskiy]

Nothing critical here.

01 - these functions are unused for now, but the will be used in
 the qmp bitmaps api.

Vladimir Sementsov-Ogievskiy (7):
  block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap
  dirty-bitmaps: fix comment about dirty_bitmap_mutex
  dirty-bitmap: remove missed bdrv_dirty_bitmap_get_autoload header
  dirty-bitmap: separate unused meta-bitmap related functions
  blockdev: refactor block-dirty-bitmap-clear transaction
  block/dirty-bitmap: bdrv_clear_dirty_bitmap: drop unused parameter
  blockdev: unify block-dirty-bitmap-clear command and transaction
action

 include/block/block_int.h| 14 +
 include/block/dirty-bitmap.h | 15 ++
 block/dirty-bitmap.c | 23 +--
 blockdev.c   | 69 ++--
 4 files changed, 54 insertions(+), 67 deletions(-)

-- 
2.11.1

[Qemu-block] [PATCH 2/7] dirty-bitmaps: fix comment about dirty_bitmap_mutex

[Vladimir Sementsov-Ogievskiy]

Clarify first two cases and fix Modify -> Any access in third case.
Also, drop 'only' from third case, as it a bit confuses, when thinking
about case where we modify BdrvDirtyBitmap and access HBitmap.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block_int.h | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 27e17addba..e5af7820ff 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -701,10 +701,14 @@ struct BlockDriverState {
 uint64_t write_threshold_offset;
 NotifierWithReturn write_threshold_notifier;
 
-/* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
- * Reading from the list can be done with either the BQL or the
- * dirty_bitmap_mutex.  Modifying a bitmap only requires
- * dirty_bitmap_mutex.  */
+/* Writing to the list (i.e. to any field of BdrvDirtyBitmap or to the
+ * list-head) requires both the BQL _and_ the dirty_bitmap_mutex.
+ *
+ * Reading from the list (from any field of BdrvDirtyBitmap or from the
+ * list-head) can be done with either the BQL or the dirty_bitmap_mutex.
+ *
+ * Any access to underlying HBitmap requires dirty_bitmap_mutex.
+ */
 QemuMutex dirty_bitmap_mutex;
 QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
 
-- 
2.11.1

[Qemu-block] [PATCH 4/7] dirty-bitmap: separate unused meta-bitmap related functions

[Vladimir Sementsov-Ogievskiy]

Separate them in the header and clarify needed locking in comments.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/dirty-bitmap.h | 14 +-
 block/dirty-bitmap.c |  5 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index c7e910016d..b7ccfd1363 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -9,9 +9,6 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
   uint32_t granularity,
   const char *name,
   Error **errp);
-void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-   int chunk_size);
-void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
@@ -45,7 +42,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t offset, int64_t bytes);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
  int64_t offset, int64_t bytes);
-BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
 
@@ -84,7 +80,6 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
 int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
 void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
-int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
 bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
 bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
@@ -99,4 +94,13 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap,
   Error **errp);
 
+/*
+ * Unused for now meta-bitmaps related functions
+ */
+
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, int chunk_size);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
+int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
+
 #endif
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 6c00288fd7..1812e17549 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -149,6 +149,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState 
*bs,
  * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
  * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap
  * track.
+ *
+ * Called with BQL taken.
  */
 void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int chunk_size)
@@ -160,6 +162,7 @@ void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
 qemu_mutex_unlock(bitmap->mutex);
 }
 
+/* Called with BQL taken. */
 void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
 assert(bitmap->meta);
@@ -529,6 +532,7 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap 
*bitmap)
 return iter;
 }
 
+/* Called with BQL and dirty_bitmap_mutex locked. */
 BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap)
 {
 BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1);
@@ -688,6 +692,7 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
 return hbitmap_count(bitmap->bitmap);
 }
 
+/* Called with BQL or dirty_bitmap_mutex locked */
 int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->meta);
-- 
2.11.1

Re: [Qemu-block] [Qemu-devel] [PATCH 5/7] blockdev: refactor block-dirty-bitmap-clear transaction

[Paolo Bonzini]

On 26/03/2018 13:20, Vladimir Sementsov-Ogievskiy wrote:
> bdrv_clear_dirty_bitmap do not fail, so we can call it in transaction
> commit, avoiding any rollback.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  blockdev.c | 16 +---
>  1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index c31bf3d98d..88eae60c1c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2050,7 +2050,6 @@ typedef struct BlockDirtyBitmapState {
>  BlkActionState common;
>  BdrvDirtyBitmap *bitmap;
>  BlockDriverState *bs;
> -HBitmap *backup;
>  bool prepared;
>  } BlockDirtyBitmapState;
>  
> @@ -2129,18 +2128,6 @@ static void 
> block_dirty_bitmap_clear_prepare(BlkActionState *common,
>  error_setg(errp, "Cannot clear a readonly bitmap");
>  return;
>  }
> -
> -bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
> -}
> -
> -static void block_dirty_bitmap_clear_abort(BlkActionState *common)
> -{
> -BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> - common, common);
> -
> -if (state->backup) {
> -bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
> -}

Isn't bdrv_undo_clear_dirty_bitmap new unused?

Thanks,

Paolo

>  }
>  
>  static void block_dirty_bitmap_clear_commit(BlkActionState *common)
> @@ -2148,7 +2135,7 @@ static void 
> block_dirty_bitmap_clear_commit(BlkActionState *common)
>  BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
>   common, common);
>  
> -hbitmap_free(state->backup);
> +bdrv_clear_dirty_bitmap(state->bitmap, NULL);
>  }
>  
>  static void abort_prepare(BlkActionState *common, Error **errp)
> @@ -2210,7 +2197,6 @@ static const BlkActionOps actions[] = {
>  .instance_size = sizeof(BlockDirtyBitmapState),
>  .prepare = block_dirty_bitmap_clear_prepare,
>  .commit = block_dirty_bitmap_clear_commit,
> -.abort = block_dirty_bitmap_clear_abort,
>  }
>  };
>  
> 

Re: [Qemu-block] [Qemu-devel] [PATCH 0/7] Dirty bitmaps fixing and refactoring

[Paolo Bonzini]

On 26/03/2018 13:20, Vladimir Sementsov-Ogievskiy wrote:
> Nothing critical here.
> 
> 01 - these functions are unused for now, but the will be used in
>  the qmp bitmaps api.
> 
> Vladimir Sementsov-Ogievskiy (7):
>   block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap
>   dirty-bitmaps: fix comment about dirty_bitmap_mutex
>   dirty-bitmap: remove missed bdrv_dirty_bitmap_get_autoload header
>   dirty-bitmap: separate unused meta-bitmap related functions
>   blockdev: refactor block-dirty-bitmap-clear transaction
>   block/dirty-bitmap: bdrv_clear_dirty_bitmap: drop unused parameter
>   blockdev: unify block-dirty-bitmap-clear command and transaction
> action
> 
>  include/block/block_int.h| 14 +
>  include/block/dirty-bitmap.h | 15 ++
>  block/dirty-bitmap.c | 23 +--
>  blockdev.c   | 69 
> ++--
>  4 files changed, 54 insertions(+), 67 deletions(-)
> 

Looks good apart from some dead code still being there.

Paolo

Re: [Qemu-block] [Qemu-devel] [PATCH 5/7] blockdev: refactor block-dirty-bitmap-clear transaction

[Vladimir Sementsov-Ogievskiy]

26.03.2018 14:44, Paolo Bonzini wrote:

On 26/03/2018 13:20, Vladimir Sementsov-Ogievskiy wrote:

bdrv_clear_dirty_bitmap do not fail, so we can call it in transaction
commit, avoiding any rollback.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  blockdev.c | 16 +---
  1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index c31bf3d98d..88eae60c1c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2050,7 +2050,6 @@ typedef struct BlockDirtyBitmapState {
  BlkActionState common;
  BdrvDirtyBitmap *bitmap;
  BlockDriverState *bs;
-HBitmap *backup;
  bool prepared;
  } BlockDirtyBitmapState;
  
@@ -2129,18 +2128,6 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,

  error_setg(errp, "Cannot clear a readonly bitmap");
  return;
  }
-
-bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
-}
-
-static void block_dirty_bitmap_clear_abort(BlkActionState *common)
-{
-BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
- common, common);
-
-if (state->backup) {
-bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
-}

Isn't bdrv_undo_clear_dirty_bitmap new unused?


hmm, yes. I'll remove it in v2.



Thanks,

Paolo


  }
  
  static void block_dirty_bitmap_clear_commit(BlkActionState *common)

@@ -2148,7 +2135,7 @@ static void 
block_dirty_bitmap_clear_commit(BlkActionState *common)
  BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
   common, common);
  
-hbitmap_free(state->backup);

+bdrv_clear_dirty_bitmap(state->bitmap, NULL);
  }
  
  static void abort_prepare(BlkActionState *common, Error **errp)

@@ -2210,7 +2197,6 @@ static const BlkActionOps actions[] = {
  .instance_size = sizeof(BlockDirtyBitmapState),
  .prepare = block_dirty_bitmap_clear_prepare,
  .commit = block_dirty_bitmap_clear_commit,
-.abort = block_dirty_bitmap_clear_abort,
  }
  };
  




--
Best regards,
Vladimir

Re: [Qemu-block] [Qemu-devel] [PATCH 5/7] blockdev: refactor block-dirty-bitmap-clear transaction

[Vladimir Sementsov-Ogievskiy]

26.03.2018 14:48, Vladimir Sementsov-Ogievskiy wrote:

26.03.2018 14:44, Paolo Bonzini wrote:

On 26/03/2018 13:20, Vladimir Sementsov-Ogievskiy wrote:

bdrv_clear_dirty_bitmap do not fail, so we can call it in transaction
commit, avoiding any rollback.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  blockdev.c | 16 +---
  1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index c31bf3d98d..88eae60c1c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2050,7 +2050,6 @@ typedef struct BlockDirtyBitmapState {
  BlkActionState common;
  BdrvDirtyBitmap *bitmap;
  BlockDriverState *bs;
-    HBitmap *backup;
  bool prepared;
  } BlockDirtyBitmapState;
  @@ -2129,18 +2128,6 @@ static void 
block_dirty_bitmap_clear_prepare(BlkActionState *common,

  error_setg(errp, "Cannot clear a readonly bitmap");
  return;
  }
-
-    bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
-}
-
-static void block_dirty_bitmap_clear_abort(BlkActionState *common)
-{
-    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
- common, common);
-
-    if (state->backup) {
-    bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
-    }

Isn't bdrv_undo_clear_dirty_bitmap new unused?


hmm, yes. I'll remove it in v2.


or, here, as 5.5/7





Thanks,

Paolo


  }
    static void block_dirty_bitmap_clear_commit(BlkActionState *common)
@@ -2148,7 +2135,7 @@ static void 
block_dirty_bitmap_clear_commit(BlkActionState *common)

  BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
   common, common);
  -    hbitmap_free(state->backup);
+    bdrv_clear_dirty_bitmap(state->bitmap, NULL);
  }
    static void abort_prepare(BlkActionState *common, Error **errp)
@@ -2210,7 +2197,6 @@ static const BlkActionOps actions[] = {
  .instance_size = sizeof(BlockDirtyBitmapState),
  .prepare = block_dirty_bitmap_clear_prepare,
  .commit = block_dirty_bitmap_clear_commit,
-    .abort = block_dirty_bitmap_clear_abort,
  }
  };







--
Best regards,
Vladimir

[Qemu-block] [PATCH 5.5/7] dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block_int.h | 1 -
 block/dirty-bitmap.c  | 9 -
 2 files changed, 10 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index e5af7820ff..bd857d5099 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1079,7 +1079,6 @@ bool blk_dev_is_medium_locked(BlockBackend *blk);
 void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes);
 
 void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
-void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
 
 void bdrv_inc_in_flight(BlockDriverState *bs);
 void bdrv_dec_in_flight(BlockDriverState *bs);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 1812e17549..3c69a8eed9 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -607,15 +607,6 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, 
HBitmap **out)
 bdrv_dirty_bitmap_unlock(bitmap);
 }
 
-void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)
-{
-HBitmap *tmp = bitmap->bitmap;
-assert(bdrv_dirty_bitmap_enabled(bitmap));
-assert(!bdrv_dirty_bitmap_readonly(bitmap));
-bitmap->bitmap = in;
-hbitmap_free(tmp);
-}
-
 uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,
   uint64_t offset, uint64_t bytes)
 {
-- 
2.11.1

[Qemu-block] [PULL 03/19] block/quorum: Remove protocol-related fields

[Kevin Wolf]

From: Fabiano Rosas 

The quorum driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. quorum:) will now fail gracefully:

  $ qemu-img info quorum:foo
  qemu-img: Could not open 'quorum:foo': Unknown protocol 'quorum'

Signed-off-by: Fabiano Rosas 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
Signed-off-by: Kevin Wolf 
---
 block/quorum.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 14333c18aa..cfe484a945 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1098,11 +1098,10 @@ static void quorum_refresh_filename(BlockDriverState 
*bs, QDict *options)
 
 static BlockDriver bdrv_quorum = {
 .format_name= "quorum",
-.protocol_name  = "quorum",
 
 .instance_size  = sizeof(BDRVQuorumState),
 
-.bdrv_file_open = quorum_open,
+.bdrv_open  = quorum_open,
 .bdrv_close = quorum_close,
 .bdrv_refresh_filename  = quorum_refresh_filename,
 
-- 
2.13.6

[Qemu-block] [PULL 02/19] block/replication: Remove protocol_name field

[Kevin Wolf]

From: Fabiano Rosas 

The protocol_name field is used when selecting a driver via protocol
syntax (i.e. :). Drivers that are
only selected explicitly (e.g. driver=replication,mode=primary,...)
should not have a protocol_name.

This patch removes the protocol_name field from the brdv_replication
structure so that attempts to invoke this driver using protocol syntax
will fail gracefully:

  $ qemu-img info replication:foo
  qemu-img: Could not open 'replication:': Unknown protocol 'replication'

Buglink: https://bugs.launchpad.net/qemu/+bug/1726733
Signed-off-by: Fabiano Rosas 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 replication.h   | 1 -
 block/replication.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/replication.h b/replication.h
index 8faefe005f..4c8354de23 100644
--- a/replication.h
+++ b/replication.h
@@ -67,7 +67,6 @@ typedef struct ReplicationState ReplicationState;
  *
  * BlockDriver bdrv_replication = {
  * .format_name= "replication",
- * .protocol_name  = "replication",
  * .instance_size  = sizeof(BDRVReplicationState),
  *
  * .bdrv_open  = replication_open,
diff --git a/block/replication.c b/block/replication.c
index f98ef094b9..6c0c7186d9 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -703,7 +703,6 @@ static void replication_stop(ReplicationState *rs, bool 
failover, Error **errp)
 
 BlockDriver bdrv_replication = {
 .format_name= "replication",
-.protocol_name  = "replication",
 .instance_size  = sizeof(BDRVReplicationState),
 
 .bdrv_open  = replication_open,
-- 
2.13.6

[Qemu-block] [PULL 00/19] Block layer patches

[Kevin Wolf]

The following changes since commit 7b1db0908d88f0c9cfac24e214ff72a860692e23:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180323' 
into staging (2018-03-25 13:51:33 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 0b7e7f66813a7e346e12d47be977a32a530a6316:

  qemu-iotests: Test vhdx image creation with QMP (2018-03-26 12:17:43 +0200)


Block layer patches


Alberto Garcia (1):
  qcow2: Reset free_cluster_index when allocating a new refcount block

Eric Blake (1):
  iotests: 163 is not quick

Fabiano Rosas (5):
  block/replication: Remove protocol_name field
  block/quorum: Remove protocol-related fields
  block/throttle: Remove protocol-related fields
  block/blkreplay: Remove protocol-related fields
  include/block/block_int: Document protocol related functions

Kevin Wolf (12):
  vdi: Change 'static' create option to 'preallocation' in QMP
  vdi: Fix build with CONFIG_VDI_DEBUG
  qemu-iotests: Test vdi image creation with QMP
  qemu-iotests: Enable 025 for luks
  luks: Turn another invalid assertion into check
  qemu-iotests: Test invalid resize on luks
  parallels: Check maximum cluster size on create
  qemu-iotests: Test parallels image creation with QMP
  vhdx: Require power-of-two block size on create
  vhdx: Don't use error_setg_errno() with constant errno
  vhdx: Check for 4 GB maximum log size on creation
  qemu-iotests: Test vhdx image creation with QMP

 qapi/block-core.json   |   7 +-
 include/block/block_int.h  |   8 ++
 replication.h  |   1 -
 block/blkreplay.c  |   3 +-
 block/crypto.c |   6 +-
 block/parallels.c  |   5 +
 block/qcow2-refcount.c |   7 +
 block/quorum.c |   3 +-
 block/replication.c|   1 -
 block/throttle.c   |   3 +-
 block/vdi.c|  46 --
 block/vhdx.c   |  17 ++-
 tests/qemu-iotests/025 |   9 +-
 tests/qemu-iotests/026.out |   6 +-
 tests/qemu-iotests/121 |  20 +++
 tests/qemu-iotests/121.out |  10 ++
 tests/qemu-iotests/210 |  37 +
 tests/qemu-iotests/210.out |  16 +++
 tests/qemu-iotests/211 | 246 
 tests/qemu-iotests/211.out |  97 +
 tests/qemu-iotests/212 | 326 ++
 tests/qemu-iotests/212.out | 111 ++
 tests/qemu-iotests/213 | 349 +
 tests/qemu-iotests/213.out | 121 
 tests/qemu-iotests/group   |   5 +-
 25 files changed, 1423 insertions(+), 37 deletions(-)
 create mode 100755 tests/qemu-iotests/211
 create mode 100644 tests/qemu-iotests/211.out
 create mode 100755 tests/qemu-iotests/212
 create mode 100644 tests/qemu-iotests/212.out
 create mode 100755 tests/qemu-iotests/213
 create mode 100644 tests/qemu-iotests/213.out

[Qemu-block] [PULL 08/19] vdi: Change 'static' create option to 'preallocation' in QMP

[Kevin Wolf]

What static=on really does is what we call metadata preallocation for
other block drivers. While we can still change the QMP interface, make
it more consistent by using 'preallocation' for VDI, too.

This doesn't implement any new functionality, so the only supported
preallocation modes are 'off' and 'metadata' for now.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 qapi/block-core.json |  7 +++
 block/vdi.c  | 24 ++--
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1088ab0c78..c50517bff3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3943,16 +3943,15 @@
 #
 # @file Node to create the image format on
 # @size Size of the virtual disk in bytes
-# @static   Whether to create a statically (true) or
-#   dynamically (false) allocated image
-#   (default: false, i.e. dynamic)
+# @preallocationPreallocation mode for the new image (allowed values: off,
+#   metadata; default: off)
 #
 # Since: 2.12
 ##
 { 'struct': 'BlockdevCreateOptionsVdi',
   'data': { 'file': 'BlockdevRef',
 'size': 'size',
-'*static':  'bool' } }
+'*preallocation':   'PreallocMode' } }
 
 ##
 # @BlockdevVhdxSubformat:
diff --git a/block/vdi.c b/block/vdi.c
index d939b034c4..73c059e69d 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -728,7 +728,7 @@ static int coroutine_fn 
vdi_co_do_create(BlockdevCreateOptions *create_options,
 int ret = 0;
 uint64_t bytes = 0;
 uint32_t blocks;
-uint32_t image_type = VDI_TYPE_DYNAMIC;
+uint32_t image_type;
 VdiHeader header;
 size_t i;
 size_t bmap_size;
@@ -744,9 +744,22 @@ static int coroutine_fn 
vdi_co_do_create(BlockdevCreateOptions *create_options,
 
 /* Validate options and set default values */
 bytes = vdi_opts->size;
-if (vdi_opts->q_static) {
+
+if (!vdi_opts->has_preallocation) {
+vdi_opts->preallocation = PREALLOC_MODE_OFF;
+}
+switch (vdi_opts->preallocation) {
+case PREALLOC_MODE_OFF:
+image_type = VDI_TYPE_DYNAMIC;
+break;
+case PREALLOC_MODE_METADATA:
 image_type = VDI_TYPE_STATIC;
+break;
+default:
+error_setg(errp, "Preallocation mode not supported for vdi");
+return -EINVAL;
 }
+
 #ifndef CONFIG_VDI_STATIC_IMAGE
 if (image_type == VDI_TYPE_STATIC) {
 ret = -ENOTSUP;
@@ -874,6 +887,7 @@ static int coroutine_fn vdi_co_create_opts(const char 
*filename, QemuOpts *opts,
 BlockdevCreateOptions *create_options = NULL;
 BlockDriverState *bs_file = NULL;
 uint64_t block_size = DEFAULT_CLUSTER_SIZE;
+bool is_static = false;
 Visitor *v;
 Error *local_err = NULL;
 int ret;
@@ -895,6 +909,9 @@ static int coroutine_fn vdi_co_create_opts(const char 
*filename, QemuOpts *opts,
 goto done;
 }
 #endif
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) {
+is_static = true;
+}
 
 qdict = qemu_opts_to_qdict_filtered(opts, NULL, &vdi_create_opts, true);
 
@@ -913,6 +930,9 @@ static int coroutine_fn vdi_co_create_opts(const char 
*filename, QemuOpts *opts,
 
 qdict_put_str(qdict, "driver", "vdi");
 qdict_put_str(qdict, "file", bs_file->node_name);
+if (is_static) {
+qdict_put_str(qdict, "preallocation", "metadata");
+}
 
 /* Get the QAPI object */
 v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
-- 
2.13.6

[Qemu-block] [PULL 11/19] qemu-iotests: Enable 025 for luks

[Kevin Wolf]

We want to test resizing even for luks. The only change that is needed
is to explicitly zero out new space for luks because it's undefined.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Daniel P. Berrangé 
---
 tests/qemu-iotests/025 | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/025 b/tests/qemu-iotests/025
index f5e672e6b3..70dd5f10aa 100755
--- a/tests/qemu-iotests/025
+++ b/tests/qemu-iotests/025
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.pattern
 
-_supported_fmt raw qcow2 qed
+_supported_fmt raw qcow2 qed luks
 _supported_proto file sheepdog rbd nfs
 _supported_os Linux
 
@@ -62,6 +62,13 @@ length
 EOF
 _check_test_img
 
+# bdrv_truncate() doesn't zero the new space, so we need to do that explicitly.
+# We still want to test automatic zeroing for other formats even though
+# bdrv_truncate() doesn't guarantee it.
+if [ "$IMGFMT" == "luks" ]; then
+$QEMU_IO -c "write -z $small_size $((big_size - small_size))" "$TEST_IMG" 
> /dev/null
+fi
+
 echo
 echo "=== Verifying image size after reopen"
 $QEMU_IO -c "length" "$TEST_IMG"
-- 
2.13.6

[Qemu-block] [PULL 05/19] block/blkreplay: Remove protocol-related fields

[Kevin Wolf]

From: Fabiano Rosas 

The blkreplay driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. blkreplay:) will now fail gracefully:

  $ qemu-img info blkreplay:foo
  qemu-img: Could not open 'blkreplay:foo': Unknown protocol 'blkreplay'

Signed-off-by: Fabiano Rosas 
Reviewed-by: Pavel Dovgalyuk 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/blkreplay.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/blkreplay.c b/block/blkreplay.c
index 61e44a1949..fe5a9b4a98 100755
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -129,10 +129,9 @@ static int coroutine_fn 
blkreplay_co_flush(BlockDriverState *bs)
 
 static BlockDriver bdrv_blkreplay = {
 .format_name= "blkreplay",
-.protocol_name  = "blkreplay",
 .instance_size  = 0,
 
-.bdrv_file_open = blkreplay_open,
+.bdrv_open  = blkreplay_open,
 .bdrv_close = blkreplay_close,
 .bdrv_child_perm= bdrv_filter_default_perms,
 .bdrv_getlength = blkreplay_getlength,
-- 
2.13.6

[Qemu-block] [PULL 07/19] qcow2: Reset free_cluster_index when allocating a new refcount block

[Kevin Wolf]

From: Alberto Garcia 

When we try to allocate new clusters we first look for available ones
starting from s->free_cluster_index and once we find them we increase
their reference counts. Before we get to call update_refcount() to do
this last step s->free_cluster_index is already pointing to the next
cluster after the ones we are trying to allocate.

During update_refcount() it may happen however that we also need to
allocate a new refcount block in order to store the refcounts of these
new clusters (and to complicate things further that may also require
us to grow the refcount table). After all this we don't know if the
clusters that we originally tried to allocate are still available, so
we return -EAGAIN to ask the caller to restart the search for free
clusters.

This is what can happen in a common scenario:

  1) We want to allocate a new cluster and we see that cluster N is
 free.

  2) We try to increase N's refcount but all refcount blocks are full,
 so we allocate a new one at N+1 (where s->free_cluster_index was
 pointing at).

  3) Once we're done we return -EAGAIN to look again for a free
 cluster, but now s->free_cluster_index points at N+2, so that's
 the one we allocate. Cluster N remains unallocated and we have a
 hole in the qcow2 file.

This can be reproduced easily:

 qemu-img create -f qcow2 -o cluster_size=512 hd.qcow2 1M
 qemu-io -c 'write 0 124k' hd.qcow2

After this the image has 132608 bytes (256 clusters), and the refcount
block is full. If we write 512 more bytes it should allocate two new
clusters: the data cluster itself and a new refcount block.

 qemu-io -c 'write 124k 512' hd.qcow2

However the image has now three new clusters (259 in total), and the
first one of them is empty (and unallocated):

 dd if=hd.qcow2 bs=512c skip=256 count=1 | hexdump -C

If we write larger amounts of data in the last step instead of the 512
bytes used in this example we can create larger holes in the qcow2
file.

What this patch does is reset s->free_cluster_index to its previous
value when alloc_refcount_block() returns -EAGAIN. This way the caller
will try to allocate again the original clusters if they are still
free.

The output of iotest 026 also needs to be updated because now that
images have no holes some tests fail at a different point and the
number of leaked clusters is different.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 block/qcow2-refcount.c |  7 +++
 tests/qemu-iotests/026.out |  6 +++---
 tests/qemu-iotests/121 | 20 
 tests/qemu-iotests/121.out | 10 ++
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 362deaf303..6b8b63514a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -839,6 +839,13 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 qcow2_cache_put(s->refcount_block_cache, &refcount_block);
 }
 ret = alloc_refcount_block(bs, cluster_index, &refcount_block);
+/* If the caller needs to restart the search for free clusters,
+ * try the same ones first to see if they're still free. */
+if (ret == -EAGAIN) {
+if (s->free_cluster_index > (start >> s->cluster_bits)) {
+s->free_cluster_index = (start >> s->cluster_bits);
+}
+}
 if (ret < 0) {
 goto fail;
 }
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index 86a50a2e13..8e89416a86 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -533,7 +533,7 @@ Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
-11 leaked clusters were found on the image.
+10 leaked clusters were found on the image.
 This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
@@ -561,7 +561,7 @@ Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
-11 leaked clusters were found on the image.
+10 leaked clusters were found on the image.
 This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
@@ -589,7 +589,7 @@ Failed to flush the L2 table cache: No space left on device
 Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
-11 leaked clusters were found on the image.
+10 leaked clusters were found on the image.
 This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
diff --git a/tests/qemu-iotests/121 b/tests/qemu-iotests/121
index 1307b4e327..6d6f55a5d

[Qemu-block] [PULL 06/19] include/block/block_int: Document protocol related functions

[Kevin Wolf]

From: Fabiano Rosas 

Clarify that:

- for protocols the brdv_file_open function is used instead
of bdrv_open;

- when protocol_name is set, a driver should expect
to be given only a filename and no other options.

Signed-off-by: Fabiano Rosas 
Signed-off-by: Kevin Wolf 
---
 include/block/block_int.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 27e17addba..c4dd1d4bb8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -126,6 +126,8 @@ struct BlockDriver {
 
 int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
  Error **errp);
+
+/* Protocol drivers should implement this instead of bdrv_open */
 int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
   Error **errp);
 void (*bdrv_close)(BlockDriverState *bs);
@@ -251,6 +253,12 @@ struct BlockDriver {
  */
 int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
 
+/*
+ * Drivers setting this field must be able to work with just a plain
+ * filename with ':' as a prefix, and no other options.
+ * Options may be extracted from the filename by implementing
+ * bdrv_parse_filename.
+ */
 const char *protocol_name;
 int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset,
  PreallocMode prealloc, Error **errp);
-- 
2.13.6

[Qemu-block] [PULL 01/19] iotests: 163 is not quick

[Kevin Wolf]

From: Eric Blake 

Testing on ext4, most 'quick' qcow2 tests took less than 5 seconds,
but 163 took more than 20.  Let's remove it from the quick set.

Signed-off-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/group | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index efe0e958f2..0c2983f4cd 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -167,7 +167,7 @@
 159 rw auto quick
 160 rw auto quick
 162 auto quick
-163 rw auto quick
+163 rw auto
 165 rw auto quick
 169 rw auto quick
 170 rw auto quick
-- 
2.13.6

[Qemu-block] [PULL 17/19] vhdx: Don't use error_setg_errno() with constant errno

[Kevin Wolf]

error_setg_errno() is meant for cases where we got an errno from the OS
that can add useful extra information to an error message. It's
pointless if we pass a constant errno, these cases should use plain
error_setg().

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
---
 block/vhdx.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 6a5e48eb69..4a087d8708 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1822,7 +1822,7 @@ static int coroutine_fn 
vhdx_co_create(BlockdevCreateOptions *opts,
 /* Validate options and set default values */
 image_size = vhdx_opts->size;
 if (image_size > VHDX_MAX_IMAGE_SIZE) {
-error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
+error_setg(errp, "Image size too large; max of 64TB");
 return -EINVAL;
 }
 
@@ -1832,7 +1832,7 @@ static int coroutine_fn 
vhdx_co_create(BlockdevCreateOptions *opts,
 log_size = vhdx_opts->log_size;
 }
 if (log_size < MiB || (log_size % MiB) != 0) {
-error_setg_errno(errp, EINVAL, "Log size must be a multiple of 1 MB");
+error_setg(errp, "Log size must be a multiple of 1 MB");
 return -EINVAL;
 }
 
@@ -1874,7 +1874,7 @@ static int coroutine_fn 
vhdx_co_create(BlockdevCreateOptions *opts,
 }
 
 if (block_size < MiB || (block_size % MiB) != 0) {
-error_setg_errno(errp, EINVAL, "Block size must be a multiple of 1 
MB");
+error_setg(errp, "Block size must be a multiple of 1 MB");
 return -EINVAL;
 }
 if (!is_power_of_2(block_size)) {
@@ -1882,8 +1882,7 @@ static int coroutine_fn 
vhdx_co_create(BlockdevCreateOptions *opts,
 return -EINVAL;
 }
 if (block_size > VHDX_BLOCK_SIZE_MAX) {
-error_setg_errno(errp, EINVAL, "Block size must not exceed %d",
- VHDX_BLOCK_SIZE_MAX);
+error_setg(errp, "Block size must not exceed %d", VHDX_BLOCK_SIZE_MAX);
 return -EINVAL;
 }
 
-- 
2.13.6

[Qemu-block] [PULL 15/19] qemu-iotests: Test parallels image creation with QMP

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/212 | 326 +
 tests/qemu-iotests/212.out | 111 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 438 insertions(+)
 create mode 100755 tests/qemu-iotests/212
 create mode 100644 tests/qemu-iotests/212.out

diff --git a/tests/qemu-iotests/212 b/tests/qemu-iotests/212
new file mode 100755
index 00..e5a1ba77ce
--- /dev/null
+++ b/tests/qemu-iotests/212
@@ -0,0 +1,326 @@
+#!/bin/bash
+#
+# Test parallels and file image creation
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt parallels
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+echo Testing: "$@"
+$QEMU -nographic -qmp stdio -serial none "$@"
+echo
+}
+
+function run_qemu()
+{
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+  | _filter_qemu | _filter_imgfmt \
+  | _filter_actual_image_size
+}
+
+echo
+echo "=== Successful image creation (defaults) ==="
+echo
+
+size=$((128 * 1024 * 1024))
+
+run_qemu <

[Qemu-block] [PULL 12/19] luks: Turn another invalid assertion into check

[Kevin Wolf]

Commit e39e959e fixed an invalid assertion in the .bdrv_length
implementation, but left a similar assertion in place for
.bdrv_truncate. Instead of crashing when the user requests a too large
image size, fail gracefully.

A file size of exactly INT64_MAX caused failure before, but is actually
legal.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Daniel P. Berrangé 
---
 block/crypto.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/crypto.c b/block/crypto.c
index e0b8856f74..bc6c7e3795 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -357,7 +357,11 @@ static int block_crypto_truncate(BlockDriverState *bs, 
int64_t offset,
 BlockCrypto *crypto = bs->opaque;
 uint64_t payload_offset =
 qcrypto_block_get_payload_offset(crypto->block);
-assert(payload_offset < (INT64_MAX - offset));
+
+if (payload_offset > INT64_MAX - offset) {
+error_setg(errp, "The requested file size is too large");
+return -EFBIG;
+}
 
 offset += payload_offset;
 
-- 
2.13.6

[Qemu-block] [PULL 16/19] vhdx: Require power-of-two block size on create

[Kevin Wolf]

Images with a non-power-of-two block size are invalid and cannot be
opened. Reject such block sizes when creating an image.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
---
 block/vhdx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index d2c54b7891..6a5e48eb69 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1877,6 +1877,10 @@ static int coroutine_fn 
vhdx_co_create(BlockdevCreateOptions *opts,
 error_setg_errno(errp, EINVAL, "Block size must be a multiple of 1 
MB");
 return -EINVAL;
 }
+if (!is_power_of_2(block_size)) {
+error_setg(errp, "Block size must be a power of two");
+return -EINVAL;
+}
 if (block_size > VHDX_BLOCK_SIZE_MAX) {
 error_setg_errno(errp, EINVAL, "Block size must not exceed %d",
  VHDX_BLOCK_SIZE_MAX);
-- 
2.13.6

[Qemu-block] [PULL 18/19] vhdx: Check for 4 GB maximum log size on creation

[Kevin Wolf]

It's unclear what the real maximum is, but we use an uint32_t to store
the log size in vhdx_co_create(), so we should check that the given
value fits in 32 bits.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
---
 block/vhdx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index 4a087d8708..6ac0424f61 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1829,6 +1829,10 @@ static int coroutine_fn 
vhdx_co_create(BlockdevCreateOptions *opts,
 if (!vhdx_opts->has_log_size) {
 log_size = DEFAULT_LOG_SIZE;
 } else {
+if (vhdx_opts->log_size > UINT32_MAX) {
+error_setg(errp, "Log size must be smaller than 4 GB");
+return -EINVAL;
+}
 log_size = vhdx_opts->log_size;
 }
 if (log_size < MiB || (log_size % MiB) != 0) {
-- 
2.13.6

[Qemu-block] [PULL 04/19] block/throttle: Remove protocol-related fields

[Kevin Wolf]

From: Fabiano Rosas 

The throttle driver is not a protocol so it should implement bdrv_open
instead of bdrv_file_open and not provide a protocol_name.

Attempts to invoke this driver using protocol syntax
(i.e. throttle:) will now fail gracefully:

  $ qemu-img info throttle:foo
  qemu-img: Could not open 'throttle:foo': Unknown protocol 'throttle'

Signed-off-by: Fabiano Rosas 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
Signed-off-by: Kevin Wolf 
---
 block/throttle.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/throttle.c b/block/throttle.c
index 5f4d43d0fc..95ed06acd8 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -215,10 +215,9 @@ static void coroutine_fn 
throttle_co_drain_end(BlockDriverState *bs)
 
 static BlockDriver bdrv_throttle = {
 .format_name=   "throttle",
-.protocol_name  =   "throttle",
 .instance_size  =   sizeof(ThrottleGroupMember),
 
-.bdrv_file_open =   throttle_open,
+.bdrv_open  =   throttle_open,
 .bdrv_close =   throttle_close,
 .bdrv_co_flush  =   throttle_co_flush,
 
-- 
2.13.6

[Qemu-block] [PULL 14/19] parallels: Check maximum cluster size on create

[Kevin Wolf]

It's unclear what the real maximum cluster size is for the Parallels
format, but let's at least make sure that we don't get integer
overflows in our .bdrv_co_create implementation.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/parallels.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index e2515dec81..799215e079 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -526,6 +526,11 @@ static int coroutine_fn 
parallels_co_create(BlockdevCreateOptions* opts,
 cl_size = DEFAULT_CLUSTER_SIZE;
 }
 
+/* XXX What is the real limit here? This is an insanely large maximum. */
+if (cl_size >= INT64_MAX / MAX_PARALLELS_IMAGE_FACTOR) {
+error_setg(errp, "Cluster size is too large");
+return -EINVAL;
+}
 if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) {
 error_setg(errp, "Image size is too large for this cluster size");
 return -E2BIG;
-- 
2.13.6

[Qemu-block] [PULL 09/19] vdi: Fix build with CONFIG_VDI_DEBUG

[Kevin Wolf]

Use qemu_uuid_unparse() instead of uuid_unparse() to make vdi.c compile
again when CONFIG_VDI_DEBUG is set. In order to prevent future bitrot,
replace '#ifdef CONFIG_VDI_DEBUG' by 'if (VDI_DEBUG)' so that the
compiler always sees the code.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/vdi.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 73c059e69d..4a2d1ff88d 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -235,7 +235,6 @@ static void vdi_header_to_le(VdiHeader *header)
 qemu_uuid_bswap(&header->uuid_parent);
 }
 
-#if defined(CONFIG_VDI_DEBUG)
 static void vdi_header_print(VdiHeader *header)
 {
 char uuid[37];
@@ -257,16 +256,15 @@ static void vdi_header_print(VdiHeader *header)
 logout("block extra 0x%04x\n", header->block_extra);
 logout("blocks tot. 0x%04x\n", header->blocks_in_image);
 logout("blocks all. 0x%04x\n", header->blocks_allocated);
-uuid_unparse(header->uuid_image, uuid);
+qemu_uuid_unparse(&header->uuid_image, uuid);
 logout("uuid image  %s\n", uuid);
-uuid_unparse(header->uuid_last_snap, uuid);
+qemu_uuid_unparse(&header->uuid_last_snap, uuid);
 logout("uuid snap   %s\n", uuid);
-uuid_unparse(header->uuid_link, uuid);
+qemu_uuid_unparse(&header->uuid_link, uuid);
 logout("uuid link   %s\n", uuid);
-uuid_unparse(header->uuid_parent, uuid);
+qemu_uuid_unparse(&header->uuid_parent, uuid);
 logout("uuid parent %s\n", uuid);
 }
-#endif
 
 static int coroutine_fn vdi_co_check(BlockDriverState *bs, BdrvCheckResult 
*res,
  BdrvCheckMode fix)
@@ -387,9 +385,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 
 vdi_header_to_cpu(&header);
-#if defined(CONFIG_VDI_DEBUG)
-vdi_header_print(&header);
-#endif
+if (VDI_DEBUG) {
+vdi_header_print(&header);
+}
 
 if (header.disk_size > VDI_DISK_SIZE_MAX) {
 error_setg(errp, "Unsupported VDI image size (size is 0x%" PRIx64
@@ -825,9 +823,9 @@ static int coroutine_fn 
vdi_co_do_create(BlockdevCreateOptions *create_options,
 qemu_uuid_generate(&header.uuid_image);
 qemu_uuid_generate(&header.uuid_last_snap);
 /* There is no need to set header.uuid_link or header.uuid_parent here. */
-#if defined(CONFIG_VDI_DEBUG)
-vdi_header_print(&header);
-#endif
+if (VDI_DEBUG) {
+vdi_header_print(&header);
+}
 vdi_header_to_le(&header);
 ret = blk_pwrite(blk, offset, &header, sizeof(header), 0);
 if (ret < 0) {
-- 
2.13.6

[Qemu-block] [PULL 19/19] qemu-iotests: Test vhdx image creation with QMP

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/213 | 349 +
 tests/qemu-iotests/213.out | 121 
 tests/qemu-iotests/group   |   1 +
 3 files changed, 471 insertions(+)
 create mode 100755 tests/qemu-iotests/213
 create mode 100644 tests/qemu-iotests/213.out

diff --git a/tests/qemu-iotests/213 b/tests/qemu-iotests/213
new file mode 100755
index 00..3a00a0f6d6
--- /dev/null
+++ b/tests/qemu-iotests/213
@@ -0,0 +1,349 @@
+#!/bin/bash
+#
+# Test vhdx and file image creation
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt vhdx
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+echo Testing: "$@"
+$QEMU -nographic -qmp stdio -serial none "$@"
+echo
+}
+
+function run_qemu()
+{
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+  | _filter_qemu | _filter_imgfmt \
+  | _filter_actual_image_size
+}
+
+echo
+echo "=== Successful image creation (defaults) ==="
+echo
+
+size=$((128 * 1024 * 1024))
+
+run_qemu <

[Qemu-block] [PULL 13/19] qemu-iotests: Test invalid resize on luks

[Kevin Wolf]

This tests that the .bdrv_truncate implementation for luks doesn't crash
for invalid image sizes.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/210 | 37 +
 tests/qemu-iotests/210.out | 16 
 2 files changed, 53 insertions(+)

diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index 96a5213e77..e607c0d296 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -204,6 +204,43 @@ run_qemu -blockdev 
driver=file,filename="$TEST_IMG_FILE",node-name=node0 \
 { "execute": "quit" }
 EOF
 
+echo
+echo "=== Resize image with invalid sizes ==="
+echo
+
+run_qemu -blockdev driver=file,filename="$TEST_IMG_FILE",node-name=node0 \
+ -blockdev driver=luks,file=node0,key-secret=keysec0,node-name=node1 \
+ -object secret,id=keysec0,data="foo" <0 
size"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
+
+image: json:{"driver": "IMGFMT", "file": {"driver": "file", "filename": 
"TEST_DIR/t.IMGFMT"}, "key-secret": "keysec0"}
+file format: IMGFMT
+virtual size: 0 (0 bytes)
 *** done
-- 
2.13.6

[Qemu-block] [PULL 10/19] qemu-iotests: Test vdi image creation with QMP

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/211 | 246 +
 tests/qemu-iotests/211.out |  97 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 344 insertions(+)
 create mode 100755 tests/qemu-iotests/211
 create mode 100644 tests/qemu-iotests/211.out

diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211
new file mode 100755
index 00..1edec26517
--- /dev/null
+++ b/tests/qemu-iotests/211
@@ -0,0 +1,246 @@
+#!/bin/bash
+#
+# Test VDI and file image creation
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt vdi
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+echo Testing: "$@"
+$QEMU -nographic -qmp stdio -serial none "$@"
+echo
+}
+
+function run_qemu()
+{
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+  | _filter_qemu | _filter_imgfmt \
+  | _filter_actual_image_size
+}
+
+echo
+echo "=== Successful image creation (defaults) ==="
+echo
+
+size=$((128 * 1024 * 1024))
+
+run_qemu <

Re: [Qemu-block] [Qemu-devel] [PATCH 5.5/7] dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

[no-reply]

Hi,

This series failed docker-build@min-glib build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180326115346.11939-1-vsement...@virtuozzo.com
Subject: [Qemu-devel] [PATCH 5.5/7] dirty-bitmap: drop unused 
bdrv_undo_clear_dirty_bitmap

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
410b0c5abb dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-yebfeo97/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   min-glib
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-yebfeo97/src'
  GEN 
/var/tmp/patchew-tester-tmp-yebfeo97/src/docker-src.2018-03-26-11.54.52.21487/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-yebfeo97/src/docker-src.2018-03-26-11.54.52.21487/qemu.tar.vroot'...
done.
Checking out files:  50% (3080/6057)   
Checking out files:  51% (3090/6057)   
Checking out files:  52% (3150/6057)   
Checking out files:  53% (3211/6057)   
Checking out files:  54% (3271/6057)   
Checking out files:  55% (3332/6057)   
Checking out files:  56% (3392/6057)   
Checking out files:  57% (3453/6057)   
Checking out files:  58% (3514/6057)   
Checking out files:  59% (3574/6057)   
Checking out files:  60% (3635/6057)   
Checking out files:  61% (3695/6057)   
Checking out files:  62% (3756/6057)   
Checking out files:  63% (3816/6057)   
Checking out files:  64% (3877/6057)   
Checking out files:  65% (3938/6057)   
Checking out files:  66% (3998/6057)   
Checking out files:  67% (4059/6057)   
Checking out files:  68% (4119/6057)   
Checking out files:  68% (4131/6057)   
Checking out files:  69% (4180/6057)   
Checking out files:  70% (4240/6057)   
Checking out files:  71% (4301/6057)   
Checking out files:  72% (4362/6057)   
Checking out files:  73% (4422/6057)   
Checking out files:  74% (4483/6057)   
Checking out files:  75% (4543/6057)   
Checking out files:  76% (4604/6057)   
Checking out files:  77% (4664/6057)   
Checking out files:  78% (4725/6057)   
Checking out files:  79% (4786/6057)   
Checking out files:  80% (4846/6057)   
Checking out files:  81% (4907/6057)   
Checking out files:  82% (4967/6057)   
Checking out files:  83% (5028/6057)   
Checking out files:  84% (5088/6057)   
Checking out files:  85% (5149/6057)   
Checking out files:  86% (5210/6057)   
Checking out files:  87% (5270/6057)   
Checking out files:  88% (5331/6057)   
Checking out files:  89% (5391/6057)   
Checking out files:  90% (5452/6057)   
Checking out files:  91% (5512/6057)   
Checking out files:  92% (5573/6057)   
Checking out files:  93% (5634/6057)   
Checking out files:  94% (5694/6057)   
Checking out files:  95% (5755/6057)   
Checking out files:  96% (5815/6057)   
Checking out files:  97% (5876/6057)   
Checking out files:  98% (5936/6057)   
Checking out files:  99% (5997/6057)   
Checking out files: 100% (6057/6057)   
Checking out files: 100% (6057/6057), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-yebfeo97/src/docker-src.2018-03-26-11.54.52.21487/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-yebfeo97/src/docker-src.2018-03-26-11.54.52.21487/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-build in qemu:min-glib 
Environment variables:
HOSTNAME=a06a8d03bde1
MAKEFLAGS= -j8
J=8
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
TARGET_LIST=
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
FEATURES= dtc
DEBUG=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/tmp/qemu-test/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/install
BIOS directory/tmp/qemu-test/install/share/qemu
firmware path /tmp/qemu-test/install/share/qemu-firmware
binary directory  /tmp/qemu-test/install/bin
library directory /tmp/qemu-test/install/lib
module directory  /tmp/qemu-test/install/lib/qemu
libexec directory /tmp/qemu-test/install/libexec
include directory /tmp/qemu-test/install/include
config directory  /tmp/qemu-t

Re: [Qemu-block] [PATCH v3] vmdk: return ERROR when cluster sector is larger than vmdk limitation

[Max Reitz]

On 2018-03-22 14:33, yuchen...@synology.com wrote:
> From: yuchenlin 
> 
> VMDK has a hard limitation of extent size, which is due to the size of grain
> table entry is 32 bits. It means it can only point to a grain located at
> offset = 2^32. To avoid writing the user data beyond limitation and record a 
> useless offset
> in grain table. We should return ERROR here.
> 
> Signed-off-by: yuchenlin 
> ---
> v2->v3:
>  - use (1ULL << 32) to clearly show the limitation of offset is 2^32.
> 
>  thanks
> 
>  block/vmdk.c | 6 ++
>  1 file changed, 6 insertions(+)

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH 5.5/7] dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Message-id: 20180326115346.11939-1-vsement...@virtuozzo.com
Subject: [Qemu-devel] [PATCH 5.5/7] dirty-bitmap: drop unused 
bdrv_undo_clear_dirty_bitmap

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
410b0c5abb dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

=== OUTPUT BEGIN ===
=== ENV ===
LANG=en_US.UTF-8
XDG_SESSION_ID=114676
USER=fam
PWD=/var/tmp/patchew-tester-tmp-bobpnwo0/src
HOME=/home/fam
SHELL=/bin/sh
SHLVL=2
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
PATH=/usr/bin:/bin
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
glibc-debuginfo-common-2.24-10.fc25.s390x
fedora-release-26-1.noarch
dejavu-sans-mono-fonts-2.35-4.fc26.noarch
xemacs-filesystem-21.5.34-22.20170124hgf412e9f093d4.fc26.noarch
bash-4.4.12-7.fc26.s390x
freetype-2.7.1-9.fc26.s390x
libSM-1.2.2-5.fc26.s390x
libmpc-1.0.2-6.fc26.s390x
libaio-0.3.110-7.fc26.s390x
libverto-0.2.6-7.fc26.s390x
perl-Scalar-List-Utils-1.48-1.fc26.s390x
iptables-libs-1.6.1-2.fc26.s390x
p11-kit-trust-0.23.9-2.fc26.s390x
tcl-8.6.6-2.fc26.s390x
libxshmfence-1.2-4.fc26.s390x
expect-5.45-23.fc26.s390x
perl-Thread-Queue-3.12-1.fc26.noarch
perl-encoding-2.19-6.fc26.s390x
keyutils-1.5.10-1.fc26.s390x
gmp-devel-6.1.2-4.fc26.s390x
enchant-1.6.0-16.fc26.s390x
net-snmp-libs-5.7.3-17.fc26.s390x
python-gobject-base-3.24.1-1.fc26.s390x
python3-enchant-1.6.10-1.fc26.noarch
python-lockfile-0.11.0-6.fc26.noarch
python2-pyparsing-2.1.10-3.fc26.noarch
python2-lxml-4.1.1-1.fc26.s390x
librados2-10.2.7-2.fc26.s390x
trousers-lib-0.3.13-7.fc26.s390x
libpaper-1.1.24-14.fc26.s390x
libdatrie-0.2.9-4.fc26.s390x
libsoup-2.58.2-1.fc26.s390x
passwd-0.79-9.fc26.s390x
bind99-libs-9.9.10-3.P3.fc26.s390x
python3-rpm-4.13.0.2-1.fc26.s390x
systemd-233-7.fc26.s390x
virglrenderer-0.6.0-1.20170210git76b3da97b.fc26.s390x
s390utils-ziomon-1.36.1-3.fc26.s390x
s390utils-osasnmpd-1.36.1-3.fc26.s390x
libXrandr-1.5.1-2.fc26.s390x
libglvnd-glx-1.0.0-1.fc26.s390x
texlive-ifxetex-svn19685.0.5-33.fc26.2.noarch
texlive-psnfss-svn33946.9.2a-33.fc26.2.noarch
texlive-dvipdfmx-def-svn40328-33.fc26.2.noarch
texlive-natbib-svn20668.8.31b-33.fc26.2.noarch
texlive-xdvi-bin-svn40750-33.20160520.fc26.2.s390x
texlive-cm-svn32865.0-33.fc26.2.noarch
texlive-beton-svn15878.0-33.fc26.2.noarch
texlive-fpl-svn15878.1.002-33.fc26.2.noarch
texlive-mflogo-svn38628-33.fc26.2.noarch
texlive-texlive-docindex-svn41430-33.fc26.2.noarch
texlive-luaotfload-bin-svn34647.0-33.20160520.fc26.2.noarch
texlive-koma-script-svn41508-33.fc26.2.noarch
texlive-pst-tree-svn24142.1.12-33.fc26.2.noarch
texlive-breqn-svn38099.0.98d-33.fc26.2.noarch
texlive-xetex-svn41438-33.fc26.2.noarch
gstreamer1-plugins-bad-free-1.12.3-1.fc26.s390x
xorg-x11-font-utils-7.5-33.fc26.s390x
ghostscript-fonts-5.50-36.fc26.noarch
libXext-devel-1.3.3-5.fc26.s390x
libusbx-devel-1.0.21-2.fc26.s390x
libglvnd-devel-1.0.0-1.fc26.s390x
emacs-25.3-3.fc26.s390x
alsa-lib-devel-1.1.4.1-1.fc26.s390x
kbd-2.0.4-2.fc26.s390x
dconf-0.26.0-2.fc26.s390x
ccache-3.3.4-1.fc26.s390x
mc-4.8.19-5.fc26.s390x
doxygen-1.8.13-9.fc26.s390x
dpkg-1.18.24-1.fc26.s390x
libtdb-1.3.13-1.fc26.s390x
python2-pynacl-1.1.1-1.fc26.s390x
nss-sysinit-3.34.0-1.0.fc26.s390x
kernel-4.13.16-202.fc26.s390x
perl-Filter-1.58-1.fc26.s390x
python2-pip-9.0.1-11.fc26.noarch
dnf-2.7.5-2.fc26.noarch
sssd-common-1.16.0-4.fc26.s390x
python2-sssdconfig-1.16.0-4.fc26.noarch
bind-license-9.11.2-1.P1.fc26.noarch
libtasn1-4.13-1.fc26.s390x
glusterfs-fuse-3.10.10-1.fc26.s390x
cpp-7.3.1-2.fc26.s390x
pkgconf-1.3.12-2.fc26.s390x
python2-fedora-0.10.0-1.fc26.noarch
cmake-filesystem-3.10.1-11.fc26.s390x
selinux-policy-targeted-3.13.1-260.18.fc26.noarch
python3-requests-kerberos-0.12.0-1.fc26.noarch
libmicrohttpd-0.9.59-1.fc26.s390x
GeoIP-GeoLite-data-2018.01-1.fc26.noarch
glibc-debuginfo-2.24-10.fc25.s390x
dejavu-fonts-common-2.35-4.fc26.noarch
bind99-license-9.9.10-3.P3.fc26.noarch
ncurses-libs-6.0-8.20170212.fc26.s390x
libpng-1.6.28-2.fc26.s390x
libICE-1.0.9-9.fc26.s390x
perl-Text-ParseWords-3.30-366.fc26.noarch
libtool-ltdl-2.4.6-17.fc26.s390x
libselinux-utils-2.6-7.fc26.s390x
userspace-rcu-0.9.3-2.fc26.s390x
libXfont-1.5.2-5.fc26.s390x
perl-Class-Inspector-1.31-3.fc26.noarch
perl-open-1.10-395.fc26.noarch
keyutils-libs-devel-1.5.10-1.fc26

Re: [Qemu-block] [Qemu-devel] [PATCH 5.5/7] dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

[no-reply]

Hi,

This series failed docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180326115346.11939-1-vsement...@virtuozzo.com
Subject: [Qemu-devel] [PATCH 5.5/7] dirty-bitmap: drop unused 
bdrv_undo_clear_dirty_bitmap

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   7b1db0908d..2ffd221d07  master -> master
Switched to a new branch 'test'
410b0c5abb dirty-bitmap: drop unused bdrv_undo_clear_dirty_bitmap

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-7dmtkgpr/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-7dmtkgpr/src'
  GEN 
/var/tmp/patchew-tester-tmp-7dmtkgpr/src/docker-src.2018-03-26-09.19.04.21085/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-7dmtkgpr/src/docker-src.2018-03-26-09.19.04.21085/qemu.tar.vroot'...
done.
Checking out files:  46% (2793/6057)   
Checking out files:  47% (2847/6057)   
Checking out files:  48% (2908/6057)   
Checking out files:  49% (2968/6057)   
Checking out files:  50% (3029/6057)   
Checking out files:  51% (3090/6057)   
Checking out files:  52% (3150/6057)   
Checking out files:  53% (3211/6057)   
Checking out files:  54% (3271/6057)   
Checking out files:  55% (3332/6057)   
Checking out files:  56% (3392/6057)   
Checking out files:  57% (3453/6057)   
Checking out files:  58% (3514/6057)   
Checking out files:  59% (3574/6057)   
Checking out files:  60% (3635/6057)   
Checking out files:  61% (3695/6057)   
Checking out files:  62% (3756/6057)   
Checking out files:  63% (3816/6057)   
Checking out files:  64% (3877/6057)   
Checking out files:  65% (3938/6057)   
Checking out files:  66% (3998/6057)   
Checking out files:  67% (4059/6057)   
Checking out files:  68% (4119/6057)   
Checking out files:  69% (4180/6057)   
Checking out files:  70% (4240/6057)   
Checking out files:  71% (4301/6057)   
Checking out files:  72% (4362/6057)   
Checking out files:  73% (4422/6057)   
Checking out files:  74% (4483/6057)   
Checking out files:  75% (4543/6057)   
Checking out files:  76% (4604/6057)   
Checking out files:  77% (4664/6057)   
Checking out files:  78% (4725/6057)   
Checking out files:  79% (4786/6057)   
Checking out files:  80% (4846/6057)   
Checking out files:  81% (4907/6057)   
Checking out files:  82% (4967/6057)   
Checking out files:  83% (5028/6057)   
Checking out files:  84% (5088/6057)   
Checking out files:  85% (5149/6057)   
Checking out files:  86% (5210/6057)   
Checking out files:  87% (5270/6057)   
Checking out files:  88% (5331/6057)   
Checking out files:  89% (5391/6057)   
Checking out files:  90% (5452/6057)   
Checking out files:  91% (5512/6057)   
Checking out files:  92% (5573/6057)   
Checking out files:  93% (5634/6057)   
Checking out files:  94% (5694/6057)   
Checking out files:  95% (5755/6057)   
Checking out files:  96% (5815/6057)   
Checking out files:  97% (5876/6057)   
Checking out files:  98% (5936/6057)   
Checking out files:  99% (5997/6057)   
Checking out files: 100% (6057/6057)   
Checking out files: 100% (6057/6057), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-7dmtkgpr/src/docker-src.2018-03-26-09.19.04.21085/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-7dmtkgpr/src/docker-src.2018-03-26-09.19.04.21085/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-mingw in qemu:fedora 
Packages installed:
PyYAML-3.12-5.fc27.x86_64
SDL-devel-1.2.15-29.fc27.x86_64
bc-1.07.1-3.fc27.x86_64
bison-3.0.4-8.fc27.x86_64
bzip2-1.0.6-24.fc27.x86_64
ccache-3.3.6-1.fc27.x86_64
clang-5.0.1-3.fc27.x86_64
findutils-4.6.0-16.fc27.x86_64
flex-2.6.1-5.fc27.x86_64
gcc-7.3.1-5.fc27.x86_64
gcc-c++-7.3.1-5.fc27.x86_64
gettext-0.19.8.1-12.fc27.x86_64
git-2.14.3-3.fc27.x86_64
glib2-devel-2.54.3-2.fc27.x86_64
hostname-3.18-4.fc27.x86_64
libaio-devel-0.3.110-9.fc27.x86_64
libasan-7.3.1-5.fc27.x86_64
libfdt-devel-1.4.6-1.fc27.x86_64
libubsan-7.3.1-5.fc27.x86_64
llvm-5.0.1-3.fc27.x86_64
make-4.2.1-4.fc27.x86_64
mingw32-SDL-1.2.15-9.fc27.noarch
mingw32-bzip2-1.0.6-9.fc27.noarch
mingw32-curl-7.54.1-2.fc27.noarch
mingw32-g

Re: [Qemu-block] [PATCH v4 for 2.12 0/3] fix bitmaps migration through shared storage

[Max Reitz]

On 2018-03-20 18:05, Vladimir Sementsov-Ogievskiy wrote:
> Hi all.
> 
> This fixes bitmaps migration through shared storage. Look at 02 for
> details.
> 
> The bug introduced in 2.10 with the whole qcow2 bitmaps feature, so
> qemu-stable in CC. However I doubt that someone really suffered from this.
> 
> Do we need dirty bitmaps at all in inactive case? - that was a question in v2.
> And, keeping in mind that we are going to use inactive mode not only for
> incoming migration, I'm not sure that answer is NO (but, it may be "NO" for 
> 2.10, 2.11), so let's fix it in proposed here manner at least for 2.12.

For some reason, I can't get 169 to work now at all[1].  What's more,
whenever I run it, two (on current master, maybe more after this series)
"cat $TEST_DIR/mig_file" processes stay around.  That doesn't seem right.

However, this series doesn't seem to make it worse[2]...  So I'm keeping
it.  I suppose it's just some issue with the test.

Max


[1] Sometimes there are migration even timeouts, sometimes just VM
launch timeouts (specifically when VM B is supposed to be re-launched
just after it has been shut down), and sometimes I get a dirty bitmap
hash mismatch.


[2] The whole timeline was:

- Apply this series, everything seems alright

(a couple of hours later)
- Test some other things, stumble over 169 once or so

- Focus on 169, fails a bit more often

(today)
- Can't get it to work at all

- Can't get it to work in any version, neither before nor after this patch

- Lose my sanity

- Write this email

O:-)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] qemu-nbd

[Max Reitz]

On 2018-03-22 13:17, Pascal wrote:
> hi everybody,
> 
> when I use `qemu-nbd -s -c /dev/nbd0 /dev/sda` where are changes to
> /dev/ndb0 saved ?
> I searched in /proc/{pid_of_qemu-nbd}/ but found nothing there...

In snapshot mode, qemu (or in this case qemu-nbd) creates a temporary
qcow2 file in $TMPDIR (or /var/tmp) which will get the changes.  You
should see a link to that file in /proc/$pid/fd, like this:

lrwx--. [...] 10 -> /var/tmp/vl.JjYLDm (deleted)

As you can see, the file is deleted right after it is opened, though.

If you want to have control over where the data is stored, you can just
create the overlay yourself, like so:

$ qemu-img create -f qcow2 -b /dev/sda -F raw overlay.qcow2
$ qemu-nbd -c /dev/nbd0 overlay.qcow2

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-block] nbd BLOCK_STATUS with -m32

[Max Reitz]

Hi,

iotests 209 and 123 fail under 32-bit (or at least with -m32), both with
gcc and mingw.

Diff for 209:

@@ -1,2 +1,2 @@
-[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true},
-{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data":
false}]
+qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS
+qemu-img: Could not read file metadata: Invalid argument


Diff for 123:

@@ -3,6 +3,10 @@
 Formatting 'TEST_DIR/source.IMGFMT', fmt=IMGFMT size=1048576
 wrote 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS
+qemu-img: error getting block status at offset 0: Invalid argument
+qemu-img: error while writing sector 0: Input/output error
+Pattern verification failed at offset 0, 1048576 bytes
 read 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)


Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] nbd BLOCK_STATUS with -m32

[Eric Blake]

On 03/26/2018 01:39 PM, Max Reitz wrote:

Hi,

iotests 209 and 123 fail under 32-bit (or at least with -m32), both with
gcc and mingw.

Diff for 209:

@@ -1,2 +1,2 @@
-[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true},
-{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data":
false}]
+qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS
+qemu-img: Could not read file metadata: Invalid argument


Diff for 123:

@@ -3,6 +3,10 @@
  Formatting 'TEST_DIR/source.IMGFMT', fmt=IMGFMT size=1048576
  wrote 1048576/1048576 bytes at offset 0
  1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS
+qemu-img: error getting block status at offset 0: Invalid argument


Looks to be same root cause; probably an incorrect type promotion or 
truncation somewhere.  I'll step through the code and submit a patch, 
hopefully in time for -rc1.


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

[Qemu-block] [PULL 2/4] qcow2: fix bitmaps loading when bitmaps already exist

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

On reopen with existing bitmaps, instead of loading bitmaps, lets
reopen them if needed. This also fixes bitmaps migration through
shared storage.
Consider the case. Persistent bitmaps are stored on bdrv_inactivate.
Then, on destination process_incoming_migration_bh() calls
bdrv_invalidate_cache_all() which leads to
qcow2_load_autoloading_dirty_bitmaps() which fails if bitmaps are
already loaded on destination start.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20180320170521.32152-3-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/qcow2.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index cf4f3becae..486f3e83b7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1480,7 +1480,22 @@ static int coroutine_fn qcow2_do_open(BlockDriverState 
*bs, QDict *options,
 s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
 }
 
-if (qcow2_load_dirty_bitmaps(bs, &local_err)) {
+if (bdrv_dirty_bitmap_next(bs, NULL)) {
+/* It's some kind of reopen with already existing dirty bitmaps. There
+ * are no known cases where we need loading bitmaps in such situation,
+ * so it's safer don't load them.
+ *
+ * Moreover, if we have some readonly bitmaps and we are reopening for
+ * rw we should reopen bitmaps correspondingly.
+ */
+if (bdrv_has_readonly_bitmaps(bs) &&
+!bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
+{
+bool header_updated = false;
+qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err);
+update_header = update_header && !header_updated;
+}
+} else if (qcow2_load_dirty_bitmaps(bs, &local_err)) {
 update_header = false;
 }
 if (local_err != NULL) {
-- 
2.14.3

[Qemu-block] [PULL 3/4] iotests: enable shared migration cases in 169

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Shared migration for dirty bitmaps is fixed by previous patches,
so we can enable the test.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20180320170521.32152-5-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/169 | 8 +++-
 tests/qemu-iotests/169.out | 4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
index 3a8db91f6f..153b10b6e7 100755
--- a/tests/qemu-iotests/169
+++ b/tests/qemu-iotests/169
@@ -140,16 +140,14 @@ def inject_test_case(klass, name, method, *args, 
**kwargs):
 mc = operator.methodcaller(method, *args, **kwargs)
 setattr(klass, 'test_' + name, new.instancemethod(mc, None, klass))
 
-for cmb in list(itertools.product((True, False), repeat=3)):
+for cmb in list(itertools.product((True, False), repeat=4)):
 name = ('_' if cmb[0] else '_not_') + 'persistent_'
 name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
 name += '_online' if cmb[2] else '_offline'
-
-# TODO fix shared-storage bitmap migration and enable cases for it
-args = list(cmb) + [False]
+name += '_shared' if cmb[3] else '_nonshared'
 
 inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
- *args)
+ *list(cmb))
 
 
 if __name__ == '__main__':
diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
index 594c16f49f..b6f257674e 100644
--- a/tests/qemu-iotests/169.out
+++ b/tests/qemu-iotests/169.out
@@ -1,5 +1,5 @@
-
+
 --
-Ran 8 tests
+Ran 16 tests
 
 OK
-- 
2.14.3

[Qemu-block] [PULL 0/4] Block patches for 2.12.0-rc1

[Max Reitz]

The following changes since commit 7b93d78a04aa242d377ae213b79db6c319c71847:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2018-03-26 15:17:25 +0100)

are available in the Git repository at:

  git://github.com/XanClic/qemu.git tags/pull-block-2018-03-26

for you to fetch changes up to a77672ea3d95094a0cb4f974de84fb7353c67cc0:

  vmdk: return ERROR when cluster sector is larger than vmdk limitation 
(2018-03-26 21:17:24 +0200)


A fix for dirty bitmap migration through shared storage, and a VMDK
patch keeping us from creating too large extents.


Vladimir Sementsov-Ogievskiy (3):
  qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()
  qcow2: fix bitmaps loading when bitmaps already exist
  iotests: enable shared migration cases in 169

yuchenlin (1):
  vmdk: return ERROR when cluster sector is larger than vmdk limitation

 block/qcow2.h  |  2 ++
 block/qcow2-bitmap.c   | 15 ++-
 block/qcow2.c  | 17 -
 block/vmdk.c   |  6 ++
 tests/qemu-iotests/169 |  8 +++-
 tests/qemu-iotests/169.out |  4 ++--
 6 files changed, 43 insertions(+), 9 deletions(-)

-- 
2.14.3

[Qemu-block] [PULL 1/4] qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Add version of qcow2_reopen_bitmaps_rw, which do the same work but
also return a hint about was header updated or not. This will be
used in the following fix for bitmaps reloading after migration.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: John Snow 
Reviewed-by: Max Reitz 
Message-id: 20180320170521.32152-2-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/qcow2.h|  2 ++
 block/qcow2-bitmap.c | 15 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index ccb92a9696..d301f77cea 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -671,6 +671,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
   void **refcount_table,
   int64_t *refcount_table_size);
 bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
+ Error **errp);
 int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
 void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
 int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 3010adb909..6e93ec43e1 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1004,7 +1004,8 @@ fail:
 return false;
 }
 
-int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
+int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
+ Error **errp)
 {
 BDRVQcow2State *s = bs->opaque;
 Qcow2BitmapList *bm_list;
@@ -1012,6 +1013,10 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error 
**errp)
 GSList *ro_dirty_bitmaps = NULL;
 int ret = 0;
 
+if (header_updated != NULL) {
+*header_updated = false;
+}
+
 if (s->nb_bitmaps == 0) {
 /* No bitmaps - nothing to do */
 return 0;
@@ -1055,6 +1060,9 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error 
**errp)
 error_setg_errno(errp, -ret, "Can't update bitmap directory");
 goto out;
 }
+if (header_updated != NULL) {
+*header_updated = true;
+}
 g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
 }
 
@@ -1065,6 +1073,11 @@ out:
 return ret;
 }
 
+int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
+{
+return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp);
+}
+
 /* store_bitmap_data()
  * Store bitmap to image, filling bitmap table accordingly.
  */
-- 
2.14.3

[Qemu-block] [PULL 4/4] vmdk: return ERROR when cluster sector is larger than vmdk limitation

[Max Reitz]

From: yuchenlin 

VMDK has a hard limitation of extent size, which is due to the size of grain
table entry is 32 bits. It means it can only point to a grain located at
offset = 2^32. To avoid writing the user data beyond limitation and record a 
useless offset
in grain table. We should return ERROR here.

Signed-off-by: yuchenlin 
Message-id: 2018032217.28024-1-yuchen...@synology.com
Reviewed-by: Fam Zheng 
Signed-off-by: Max Reitz 
---
 block/vmdk.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index f94c49a9c0..84f8bbe480 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -47,6 +47,8 @@
 #define VMDK4_FLAG_MARKER (1 << 17)
 #define VMDK4_GD_AT_END 0xULL
 
+#define VMDK_EXTENT_MAX_SECTORS (1ULL << 32)
+
 #define VMDK_GTE_ZEROED 0x1
 
 /* VMDK internal error codes */
@@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs,
 return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
 }
 
+if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) {
+return VMDK_ERROR;
+}
+
 cluster_sector = extent->next_cluster_sector;
 extent->next_cluster_sector += extent->cluster_sectors;
 
-- 
2.14.3

Re: [Qemu-block] [PATCH 0/2] block/file-posix: Fix fully preallocated truncate

[Max Reitz]

On 2018-02-28 14:13, Max Reitz wrote:
> Fully preallocated truncation has a 50 % chance of working on images
> files over file-posix.  It works if $SIZE % 4G < 2G, and it fails
> otherwise.  To make things even more interesting, often you would not
> even notice because qemu reported success even though it did nothing
> (because after the successful lseek(), errno was still 0, so when the
> file-posix driver tried to return a negative error code, it actually
> reported success).
> 
> This issue is fixed by patch 1 in this series.  Thanks to Daniel for
> reporting!
> 
> 
> Max Reitz (2):
>   block/file-posix: Fix fully preallocated truncate
>   iotests: Test preallocated truncate of 2G image
> 
>  block/file-posix.c |  5 +++--
>  tests/qemu-iotests/106 | 24 
>  tests/qemu-iotests/106.out | 10 ++
>  3 files changed, 37 insertions(+), 2 deletions(-)

Applied to my block tree...  (*cough* *cough* -- I'm the worst when it
comes to keeping track of my own patches.)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH 3/4] qdict: remove useless cast

[Fam Zheng]

On Thu, 03/22 17:12, Laurent Vivier wrote:
> Re-run Coccinelle script scripts/coccinelle/qobject.cocci
> 
> Signed-off-by: Laurent Vivier 

Acked-by: Fam Zheng 

Re: [Qemu-block] [PATCH v2 4/5] qdict: remove useless cast

[Fam Zheng]

On Fri, 03/23 15:32, Laurent Vivier wrote:
> Re-run Coccinelle script scripts/coccinelle/qobject.cocci
> 
> Signed-off-by: Laurent Vivier 

This time for the right revision of the patch:

Acked-by: Fam Zheng 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/1] iotests: fix test case 185

[QingFeng Hao]

在 2018/3/23 18:04, Stefan Hajnoczi 写道:

On Fri, Mar 23, 2018 at 3:43 AM, QingFeng Hao  wrote:

Test case 185 failed since commit 4486e89c219 --- "vl: introduce vm_shutdown()".
It's because of the newly introduced function vm_shutdown calls bdrv_drain_all,
which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs
that doubles the speed and offset is doubled.
Some jobs' status are changed as well.

The fix is to not resume the jobs that are already yielded and also change
185.out accordingly.

Suggested-by: Stefan Hajnoczi 
Signed-off-by: QingFeng Hao 
---
  blockjob.c | 10 +-
  include/block/blockjob.h   |  5 +
  tests/qemu-iotests/185.out | 11 +--


If drain no longer forces the block job to iterate, shouldn't the test
output remain the same?  (The means the test is fixed by the QEMU
patch.)


  3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index ef3ed69ff1..fa9838ac97 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -206,11 +206,16 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob 
*job)

  static void block_job_pause(BlockJob *job)
  {
-job->pause_count++;
+if (!job->yielded) {
+job->pause_count++;
+}


The pause cannot be ignored.  This change introduces a bug.

Pause is not a synchronous operation that stops the job immediately.
Pause just remembers that the job needs to be paused.   When the job
runs again (e.g. timer callback, fd handler) it eventually reaches
block_job_pause_point() where it really pauses.

The bug in this patch is:

1. The job has a timer pending.
2. block_job_pause() is called during drain.
3. The timer fires during drain but now the job doesn't know it needs
to pause, so it continues running!

Instead what needs to happen is that block_job_pause() remains
unmodified but block_job_resume if extended:

static void block_job_resume(BlockJob *job)
{
 assert(job->pause_count > 0);
 job->pause_count--;
 if (job->pause_count) {
 return;
 }
+if (job_yielded_before_pause_and_is_still_yielded) {
Thanks a lot for your great comments! But I can't figure out how to 
check this.

 block_job_enter(job);
+}
}

This handles the case I mentioned above, where the yield ends before
pause ends (therefore resume must enter the job!).

To make this a little clearer, there are two cases to consider:

Case 1:
1. Job yields
2. Pause
3. Job is entered from timer/fd callback

How do I know that if job is entered from ...? thanks

4. Resume (enter job? yes)

Case 2:
1. Job yields
2. Pause
3. Resume (enter job? no)
4. Job is entered from timer/fd callback

Stefan



--
Regards
QingFeng Hao

Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/1] iotests: fix test case 185

[QingFeng Hao]

在 2018/3/26 18:29, Kevin Wolf 写道:

Am 23.03.2018 um 04:43 hat QingFeng Hao geschrieben:

Test case 185 failed since commit 4486e89c219 --- "vl: introduce vm_shutdown()".
It's because of the newly introduced function vm_shutdown calls bdrv_drain_all,
which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs
that doubles the speed and offset is doubled.
Some jobs' status are changed as well.

The fix is to not resume the jobs that are already yielded and also change
185.out accordingly.

Suggested-by: Stefan Hajnoczi 
Signed-off-by: QingFeng Hao 


Stefan already commented on the fix itself, but I want to add two more
points:

Please change your subject line. "iotests: fix test case 185" means that
you are fixing the test case, not qemu code that makes the test case
fail. The subject line should describe the actual change. In all
likelihood it will start with "blockjob:" rather than "iotests:".

Sure! thanks for pointing that.



diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index fc645dac68..f8f208bbcf 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -99,6 +99,11 @@ typedef struct BlockJob {
  bool ready;
  
  /**

+ * Set to true when the job is yielded.
+ */
+bool yielded;


This is the same as !busy, so we don't need a new field for this.

Mostly yes, but the trick is that busy is set to be true in 
block_job_do_yield.

Kevin



--
Regards
QingFeng Hao