qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/3] Bitmaps patches
@ 2019-02-13 23:49 John Snow
  2019-02-13 23:49 ` [Qemu-devel] [PULL 1/3] dirty-bitmap: Expose persistent flag to 'query-block' John Snow
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: John Snow @ 2019-02-13 23:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, jsnow

The following changes since commit 0b5e750bea635b167eb03d86c3d9a09bbd43bc06:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-02-12 10:53:37 +0000)

are available in the Git repository at:

  https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request

for you to fetch changes up to fbe8c2e3b041c682d3d52030b53e1627f285a84b:

  blockdev: acquire aio_context for bitmap add/remove (2019-02-13 18:42:46 -0500)

----------------------------------------------------------------
Pull request

----------------------------------------------------------------

Eric Blake (1):
  dirty-bitmap: Expose persistent flag to 'query-block'

John Snow (2):
  block/dirty-bitmap: Documentation and Comment fixups
  blockdev: acquire aio_context for bitmap add/remove

 block/dirty-bitmap.c       | 21 +++++++++++++++------
 blockdev.c                 | 26 ++++++++++++++++++++------
 qapi/block-core.json       | 37 ++++++++++++++++++++++++++++---------
 tests/qemu-iotests/124     |  1 +
 tests/qemu-iotests/236.out | 14 ++++++++++++++
 5 files changed, 78 insertions(+), 21 deletions(-)

-- 
2.17.2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 1/3] dirty-bitmap: Expose persistent flag to 'query-block'
  2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
@ 2019-02-13 23:49 ` John Snow
  2019-02-13 23:49 ` [Qemu-devel] [PULL 2/3] block/dirty-bitmap: Documentation and Comment fixups John Snow
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2019-02-13 23:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, jsnow, Eric Blake

From: Eric Blake <eblake@redhat.com>

Since qemu currently doesn't flush persistent bitmaps to disk until
shutdown (which might be MUCH later), it's useful if 'query-block'
at least shows WHICH bitmaps will (eventually) make it to persistent
storage.  Update affected iotests.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190204210512.27458-1-eblake@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/dirty-bitmap.c       |  1 +
 qapi/block-core.json       |  5 ++++-
 tests/qemu-iotests/124     |  1 +
 tests/qemu-iotests/236.out | 14 ++++++++++++++
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 00ea36f554..e46f72b346 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -440,6 +440,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
         info->has_name = !!bm->name;
         info->name = g_strdup(bm->name);
         info->status = bdrv_dirty_bitmap_status(bm);
+        info->persistent = bm->persistent;
         entry->value = info;
         *plist = entry;
         plist = &entry->next;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0f349d4603..7e8656d04c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -450,11 +450,14 @@
 #
 # @status: current status of the dirty bitmap (since 2.4)
 #
+# @persistent: true if the bitmap will eventually be flushed to persistent
+#              storage (since 4.0)
+#
 # Since: 1.3
 ##
 { 'struct': 'BlockDirtyInfo',
   'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
-           'status': 'DirtyBitmapStatus'} }
+           'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
 
 ##
 # @Qcow2BitmapInfoFlags:
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 9f189e3b54..5aa1bf1bd6 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -350,6 +350,7 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
         self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/count', 458752)
         self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/granularity', 65536)
         self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/status', 'active')
+        self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/persistent', False)
 
         # Prepare a cluster_size=128k backup target without a backing file.
         (target, _) = bitmap0.new_target()
diff --git a/tests/qemu-iotests/236.out b/tests/qemu-iotests/236.out
index bb2d71ea5e..5006f7bca1 100644
--- a/tests/qemu-iotests/236.out
+++ b/tests/qemu-iotests/236.out
@@ -25,12 +25,14 @@ write -P0xcd 0x3ff0000 64k
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapB",
+        "persistent": false,
         "status": "active"
       },
       {
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapA",
+        "persistent": false,
         "status": "active"
       }
     ]
@@ -85,12 +87,14 @@ write -P0xcd 0x3ff0000 64k
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapB",
+        "persistent": false,
         "status": "active"
       },
       {
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapA",
+        "persistent": false,
         "status": "active"
       }
     ]
@@ -183,18 +187,21 @@ write -P0xea 0x3fe0000 64k
         "count": 393216,
         "granularity": 65536,
         "name": "bitmapC",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapB",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 458752,
         "granularity": 65536,
         "name": "bitmapA",
+        "persistent": false,
         "status": "disabled"
       }
     ]
@@ -247,18 +254,21 @@ write -P0xea 0x3fe0000 64k
         "count": 393216,
         "granularity": 65536,
         "name": "bitmapC",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapB",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 458752,
         "granularity": 65536,
         "name": "bitmapA",
+        "persistent": false,
         "status": "disabled"
       }
     ]
@@ -304,24 +314,28 @@ write -P0xea 0x3fe0000 64k
         "count": 458752,
         "granularity": 65536,
         "name": "bitmapD",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 393216,
         "granularity": 65536,
         "name": "bitmapC",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 262144,
         "granularity": 65536,
         "name": "bitmapB",
+        "persistent": false,
         "status": "disabled"
       },
       {
         "count": 458752,
         "granularity": 65536,
         "name": "bitmapA",
+        "persistent": false,
         "status": "disabled"
       }
     ]
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 2/3] block/dirty-bitmap: Documentation and Comment fixups
  2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
  2019-02-13 23:49 ` [Qemu-devel] [PULL 1/3] dirty-bitmap: Expose persistent flag to 'query-block' John Snow
@ 2019-02-13 23:49 ` John Snow
  2019-02-13 23:49 ` [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove John Snow
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2019-02-13 23:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, jsnow

The meaning of the states has changed subtly over time,
this should bring the understanding more in-line with the
current, actual usages.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190202011048.12343-1-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/dirty-bitmap.c | 20 ++++++++++++++------
 qapi/block-core.json | 32 ++++++++++++++++++++++++--------
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index e46f72b346..c6d4acebfa 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -29,12 +29,20 @@
 #include "block/blockjob.h"
 
 /**
- * A BdrvDirtyBitmap can be in three possible states:
- * (1) successor is NULL and disabled is false: full r/w mode
- * (2) successor is NULL and disabled is true: read only mode ("disabled")
- * (3) successor is set: frozen mode.
- *     A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
- *     or enabled. A frozen bitmap can only abdicate() or reclaim().
+ * A BdrvDirtyBitmap can be in four possible user-visible states:
+ * (1) Active:   successor is NULL, and disabled is false: full r/w mode
+ * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
+ *               guest writes are dropped, but monitor writes are possible,
+ *               through commands like merge and clear.
+ * (3) Frozen:   successor is not NULL.
+ *               A frozen bitmap cannot be renamed, deleted, cleared, set,
+ *               enabled, merged to, etc. A frozen bitmap can only abdicate()
+ *               or reclaim().
+ *               In this state, the anonymous successor bitmap may be either
+ *               Active and recording writes from the guest (e.g. backup jobs),
+ *               but it can be Disabled and not recording writes.
+ * (4) Locked:   Whether Active or Disabled, the user cannot modify this bitmap
+ *               in any way from the monitor.
  */
 struct BdrvDirtyBitmap {
     QemuMutex *mutex;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7e8656d04c..8f23f2ebb8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -420,17 +420,27 @@
 #
 # An enumeration of possible states that a dirty bitmap can report to the user.
 #
-# @frozen: The bitmap is currently in-use by a backup operation or block job,
-#          and is immutable.
+# @frozen: The bitmap is currently in-use by some operation and is immutable.
+#          If the bitmap was @active prior to the operation, new writes by the
+#          guest are being recorded in a temporary buffer, and will not be lost.
+#          Generally, bitmaps are cleared on successful use in an operation and
+#          the temporary buffer is committed into the bitmap. On failure, the
+#          temporary buffer is merged back into the bitmap without first
+#          clearing it.
+#          Please refer to the documentation for each bitmap-using operation,
+#          See also @blockdev-backup, @drive-backup.
 #
-# @disabled: The bitmap is currently in-use by an internal operation and is
-#            read-only. It can still be deleted.
+# @disabled: The bitmap is not currently recording new writes by the guest.
+#            This is requested explicitly via @block-dirty-bitmap-disable.
+#            It can still be cleared, deleted, or used for backup operations.
 #
 # @active: The bitmap is actively monitoring for new writes, and can be cleared,
 #          deleted, or used for backup operations.
 #
-# @locked: The bitmap is currently in-use by some operation and can not be
-#          cleared, deleted, or used for backup operations. (Since 2.12)
+# @locked: The bitmap is currently in-use by some operation and is immutable.
+#          If the bitmap was @active prior to the operation, it is still
+#          recording new writes. If the bitmap was @disabled, it is not
+#          recording new writes. (Since 2.12)
 #
 # Since: 2.4
 ##
@@ -2094,9 +2104,15 @@
 # @block-dirty-bitmap-merge:
 #
 # Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
-# The @bitmaps dirty bitmaps are unchanged.
+# Dirty bitmaps in @bitmaps will be unchanged, except if it also appears
+# as the @target bitmap. Any bits already set in @target will still be
+# set after the merge, i.e., this operation does not clear the target.
 # On error, @target is unchanged.
 #
+# The resulting bitmap will count as dirty any clusters that were dirty in any
+# of the source bitmaps. This can be used to achieve backup checkpoints, or in
+# simpler usages, to copy bitmaps.
+#
 # Returns: nothing on success
 #          If @node is not a valid block device, DeviceNotFound
 #          If any bitmap in @bitmaps or @target is not found, GenericError
@@ -2131,7 +2147,7 @@
 ##
 # @x-debug-block-dirty-bitmap-sha256:
 #
-# Get bitmap SHA256
+# Get bitmap SHA256.
 #
 # Returns: BlockDirtyBitmapSha256 on success
 #          If @node is not a valid block device, DeviceNotFound
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove
  2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
  2019-02-13 23:49 ` [Qemu-devel] [PULL 1/3] dirty-bitmap: Expose persistent flag to 'query-block' John Snow
  2019-02-13 23:49 ` [Qemu-devel] [PULL 2/3] block/dirty-bitmap: Documentation and Comment fixups John Snow
@ 2019-02-13 23:49 ` John Snow
  2019-02-14 15:46   ` Eric Blake
  2019-02-13 23:50 ` [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: John Snow @ 2019-02-13 23:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, jsnow

When bitmaps are persistent, they may incur a disk read or write when bitmaps
are added or removed. For configurations like virtio-dataplane, failing to
acquire this lock will abort QEMU when disk IO occurs.

We used to acquire aio_context as part of the bitmap lookup, so re-introduce
the lock for just the cases that have an IO penalty. Commit 2119882c removed
these locks, and I failed to notice this when we committed fd5ae4cc, so this
has been broken since persistent bitmaps were introduced.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1672010
Reported-By: Aihua Liang <aliang@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20190213195501.7790-2-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fb18e9c975..790b0e0ac8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2820,6 +2820,7 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
 {
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
+    AioContext *aio_context = NULL;
 
     if (!name || name[0] == '\0') {
         error_setg(errp, "Bitmap name cannot be empty");
@@ -2854,15 +2855,17 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
         disabled = false;
     }
 
-    if (persistent &&
-        !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
-    {
-        return;
+    if (persistent) {
+        aio_context = bdrv_get_aio_context(bs);
+        aio_context_acquire(aio_context);
+        if (!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp)) {
+            goto out;
+        }
     }
 
     bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
     if (bitmap == NULL) {
-        return;
+        goto out
     }
 
     if (disabled) {
@@ -2870,6 +2873,10 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
     }
 
     bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
+ out:
+    if (aio_context) {
+        aio_context_release(aio_context);
+    }
 }
 
 void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
@@ -2878,6 +2885,7 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
     BlockDriverState *bs;
     BdrvDirtyBitmap *bitmap;
     Error *local_err = NULL;
+    AioContext *aio_context = NULL;
 
     bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
     if (!bitmap || !bs) {
@@ -2892,14 +2900,20 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
     }
 
     if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
+        aio_context = bdrv_get_aio_context(bs);
+        aio_context_acquire(aio_context);
         bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err);
         if (local_err != NULL) {
             error_propagate(errp, local_err);
-            return;
+            goto out;
         }
     }
 
     bdrv_release_dirty_bitmap(bs, bitmap);
+ out:
+    if (aio_context) {
+        aio_context_release(aio_context);
+    }
 }
 
 /**
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 0/3] Bitmaps patches
  2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
                   ` (2 preceding siblings ...)
  2019-02-13 23:49 ` [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove John Snow
@ 2019-02-13 23:50 ` John Snow
  2019-02-14 15:49   ` Eric Blake
  2019-02-15 16:23 ` Peter Maydell
  2019-02-21 20:14 ` no-reply
  5 siblings, 1 reply; 10+ messages in thread
From: John Snow @ 2019-02-13 23:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell


On 2/13/19 6:49 PM, John Snow wrote:
> The following changes since commit 0b5e750bea635b167eb03d86c3d9a09bbd43bc06:
> 
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-02-12 10:53:37 +0000)
> 
> are available in the Git repository at:
> 
>   https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
> 
> for you to fetch changes up to fbe8c2e3b041c682d3d52030b53e1627f285a84b:
> 
>   blockdev: acquire aio_context for bitmap add/remove (2019-02-13 18:42:46 -0500)
> 
> ----------------------------------------------------------------
> Pull request
> 

P.S. I have extended the expiry on my GPG key and extended the expiry to
2021, you may need to refresh keys.

(Hopefully I did it right. This does not come up often.)

> ----------------------------------------------------------------
> 
> Eric Blake (1):
>   dirty-bitmap: Expose persistent flag to 'query-block'
> 
> John Snow (2):
>   block/dirty-bitmap: Documentation and Comment fixups
>   blockdev: acquire aio_context for bitmap add/remove
> 
>  block/dirty-bitmap.c       | 21 +++++++++++++++------
>  blockdev.c                 | 26 ++++++++++++++++++++------
>  qapi/block-core.json       | 37 ++++++++++++++++++++++++++++---------
>  tests/qemu-iotests/124     |  1 +
>  tests/qemu-iotests/236.out | 14 ++++++++++++++
>  5 files changed, 78 insertions(+), 21 deletions(-)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove
  2019-02-13 23:49 ` [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove John Snow
@ 2019-02-14 15:46   ` Eric Blake
  2019-02-14 23:12     ` John Snow
  0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2019-02-14 15:46 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: peter.maydell

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

On 2/13/19 5:49 PM, John Snow wrote:
> When bitmaps are persistent, they may incur a disk read or write when bitmaps
> are added or removed. For configurations like virtio-dataplane, failing to
> acquire this lock will abort QEMU when disk IO occurs.
> 
> We used to acquire aio_context as part of the bitmap lookup, so re-introduce
> the lock for just the cases that have an IO penalty. Commit 2119882c removed
> these locks, and I failed to notice this when we committed fd5ae4cc, so this
> has been broken since persistent bitmaps were introduced.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1672010
> Reported-By: Aihua Liang <aliang@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Message-id: 20190213195501.7790-2-jsnow@redhat.com
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  blockdev.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)

>      bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
>      if (bitmap == NULL) {
> -        return;
> +        goto out

Embarrassing enough that I missed this syntax error in spite of my R-b,
but even more embarrassing that we have a pull request that has
obviously not been compiled. At least patchew called it out.

We're not in enough of a rush to need this PR to land this week, so I
won't try to post a v2 on your behalf, but will instead wait for your
return next week.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 0/3] Bitmaps patches
  2019-02-13 23:50 ` [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
@ 2019-02-14 15:49   ` Eric Blake
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2019-02-14 15:49 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: peter.maydell

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

On 2/13/19 5:50 PM, John Snow wrote:
> 
> On 2/13/19 6:49 PM, John Snow wrote:
>> The following changes since commit 0b5e750bea635b167eb03d86c3d9a09bbd43bc06:
>>
>>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-02-12 10:53:37 +0000)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
>>
>> for you to fetch changes up to fbe8c2e3b041c682d3d52030b53e1627f285a84b:
>>
>>   blockdev: acquire aio_context for bitmap add/remove (2019-02-13 18:42:46 -0500)
>>
>> ----------------------------------------------------------------
>> Pull request
>>
> 
> P.S. I have extended the expiry on my GPG key and extended the expiry to
> 2021, you may need to refresh keys.
> 
> (Hopefully I did it right. This does not come up often.)

Expiration on the key shows as 2021 on my end after a re-fetch of key
0x88A9064D183561EB, so that part looks right. But we'll need a v2 pull
request, due to the fact that this one fails compilation on all
platforms. :(

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove
  2019-02-14 15:46   ` Eric Blake
@ 2019-02-14 23:12     ` John Snow
  0 siblings, 0 replies; 10+ messages in thread
From: John Snow @ 2019-02-14 23:12 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: peter.maydell



On 2/14/19 10:46 AM, Eric Blake wrote:
> On 2/13/19 5:49 PM, John Snow wrote:
>> When bitmaps are persistent, they may incur a disk read or write when bitmaps
>> are added or removed. For configurations like virtio-dataplane, failing to
>> acquire this lock will abort QEMU when disk IO occurs.
>>
>> We used to acquire aio_context as part of the bitmap lookup, so re-introduce
>> the lock for just the cases that have an IO penalty. Commit 2119882c removed
>> these locks, and I failed to notice this when we committed fd5ae4cc, so this
>> has been broken since persistent bitmaps were introduced.
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1672010
>> Reported-By: Aihua Liang <aliang@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Message-id: 20190213195501.7790-2-jsnow@redhat.com
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  blockdev.c | 26 ++++++++++++++++++++------
>>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
>>      bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
>>      if (bitmap == NULL) {
>> -        return;
>> +        goto out
> 
> Embarrassing enough that I missed this syntax error in spite of my R-b,
> but even more embarrassing that we have a pull request that has
> obviously not been compiled. At least patchew called it out.

Hm, yes, I swore I *did* compile and test this, but it clearly must have
been on the outdated patch.

> 
> We're not in enough of a rush to need this PR to land this week, so I
> won't try to post a v2 on your behalf, but will instead wait for your
> return next week.
> 

Was trying to stay ahead of my long weekend instead of carrying these
patches forever. Please forgive the error.

--js

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 0/3] Bitmaps patches
  2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
                   ` (3 preceding siblings ...)
  2019-02-13 23:50 ` [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
@ 2019-02-15 16:23 ` Peter Maydell
  2019-02-21 20:14 ` no-reply
  5 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2019-02-15 16:23 UTC (permalink / raw)
  To: John Snow; +Cc: QEMU Developers

On Wed, 13 Feb 2019 at 23:49, John Snow <jsnow@redhat.com> wrote:
>
> The following changes since commit 0b5e750bea635b167eb03d86c3d9a09bbd43bc06:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-02-12 10:53:37 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
>
> for you to fetch changes up to fbe8c2e3b041c682d3d52030b53e1627f285a84b:
>
>   blockdev: acquire aio_context for bitmap add/remove (2019-02-13 18:42:46 -0500)
>
> ----------------------------------------------------------------
> Pull request
>
> ----------------------------------------------------------------

As Eric says, this doesn't compile:

/home/petmay01/linaro/qemu-for-merges/blockdev.c: In function
‘qmp_block_dirty_bitmap_add’:
/home/petmay01/linaro/qemu-for-merges/blockdev.c:2869:5: error:
expected ‘;’ before ‘}’ token
     }
     ^

but I've confirmed that I have your updated gpg key info and
could verify the tag signature.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 0/3] Bitmaps patches
  2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
                   ` (4 preceding siblings ...)
  2019-02-15 16:23 ` Peter Maydell
@ 2019-02-21 20:14 ` no-reply
  5 siblings, 0 replies; 10+ messages in thread
From: no-reply @ 2019-02-21 20:14 UTC (permalink / raw)
  To: jsnow; +Cc: fam, qemu-devel, peter.maydell

Patchew URL: https://patchew.org/QEMU/20190213234907.24173-1-jsnow@redhat.com/



Hi,

This series failed the 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.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14
=== TEST SCRIPT END ===

  CC      qapi/qapi-commands.o
  CC      qapi/qapi-commands-block-core.o
/tmp/qemu-test/src/blockdev.c: In function 'qmp_block_dirty_bitmap_add':
/tmp/qemu-test/src/blockdev.c:2868:17: error: expected ';' before '}' token
         goto out
                 ^
                 ;


The full log is available at
http://patchew.org/logs/20190213234907.24173-1-jsnow@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-02-21 20:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-13 23:49 [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
2019-02-13 23:49 ` [Qemu-devel] [PULL 1/3] dirty-bitmap: Expose persistent flag to 'query-block' John Snow
2019-02-13 23:49 ` [Qemu-devel] [PULL 2/3] block/dirty-bitmap: Documentation and Comment fixups John Snow
2019-02-13 23:49 ` [Qemu-devel] [PULL 3/3] blockdev: acquire aio_context for bitmap add/remove John Snow
2019-02-14 15:46   ` Eric Blake
2019-02-14 23:12     ` John Snow
2019-02-13 23:50 ` [Qemu-devel] [PULL 0/3] Bitmaps patches John Snow
2019-02-14 15:49   ` Eric Blake
2019-02-15 16:23 ` Peter Maydell
2019-02-21 20:14 ` no-reply

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).