* [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-22 16:17 ` Max Reitz
2015-04-22 19:20 ` Eric Blake
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 02/21] qapi: Add optional field "name" to block dirty bitmap John Snow
` (21 subsequent siblings)
22 siblings, 2 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Signed-off-by: John Snow <jsnow@redhat.com>
---
docs/bitmaps.md | 352 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 352 insertions(+)
create mode 100644 docs/bitmaps.md
diff --git a/docs/bitmaps.md b/docs/bitmaps.md
new file mode 100644
index 0000000..f066b48
--- /dev/null
+++ b/docs/bitmaps.md
@@ -0,0 +1,352 @@
+<!--
+Copyright 2015 John Snow <jsnow@redhat.com> and Red Hat, Inc.
+All rights reserved.
+
+This file is licensed via The FreeBSD Documentation License, the full text of
+which is included at the end of this document.
+-->
+
+# Dirty Bitmaps and Incremental Backup
+
+* Dirty Bitmaps are objects that track which data needs to be backed up for the
+ next incremental backup.
+
+* Dirty bitmaps can be created at any time and attached to any node
+ (not just complete drives.)
+
+## Dirty Bitmap Names
+
+* A dirty bitmap's name is unique to the node, but bitmaps attached to different
+ nodes can share the same name.
+
+## Bitmap Modes
+
+* A Bitmap can be "frozen," which means that it is currently in-use by a backup
+ operation and cannot be deleted, renamed, written to, reset,
+ etc.
+
+## Basic QMP Usage
+
+### Supported Commands ###
+
+* block-dirty-bitmap-add
+* block-dirty-bitmap-remove
+* block-dirty-bitmap-clear
+
+### Creation
+
+* To create a new bitmap, enabled, on the drive with id=drive0:
+
+```json
+{ "execute": "block-dirty-bitmap-add",
+ "arguments": {
+ "node": "drive0",
+ "name": "bitmap0"
+ }
+}
+```
+
+* This bitmap will have a default granularity that matches the cluster size of
+ its associated drive, if available, clamped to between [4KiB, 64KiB].
+ The current default for qcow2 is 64KiB.
+
+* To create a new bitmap that tracks changes in 32KiB segments:
+
+```json
+{ "execute": "block-dirty-bitmap-add",
+ "arguments": {
+ "node": "drive0",
+ "name": "bitmap0",
+ "granularity": 32768
+ }
+}
+```
+
+### Deletion
+
+* Bitmaps that are frozen cannot be deleted.
+
+* Deleting the bitmap does not impact any other bitmaps attached to the same
+ node, nor does it affect any backups already created from this node.
+
+* Because bitmaps are only unique to the node to which they are attached,
+ you must specify the node/drive name here, too.
+
+```json
+{ "execute": "block-dirty-bitmap-remove",
+ "arguments": {
+ "node": "drive0",
+ "name": "bitmap0"
+ }
+}
+```
+
+### Resetting
+
+* Resetting a bitmap will clear all information it holds.
+
+* An incremental backup created from an empty bitmap will copy no data,
+ as if nothing has changed.
+
+```json
+{ "execute": "block-dirty-bitmap-clear",
+ "arguments": {
+ "node": "drive0",
+ "name": "bitmap0"
+ }
+}
+```
+
+## Transactions (Not yet implemented)
+
+* Transactional commands are forthcoming in a future version,
+ and are not yet available for use. This section serves as
+ documentation of intent for their design and usage.
+
+### Justification
+
+Bitmaps can be safely modified when the VM is paused or halted by using
+the basic QMP commands. For instance, you might perform the following actions:
+
+1. Boot the VM in a paused state.
+2. Create a full drive backup of drive0.
+3. Create a new bitmap attached to drive0.
+4. Resume execution of the VM.
+5. Incremental backups are ready to be created.
+
+At this point, the bitmap and drive backup would be correctly in sync,
+and incremental backups made from this point forward would be correctly aligned
+to the full drive backup.
+
+This is not particularly useful if we decide we want to start incremental
+backups after the VM has been running for a while, for which we will need to
+perform actions such as the following:
+
+1. Boot the VM and begin execution.
+2. Using a single transaction, perform the following operations:
+ * Create bitmap0.
+ * Create a full drive backup of drive0.
+3. Incremental backups are now ready to be created.
+
+### Supported Bitmap Transactions
+
+* block-dirty-bitmap-add
+* block-dirty-bitmap-clear
+
+The usages are identical to their respective QMP commands, but see below
+for examples.
+
+### Example: New Incremental Backup
+
+As outlined in the justification, perhaps we want to create a new incremental
+backup chain attached to a drive.
+
+```json
+{ "execute": "transaction",
+ "arguments": {
+ "actions": [
+ {"type": "block-dirty-bitmap-add",
+ "data": {"node": "drive0", "name": "bitmap0"} },
+ {"type": "drive-backup",
+ "data": {"device": "drive0", "target": "/path/to/full_backup.img",
+ "sync": "full", "format": "qcow2"} }
+ ]
+ }
+}
+```
+
+### Example: New Incremental Backup Anchor Point
+
+Maybe we just want to create a new full backup with an existing bitmap and
+want to reset the bitmap to track the new chain.
+
+```json
+{ "execute": "transaction",
+ "arguments": {
+ "actions": [
+ {"type": "block-dirty-bitmap-clear",
+ "data": {"node": "drive0", "name": "bitmap0"} },
+ {"type": "drive-backup",
+ "data": {"device": "drive0", "target": "/path/to/new_full_backup.img",
+ "sync": "full", "format": "qcow2"} }
+ ]
+ }
+}
+```
+
+## Incremental Backups
+
+The star of the show.
+
+**Nota Bene!** Only incremental backups of entire drives are supported for now.
+So despite the fact that you can attach a bitmap to any arbitrary node, they are
+only currently useful when attached to the root node. This is because
+drive-backup only supports drives/devices instead of arbitrary nodes.
+
+### Example: First Incremental Backup
+
+1. Create a full backup and sync it to the dirty bitmap, as in the transactional
+examples above; or with the VM offline, manually create a full copy and then
+create a new bitmap before the VM begins execution.
+
+ * Let's assume the full backup is named 'full_backup.img'.
+ * Let's assume the bitmap you created is 'bitmap0' attached to 'drive0'.
+
+2. Create a destination image for the incremental backup that utilizes the
+full backup as a backing image.
+
+ * Let's assume it is named 'incremental.0.img'.
+
+ ```sh
+ # qemu-img create -f qcow2 incremental.0.img -b full_backup.img -F qcow2
+ ```
+
+3. Issue the incremental backup command:
+
+ ```json
+ { "execute": "drive-backup",
+ "arguments": {
+ "device": "drive0",
+ "bitmap": "bitmap0",
+ "target": "incremental.0.img",
+ "format": "qcow2",
+ "sync": "dirty-bitmap",
+ "mode": "existing"
+ }
+ }
+ ```
+
+### Example: Second Incremental Backup
+
+1. Create a new destination image for the incremental backup that points to the
+ previous one, e.g.: 'incremental.1.img'
+
+ ```sh
+ # qemu-img create -f qcow2 incremental.1.img -b incremental.0.img -F qcow2
+ ```
+
+2. Issue a new incremental backup command. The only difference here is that we
+ have changed the target image below.
+
+ ```json
+ { "execute": "drive-backup",
+ "arguments": {
+ "device": "drive0",
+ "bitmap": "bitmap0",
+ "target": "incremental.1.img",
+ "format": "qcow2",
+ "sync": "dirty-bitmap",
+ "mode": "existing"
+ }
+ }
+ ```
+
+## Errors
+
+* In the event of an error that occurs after a backup job is successfully
+ launched, either by a direct QMP command or a QMP transaction, the user
+ will receive a BLOCK_JOB_COMPLETE event with a failure message, accompanied
+ by a BLOCK_JOB_ERROR event.
+
+* In the case of an event being cancelled, the user will receive a
+ BLOCK_JOB_CANCELLED event instead of a pair of COMPLETE and ERROR events.
+
+* In either case, the incremental backup data contained within the bitmap is
+ safely rolled back, and the data within the bitmap is not lost. The image
+ file created for the failed attempt can be safely deleted.
+
+* Once the underlying problem is fixed (e.g. more storage space is freed up),
+ you can simply retry the incremental backup command with the same bitmap.
+
+### Example
+
+1. Create a target image:
+
+ ```sh
+ # qemu-img create -f qcow2 incremental.0.img -b full_backup.img -F qcow2
+ ```
+
+2. Attempt to create an incremental backup via QMP:
+
+ ```json
+ { "execute": "drive-backup",
+ "arguments": {
+ "device": "drive0",
+ "bitmap": "bitmap0",
+ "target": "incremental.0.img",
+ "format": "qcow2",
+ "sync": "dirty-bitmap",
+ "mode": "existing"
+ }
+ }
+ ```
+
+3. Receive an event notifying us of failure:
+
+ ```json
+ { "timestamp": { "seconds": 1424709442, "microseconds": 844524 },
+ "data": { "speed": 0, "offset": 0, "len": 67108864,
+ "error": "No space left on device",
+ "device": "drive1", "type": "backup" },
+ "event": "BLOCK_JOB_COMPLETED" }
+ ```
+
+4. Delete the failed incremental, and re-create the image.
+
+ ```sh
+ # rm incremental.0.img
+ # qemu-img create -f qcow2 incremental.0.img -b full_backup.img -F qcow2
+ ```
+
+5. Retry the command after fixing the underlying problem,
+ such as freeing up space on the backup volume:
+
+ ```json
+ { "execute": "drive-backup",
+ "arguments": {
+ "device": "drive0",
+ "bitmap": "bitmap0",
+ "target": "incremental.0.img",
+ "format": "qcow2",
+ "sync": "dirty-bitmap",
+ "mode": "existing"
+ }
+ }
+ ```
+
+6. Receive confirmation that the job completed successfully:
+
+ ```json
+ { "timestamp": { "seconds": 1424709668, "microseconds": 526525 },
+ "data": { "device": "drive1", "type": "backup",
+ "speed": 0, "len": 67108864, "offset": 67108864},
+ "event": "BLOCK_JOB_COMPLETED" }
+ ```
+
+<!--
+The FreeBSD Documentation License
+
+Redistribution and use in source (Markdown) and 'compiled' forms (SGML, HTML,
+PDF, PostScript, RTF and so forth) with or without modification, are permitted
+provided that the following conditions are met:
+
+Redistributions of source code (Markdown) must retain the above copyright
+notice, this list of conditions and the following disclaimer of this file
+unmodified.
+
+Redistributions in compiled form (transformed to other DTDs, converted to PDF,
+PostScript, RTF and other formats) must reproduce the above copyright notice,
+this list of conditions and the following disclaimer in the documentation and/or
+other materials provided with the distribution.
+
+THIS DOCUMENTATION IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+THIS DOCUMENTATION, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+-->
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation John Snow
@ 2015-04-22 16:17 ` Max Reitz
2015-04-22 19:20 ` Eric Blake
1 sibling, 0 replies; 52+ messages in thread
From: Max Reitz @ 2015-04-22 16:17 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha
On 18.04.2015 01:49, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> docs/bitmaps.md | 352 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 352 insertions(+)
> create mode 100644 docs/bitmaps.md
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation John Snow
2015-04-22 16:17 ` Max Reitz
@ 2015-04-22 19:20 ` Eric Blake
1 sibling, 0 replies; 52+ messages in thread
From: Eric Blake @ 2015-04-22 19:20 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 418 bytes --]
On 04/17/2015 05:49 PM, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> docs/bitmaps.md | 352 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 352 insertions(+)
> create mode 100644 docs/bitmaps.md
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 02/21] qapi: Add optional field "name" to block dirty bitmap
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 03/21] qmp: Ensure consistent granularity type John Snow
` (20 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
From: Fam Zheng <famz@redhat.com>
This field will be set for user created dirty bitmap. Also pass in an
error pointer to bdrv_create_dirty_bitmap, so when a name is already
taken on this BDS, it can report an error message. This is not global
check, two BDSes can have dirty bitmap with a common name.
Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
be used later when other QMP commands want to reference dirty bitmap by
name.
Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 32 +++++++++++++++++++++++++++++++-
block/mirror.c | 2 +-
include/block/block.h | 7 ++++++-
migration/block.c | 2 +-
qapi/block-core.json | 4 +++-
5 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index f2f8ae7..0da35ae 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ char *name;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5435,7 +5436,28 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
return true;
}
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
+{
+ BdrvDirtyBitmap *bm;
+
+ assert(name);
+ QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+ if (bm->name && !strcmp(name, bm->name)) {
+ return bm;
+ }
+ }
+ return NULL;
+}
+
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+ g_free(bitmap->name);
+ bitmap->name = NULL;
+}
+
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+ int granularity,
+ const char *name,
Error **errp)
{
int64_t bitmap_size;
@@ -5443,6 +5465,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
assert((granularity & (granularity - 1)) == 0);
+ if (name && bdrv_find_dirty_bitmap(bs, name)) {
+ error_setg(errp, "Bitmap already exists: %s", name);
+ return NULL;
+ }
granularity >>= BDRV_SECTOR_BITS;
assert(granularity);
bitmap_size = bdrv_nb_sectors(bs);
@@ -5453,6 +5479,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+ bitmap->name = g_strdup(name);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
}
@@ -5464,6 +5491,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
if (bm == bitmap) {
QLIST_REMOVE(bitmap, list);
hbitmap_free(bitmap->bitmap);
+ g_free(bitmap->name);
g_free(bitmap);
return;
}
@@ -5482,6 +5510,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->count = bdrv_get_dirty_count(bs, bm);
info->granularity =
((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ info->has_name = !!bm->name;
+ info->name = g_strdup(bm->name);
entry->value = info;
*plist = entry;
plist = &entry->next;
diff --git a/block/mirror.c b/block/mirror.c
index 4056164..f073ad7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -703,7 +703,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
s->granularity = granularity;
s->buf_size = MAX(buf_size, granularity);
- s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+ s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
if (!s->dirty_bitmap) {
return;
}
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..05e32f9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -449,8 +449,13 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
struct HBitmapIter;
typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+ int granularity,
+ const char *name,
Error **errp);
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+ const char *name);
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
diff --git a/migration/block.c b/migration/block.c
index 085c0fa..02a7d26 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -320,7 +320,7 @@ static int set_dirty_tracking(void)
QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE,
- NULL);
+ NULL, NULL);
if (!bmds->dirty_bitmap) {
ret = -errno;
goto fail;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7873084..c10e664 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -330,6 +330,8 @@
#
# Block dirty bitmap information.
#
+# @name: #optional the name of the dirty bitmap (Since 2.4)
+#
# @count: number of dirty bytes according to the dirty bitmap
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
@@ -337,7 +339,7 @@
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'count': 'int', 'granularity': 'int'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
##
# @BlockInfo:
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 03/21] qmp: Ensure consistent granularity type
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 01/21] docs: incremental backup documentation John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 02/21] qapi: Add optional field "name" to block dirty bitmap John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 04/21] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
` (19 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
We treat this field with a variety of different types everywhere
in the code. Now it's just uint32_t.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 11 ++++++-----
block/mirror.c | 4 ++--
include/block/block.h | 2 +-
include/block/block_int.h | 2 +-
qapi/block-core.json | 2 +-
5 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index 0da35ae..2b609e7 100644
--- a/block.c
+++ b/block.c
@@ -5456,12 +5456,13 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
}
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
- int granularity,
+ uint32_t granularity,
const char *name,
Error **errp)
{
int64_t bitmap_size;
BdrvDirtyBitmap *bitmap;
+ uint32_t sector_granularity;
assert((granularity & (granularity - 1)) == 0);
@@ -5469,8 +5470,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
error_setg(errp, "Bitmap already exists: %s", name);
return NULL;
}
- granularity >>= BDRV_SECTOR_BITS;
- assert(granularity);
+ sector_granularity = granularity >> BDRV_SECTOR_BITS;
+ assert(sector_granularity);
bitmap_size = bdrv_nb_sectors(bs);
if (bitmap_size < 0) {
error_setg_errno(errp, -bitmap_size, "could not get length of device");
@@ -5478,7 +5479,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
return NULL;
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
- bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+ bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
bitmap->name = g_strdup(name);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
@@ -5509,7 +5510,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
info->count = bdrv_get_dirty_count(bs, bm);
info->granularity =
- ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ ((uint32_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
entry->value = info;
diff --git a/block/mirror.c b/block/mirror.c
index f073ad7..f8aac33 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -656,7 +656,7 @@ static const BlockJobDriver commit_active_job_driver = {
static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
const char *replaces,
- int64_t speed, int64_t granularity,
+ int64_t speed, uint32_t granularity,
int64_t buf_size,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
@@ -717,7 +717,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
void mirror_start(BlockDriverState *bs, BlockDriverState *target,
const char *replaces,
- int64_t speed, int64_t granularity, int64_t buf_size,
+ int64_t speed, uint32_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb,
diff --git a/include/block/block.h b/include/block/block.h
index 05e32f9..5235086 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -450,7 +450,7 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
struct HBitmapIter;
typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
- int granularity,
+ uint32_t granularity,
const char *name,
Error **errp);
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dccb092..fb9e100 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -590,7 +590,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
*/
void mirror_start(BlockDriverState *bs, BlockDriverState *target,
const char *replaces,
- int64_t speed, int64_t granularity, int64_t buf_size,
+ int64_t speed, uint32_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c10e664..1bb23be 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -339,7 +339,7 @@
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} }
##
# @BlockInfo:
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 04/21] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (2 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 03/21] qmp: Ensure consistent granularity type John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 05/21] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
` (18 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
The new command pair is added to manage a user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.
The granularity is an optional field. If it is not specified, we will
choose a default granularity based on the cluster size if available,
clamped to between 4K and 64K to mirror how the 'mirror' code was
already choosing granularity. If we do not have cluster size info
available, we choose 64K. This code has been factored out into a helper
shared with block/mirror.
This patch also introduces the 'block_dirty_bitmap_lookup' helper,
which takes a device name and a dirty bitmap name and validates the
lookup, returning NULL and setting errp if there is a problem with
either field. This helper will be re-used in future patches in this
series.
The types added to block-core.json will be re-used in future patches
in this series, see:
'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 20 +++++++++
block/mirror.c | 10 +----
blockdev.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 1 +
qapi/block-core.json | 55 ++++++++++++++++++++++++
qmp-commands.hx | 56 ++++++++++++++++++++++++
6 files changed, 250 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index 2b609e7..ebea54d 100644
--- a/block.c
+++ b/block.c
@@ -5530,6 +5530,26 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector
}
}
+/**
+ * Chooses a default granularity based on the existing cluster size,
+ * but clamped between [4K, 64K]. Defaults to 64K in the case that there
+ * is no cluster size information available.
+ */
+uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
+{
+ BlockDriverInfo bdi;
+ uint32_t granularity;
+
+ if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size > 0) {
+ granularity = MAX(4096, bdi.cluster_size);
+ granularity = MIN(65536, granularity);
+ } else {
+ granularity = 65536;
+ }
+
+ return granularity;
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/block/mirror.c b/block/mirror.c
index f8aac33..1cb700e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -668,15 +668,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
MirrorBlockJob *s;
if (granularity == 0) {
- /* Choose the default granularity based on the target file's cluster
- * size, clamped between 4k and 64k. */
- BlockDriverInfo bdi;
- if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
- granularity = MAX(4096, bdi.cluster_size);
- granularity = MIN(65536, granularity);
- } else {
- granularity = 65536;
- }
+ granularity = bdrv_get_default_bitmap_granularity(target);
}
assert ((granularity & (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index fbb3a79..7d71b81 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1164,6 +1164,68 @@ out_aio_context:
return NULL;
}
+/**
+ * block_dirty_bitmap_lookup:
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names.
+ *
+ * @node: The name of the BDS node to search for bitmaps
+ * @name: The name of the bitmap to search for
+ * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
+ * @paio: Output pointer for aio_context acquisition, if desired. Can be NULL.
+ * @errp: Output pointer for error information. Can be NULL.
+ *
+ * @return: A bitmap object on success, or NULL on failure.
+ */
+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+ const char *name,
+ BlockDriverState **pbs,
+ AioContext **paio,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+ AioContext *aio_context;
+
+ if (!node) {
+ error_setg(errp, "Node cannot be NULL");
+ return NULL;
+ }
+ if (!name) {
+ error_setg(errp, "Bitmap name cannot be NULL");
+ return NULL;
+ }
+ bs = bdrv_lookup_bs(node, node, NULL);
+ if (!bs) {
+ error_setg(errp, "Node '%s' not found", node);
+ return NULL;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bitmap = bdrv_find_dirty_bitmap(bs, name);
+ if (!bitmap) {
+ error_setg(errp, "Dirty bitmap '%s' not found", name);
+ goto fail;
+ }
+
+ if (pbs) {
+ *pbs = bs;
+ }
+ if (paio) {
+ *paio = aio_context;
+ } else {
+ aio_context_release(aio_context);
+ }
+
+ return bitmap;
+
+ fail:
+ aio_context_release(aio_context);
+ return NULL;
+}
+
/* New and old BlockDriverState structs for atomic group operations */
typedef struct BlkTransactionState BlkTransactionState;
@@ -1953,6 +2015,61 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
aio_context_release(aio_context);
}
+void qmp_block_dirty_bitmap_add(const char *node, const char *name,
+ bool has_granularity, uint32_t granularity,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BlockDriverState *bs;
+
+ if (!name || name[0] == '\0') {
+ error_setg(errp, "Bitmap name cannot be empty");
+ return;
+ }
+
+ bs = bdrv_lookup_bs(node, node, errp);
+ if (!bs) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (has_granularity) {
+ if (granularity < 512 || !is_power_of_2(granularity)) {
+ error_setg(errp, "Granularity must be power of 2 "
+ "and at least 512");
+ goto out;
+ }
+ } else {
+ /* Default to cluster size, if available: */
+ granularity = bdrv_get_default_bitmap_granularity(bs);
+ }
+
+ bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+
+ out:
+ aio_context_release(aio_context);
+}
+
+void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
+ if (!bitmap || !bs) {
+ return;
+ }
+
+ bdrv_dirty_bitmap_make_anon(bs, bitmap);
+ bdrv_release_dirty_bitmap(bs, bitmap);
+
+ aio_context_release(aio_context);
+}
+
int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index 5235086..0f014a3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -458,6 +458,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
+uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1bb23be..1c03db1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -960,6 +960,61 @@
'*on-target-error': 'BlockdevOnError' } }
##
+# @BlockDirtyBitmap
+#
+# @node: name of device/node which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# Since 2.4
+##
+{ 'type': 'BlockDirtyBitmap',
+ 'data': { 'node': 'str', 'name': 'str' } }
+
+##
+# @BlockDirtyBitmapAdd
+#
+# @node: name of device/node which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# @granularity: #optional the bitmap granularity, default is 64k for
+# block-dirty-bitmap-add
+#
+# Since 2.4
+##
+{ 'type': 'BlockDirtyBitmapAdd',
+ 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32' } }
+
+##
+# @block-dirty-bitmap-add
+#
+# Create a dirty bitmap with a name on the node
+#
+# Returns: nothing on success
+# If @node is not a valid block device or node, DeviceNotFound
+# If @name is already taken, GenericError with an explanation
+#
+# Since 2.4
+##
+{ 'command': 'block-dirty-bitmap-add',
+ 'data': 'BlockDirtyBitmapAdd' }
+
+##
+# @block-dirty-bitmap-remove
+#
+# Remove a dirty bitmap on the node
+#
+# Returns: nothing on success
+# If @node is not a valid block device or node, DeviceNotFound
+# If @name is not found, GenericError with an explanation
+#
+# Since 2.4
+##
+{ 'command': 'block-dirty-bitmap-remove',
+ 'data': 'BlockDirtyBitmap' }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3a42ad0..b6bd455 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1270,6 +1270,62 @@ Example:
EQMP
{
+ .name = "block-dirty-bitmap-add",
+ .args_type = "node:B,name:s,granularity:i?",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_add,
+ },
+
+SQMP
+
+block-dirty-bitmap-add
+----------------------
+Since 2.4
+
+Create a dirty bitmap with a name on the device, and start tracking the writes.
+
+Arguments:
+
+- "node": device/node on which to create dirty bitmap (json-string)
+- "name": name of the new dirty bitmap (json-string)
+- "granularity": granularity to track writes with (int, optional)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-add", "arguments": { "node": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+EQMP
+
+ {
+ .name = "block-dirty-bitmap-remove",
+ .args_type = "node:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_remove,
+ },
+
+SQMP
+
+block-dirty-bitmap-remove
+-------------------------
+Since 2.4
+
+Stop write tracking and remove the dirty bitmap that was created with
+block-dirty-bitmap-add.
+
+Arguments:
+
+- "node": device/node on which to remove dirty bitmap (json-string)
+- "name": name of the dirty bitmap to remove (json-string)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-remove", "arguments": { "node": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "blockdev-snapshot-sync",
.args_type = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
.mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 05/21] block: Introduce bdrv_dirty_bitmap_granularity()
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (3 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 04/21] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 06/21] hbitmap: cache array lengths John Snow
` (17 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
This returns the granularity (in bytes) of dirty bitmap,
which matches the QMP interface and the existing query
interface.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 8 ++++++--
include/block/block.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index ebea54d..41c5a67 100644
--- a/block.c
+++ b/block.c
@@ -5509,8 +5509,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
info->count = bdrv_get_dirty_count(bs, bm);
- info->granularity =
- ((uint32_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ info->granularity = bdrv_dirty_bitmap_granularity(bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
entry->value = info;
@@ -5550,6 +5549,11 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
return granularity;
}
+uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
+{
+ return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/include/block/block.h b/include/block/block.h
index 0f014a3..493b7c5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -459,6 +459,7 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
+uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 06/21] hbitmap: cache array lengths
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (4 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 05/21] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-23 13:39 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge John Snow
` (16 subsequent siblings)
22 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
As a convenience: between incremental backups, bitmap migrations
and bitmap persistence we seem to need to recalculate these a lot.
Because the lengths are a little bit-twiddly, let's just solidly
cache them and be done with it.
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
util/hbitmap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/util/hbitmap.c b/util/hbitmap.c
index ab13971..5b78613 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -90,6 +90,9 @@ struct HBitmap {
* bitmap will still allocate HBITMAP_LEVELS arrays.
*/
unsigned long *levels[HBITMAP_LEVELS];
+
+ /* The length of each levels[] array. */
+ uint64_t sizes[HBITMAP_LEVELS];
};
/* Advance hbi to the next nonzero word and return it. hbi->pos
@@ -384,6 +387,7 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
hb->granularity = granularity;
for (i = HBITMAP_LEVELS; i-- > 0; ) {
size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+ hb->sizes[i] = size;
hb->levels[i] = g_new0(unsigned long, size);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH v6 06/21] hbitmap: cache array lengths
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 06/21] hbitmap: cache array lengths John Snow
@ 2015-04-23 13:39 ` Stefan Hajnoczi
0 siblings, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 13:39 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 574 bytes --]
On Fri, Apr 17, 2015 at 07:49:54PM -0400, John Snow wrote:
> As a convenience: between incremental backups, bitmap migrations
> and bitmap persistence we seem to need to recalculate these a lot.
>
> Because the lengths are a little bit-twiddly, let's just solidly
> cache them and be done with it.
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> util/hbitmap.c | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (5 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 06/21] hbitmap: cache array lengths John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-22 16:22 ` Max Reitz
` (2 more replies)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 08/21] block: Add bitmap disabled status John Snow
` (15 subsequent siblings)
22 siblings, 3 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
We add a bitmap merge operation to assist in error cases
where we wish to combine two bitmaps together.
This is algorithmically O(bits) provided HBITMAP_LEVELS remains
constant. For a full bitmap on a 64bit machine:
sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits
We may be able to improve running speed for particularly sparse
bitmaps by using iterators, but the running time for dense maps
will be worse.
We present the simpler solution first, and we can refine it later
if needed.
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/qemu/hbitmap.h | 13 +++++++++++++
util/hbitmap.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 550d7ce..6cb2d0e 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,19 @@ struct HBitmapIter {
HBitmap *hbitmap_alloc(uint64_t size, int granularity);
/**
+ * hbitmap_merge:
+ * @a: The bitmap to store the result in.
+ * @b: The bitmap to merge into @a.
+ * @return true if the merge was successful,
+ * false if it was not attempted.
+ *
+ * Merge two bitmaps together.
+ * A := A (BITOR) B.
+ * B is left unmodified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b);
+
+/**
* hbitmap_empty:
* @hb: HBitmap to operate on.
*
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 5b78613..150d6e9 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -399,3 +399,36 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
return hb;
}
+
+/**
+ * Given HBitmaps A and B, let A := A (BITOR) B.
+ * Bitmap B will not be modified.
+ *
+ * @return true if the merge was successful,
+ * false if it was not attempted.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b)
+{
+ int i;
+ uint64_t j;
+
+ if ((a->size != b->size) || (a->granularity != b->granularity)) {
+ return false;
+ }
+
+ if (hbitmap_count(b) == 0) {
+ return true;
+ }
+
+ /* This merge is O(size), as BITS_PER_LONG and HBITMAP_LEVELS are constant.
+ * It may be possible to improve running times for sparsely populated maps
+ * by using hbitmap_iter_next, but this is suboptimal for dense maps.
+ */
+ for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+ for (j = 0; j < a->sizes[i]; j++) {
+ a->levels[i][j] |= b->levels[i][j];
+ }
+ }
+
+ return true;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge John Snow
@ 2015-04-22 16:22 ` Max Reitz
2015-04-22 22:00 ` Eric Blake
2015-04-23 13:40 ` Stefan Hajnoczi
2 siblings, 0 replies; 52+ messages in thread
From: Max Reitz @ 2015-04-22 16:22 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha
On 18.04.2015 01:49, John Snow wrote:
> We add a bitmap merge operation to assist in error cases
> where we wish to combine two bitmaps together.
>
> This is algorithmically O(bits) provided HBITMAP_LEVELS remains
> constant. For a full bitmap on a 64bit machine:
> sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits
>
> We may be able to improve running speed for particularly sparse
> bitmaps by using iterators, but the running time for dense maps
> will be worse.
>
> We present the simpler solution first, and we can refine it later
> if needed.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> include/qemu/hbitmap.h | 13 +++++++++++++
> util/hbitmap.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge John Snow
2015-04-22 16:22 ` Max Reitz
@ 2015-04-22 22:00 ` Eric Blake
2015-04-23 13:40 ` Stefan Hajnoczi
2 siblings, 0 replies; 52+ messages in thread
From: Eric Blake @ 2015-04-22 22:00 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 941 bytes --]
On 04/17/2015 05:49 PM, John Snow wrote:
> We add a bitmap merge operation to assist in error cases
> where we wish to combine two bitmaps together.
>
> This is algorithmically O(bits) provided HBITMAP_LEVELS remains
> constant. For a full bitmap on a 64bit machine:
> sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits
>
> We may be able to improve running speed for particularly sparse
> bitmaps by using iterators, but the running time for dense maps
> will be worse.
>
> We present the simpler solution first, and we can refine it later
> if needed.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> include/qemu/hbitmap.h | 13 +++++++++++++
> util/hbitmap.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge John Snow
2015-04-22 16:22 ` Max Reitz
2015-04-22 22:00 ` Eric Blake
@ 2015-04-23 13:40 ` Stefan Hajnoczi
2 siblings, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 13:40 UTC (permalink / raw)
To: John Snow
Cc: kwolf, famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 855 bytes --]
On Fri, Apr 17, 2015 at 07:49:55PM -0400, John Snow wrote:
> We add a bitmap merge operation to assist in error cases
> where we wish to combine two bitmaps together.
>
> This is algorithmically O(bits) provided HBITMAP_LEVELS remains
> constant. For a full bitmap on a 64bit machine:
> sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits
>
> We may be able to improve running speed for particularly sparse
> bitmaps by using iterators, but the running time for dense maps
> will be worse.
>
> We present the simpler solution first, and we can refine it later
> if needed.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> include/qemu/hbitmap.h | 13 +++++++++++++
> util/hbitmap.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 08/21] block: Add bitmap disabled status
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (6 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 07/21] hbitmap: add hbitmap_merge John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 09/21] block: Add bitmap successors John Snow
` (14 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Add a status indicating the enabled/disabled state of the bitmap.
A bitmap is by default enabled, but you can lock the bitmap into
a read-only state by setting disabled = true.
A previous version of this patch added a QMP interface for changing
the state of the bitmap, but it has since been removed for now until
a use case emerges where this state must be revealed to the user.
The disabled state WILL be used internally for bitmap migration and
bitmap persistence.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 25 +++++++++++++++++++++++++
include/block/block.h | 3 +++
2 files changed, 28 insertions(+)
diff --git a/block.c b/block.c
index 41c5a67..db742a9 100644
--- a/block.c
+++ b/block.c
@@ -54,6 +54,7 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
char *name;
+ bool disabled;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5481,10 +5482,16 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
bitmap = g_new0(BdrvDirtyBitmap, 1);
bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
bitmap->name = g_strdup(name);
+ bitmap->disabled = false;
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
}
+bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
+{
+ return !bitmap->disabled;
+}
+
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
BdrvDirtyBitmap *bm, *next;
@@ -5499,6 +5506,16 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
}
}
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ bitmap->disabled = true;
+}
+
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ bitmap->disabled = false;
+}
+
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
{
BdrvDirtyBitmap *bm;
@@ -5563,12 +5580,14 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
{
+ assert(bdrv_dirty_bitmap_enabled(bitmap));
hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
}
void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
{
+ assert(bdrv_dirty_bitmap_enabled(bitmap));
hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
}
@@ -5577,6 +5596,9 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
{
BdrvDirtyBitmap *bitmap;
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+ if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+ continue;
+ }
hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
}
}
@@ -5586,6 +5608,9 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
{
BdrvDirtyBitmap *bitmap;
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+ if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+ continue;
+ }
hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
}
}
diff --git a/include/block/block.h b/include/block/block.h
index 493b7c5..029a8a7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -457,9 +457,12 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 09/21] block: Add bitmap successors
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (7 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 08/21] block: Add bitmap disabled status John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
` (13 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
A bitmap successor is an anonymous BdrvDirtyBitmap that is intended to
be created just prior to a sensitive operation (e.g. Incremental Backup)
that can either succeed or fail, but during the course of which we still
want a bitmap tracking writes.
On creating a successor, we "freeze" the parent bitmap which prevents
its deletion, enabling, anonymization, or creating a bitmap with the
same name.
On success, the parent bitmap can "abdicate" responsibility to the
successor, which will inherit its name. The successor will have been
tracking writes during the course of the backup operation. The parent
will be safely deleted.
On failure, we can "reclaim" the successor from the parent, unifying
them such that the resulting bitmap describes all writes occurring since
the last successful backup, for instance. Reclamation will thaw the
parent, but not explicitly re-enable it.
BdrvDirtyBitmap operations that target a single bitmap are protected
by assertions that the bitmap is not frozen and/or disabled.
BdrvDirtyBitmap operations that target a group of bitmaps, such as
bdrv_{set,reset}_dirty will ignore frozen/disabled drives with a
conditional instead.
Internal functions that enable/disable dirty bitmaps have assertions
added to them to prevent modifying frozen bitmaps.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++-
blockdev.c | 7 ++++
include/block/block.h | 10 +++++
qapi/block-core.json | 1 +
4 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index db742a9..8f08b6e 100644
--- a/block.c
+++ b/block.c
@@ -51,8 +51,17 @@
#include <windows.h>
#endif
+/**
+ * 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().
+ */
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ BdrvDirtyBitmap *successor;
char *name;
bool disabled;
QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -5452,6 +5461,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
+ assert(!bdrv_dirty_bitmap_frozen(bitmap));
g_free(bitmap->name);
bitmap->name = NULL;
}
@@ -5487,9 +5497,98 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
return bitmap;
}
+bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
+{
+ return bitmap->successor;
+}
+
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
{
- return !bitmap->disabled;
+ return !(bitmap->disabled || bitmap->successor);
+}
+
+/**
+ * Create a successor bitmap destined to replace this bitmap after an operation.
+ * Requires that the bitmap is not frozen and has no successor.
+ */
+int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, Error **errp)
+{
+ uint64_t granularity;
+ BdrvDirtyBitmap *child;
+
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ error_setg(errp, "Cannot create a successor for a bitmap that is "
+ "currently frozen");
+ return -1;
+ }
+ assert(!bitmap->successor);
+
+ /* Create an anonymous successor */
+ granularity = bdrv_dirty_bitmap_granularity(bitmap);
+ child = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
+ if (!child) {
+ return -1;
+ }
+
+ /* Successor will be on or off based on our current state. */
+ child->disabled = bitmap->disabled;
+
+ /* Install the successor and freeze the parent */
+ bitmap->successor = child;
+ return 0;
+}
+
+/**
+ * For a bitmap with a successor, yield our name to the successor,
+ * delete the old bitmap, and return a handle to the new bitmap.
+ */
+BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp)
+{
+ char *name;
+ BdrvDirtyBitmap *successor = bitmap->successor;
+
+ if (successor == NULL) {
+ error_setg(errp, "Cannot relinquish control if "
+ "there's no successor present");
+ return NULL;
+ }
+
+ name = bitmap->name;
+ bitmap->name = NULL;
+ successor->name = name;
+ bitmap->successor = NULL;
+ bdrv_release_dirty_bitmap(bs, bitmap);
+
+ return successor;
+}
+
+/**
+ * In cases of failure where we can no longer safely delete the parent,
+ * we may wish to re-join the parent and child/successor.
+ * The merged parent will be un-frozen, but not explicitly re-enabled.
+ */
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ Error **errp)
+{
+ BdrvDirtyBitmap *successor = parent->successor;
+
+ if (!successor) {
+ error_setg(errp, "Cannot reclaim a successor when none is present");
+ return NULL;
+ }
+
+ if (!hbitmap_merge(parent->bitmap, successor->bitmap)) {
+ error_setg(errp, "Merging of parent and successor bitmap failed");
+ return NULL;
+ }
+ bdrv_release_dirty_bitmap(bs, successor);
+ parent->successor = NULL;
+
+ return parent;
}
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
@@ -5497,6 +5596,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
BdrvDirtyBitmap *bm, *next;
QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
if (bm == bitmap) {
+ assert(!bdrv_dirty_bitmap_frozen(bm));
QLIST_REMOVE(bitmap, list);
hbitmap_free(bitmap->bitmap);
g_free(bitmap->name);
@@ -5508,11 +5608,13 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
{
+ assert(!bdrv_dirty_bitmap_frozen(bitmap));
bitmap->disabled = true;
}
void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
{
+ assert(!bdrv_dirty_bitmap_frozen(bitmap));
bitmap->disabled = false;
}
diff --git a/blockdev.c b/blockdev.c
index 7d71b81..373d19a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2064,9 +2064,16 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
return;
}
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently frozen and cannot be removed",
+ name);
+ goto out;
+ }
bdrv_dirty_bitmap_make_anon(bs, bitmap);
bdrv_release_dirty_bitmap(bs, bitmap);
+ out:
aio_context_release(aio_context);
}
diff --git a/include/block/block.h b/include/block/block.h
index 029a8a7..b02c670 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -453,6 +453,15 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
uint32_t granularity,
const char *name,
Error **errp);
+int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp);
+BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp);
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp);
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
@@ -463,6 +472,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1c03db1..710be68 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1008,6 +1008,7 @@
# Returns: nothing on success
# If @node is not a valid block device or node, DeviceNotFound
# If @name is not found, GenericError with an explanation
+# if @name is frozen by an operation, GenericError
#
# Since 2.4
##
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (8 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 09/21] block: Add bitmap successors John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-22 16:33 ` Max Reitz
` (2 more replies)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 11/21] qmp: add block-dirty-bitmap-clear John Snow
` (12 subsequent siblings)
22 siblings, 3 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
For "dirty-bitmap" sync mode, the block job will iterate through the
given dirty bitmap to decide if a sector needs backup (backup all the
dirty clusters and skip clean ones), just as allocation conditions of
"top" sync mode.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 9 +++
block/backup.c | 155 +++++++++++++++++++++++++++++++++++++++-------
block/mirror.c | 4 ++
blockdev.c | 18 +++++-
hmp.c | 3 +-
include/block/block.h | 1 +
include/block/block_int.h | 2 +
qapi/block-core.json | 14 +++--
qmp-commands.hx | 8 ++-
9 files changed, 181 insertions(+), 33 deletions(-)
diff --git a/block.c b/block.c
index 8f08b6e..185cd7f 100644
--- a/block.c
+++ b/block.c
@@ -5717,6 +5717,15 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
}
}
+/**
+ * Advance an HBitmapIter to an arbitrary offset.
+ */
+void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
+{
+ assert(hbi->hb);
+ hbitmap_iter_init(hbi, hbi->hb, offset);
+}
+
int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
return hbitmap_count(bitmap->bitmap);
diff --git a/block/backup.c b/block/backup.c
index 1c535b1..e77f7e8 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,8 @@ typedef struct CowRequest {
typedef struct BackupBlockJob {
BlockJob common;
BlockDriverState *target;
+ /* bitmap for sync=dirty-bitmap */
+ BdrvDirtyBitmap *sync_bitmap;
MirrorSyncMode sync_mode;
RateLimit limit;
BlockdevOnError on_source_error;
@@ -242,6 +244,91 @@ static void backup_complete(BlockJob *job, void *opaque)
g_free(data);
}
+static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+{
+ if (block_job_is_cancelled(&job->common)) {
+ return true;
+ }
+
+ /* we need to yield so that qemu_aio_flush() returns.
+ * (without, VM does not reboot)
+ */
+ if (job->common.speed) {
+ uint64_t delay_ns = ratelimit_calculate_delay(&job->limit,
+ job->sectors_read);
+ job->sectors_read = 0;
+ block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
+ } else {
+ block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
+ }
+
+ if (block_job_is_cancelled(&job->common)) {
+ return true;
+ }
+
+ return false;
+}
+
+static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+{
+ bool error_is_read;
+ int ret = 0;
+ int clusters_per_iter;
+ uint32_t granularity;
+ int64_t sector;
+ int64_t cluster;
+ int64_t end;
+ int64_t last_cluster = -1;
+ BlockDriverState *bs = job->common.bs;
+ HBitmapIter hbi;
+
+ granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
+ clusters_per_iter = MAX((granularity / BACKUP_CLUSTER_SIZE), 1);
+ bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+
+ /* Find the next dirty sector(s) */
+ while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+ cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
+
+ /* Fake progress updates for any clusters we skipped */
+ if (cluster != last_cluster + 1) {
+ job->common.offset += ((cluster - last_cluster - 1) *
+ BACKUP_CLUSTER_SIZE);
+ }
+
+ for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
+ do {
+ if (yield_and_check(job)) {
+ return ret;
+ }
+ ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
+ BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
+ if ((ret < 0) &&
+ backup_error_action(job, error_is_read, -ret) ==
+ BLOCK_ERROR_ACTION_REPORT) {
+ return ret;
+ }
+ } while (ret < 0);
+ }
+
+ /* If the bitmap granularity is smaller than the backup granularity,
+ * we need to advance the iterator pointer to the next cluster. */
+ if (granularity < BACKUP_CLUSTER_SIZE) {
+ bdrv_set_dirty_iter(&hbi, cluster * BACKUP_SECTORS_PER_CLUSTER);
+ }
+
+ last_cluster = cluster - 1;
+ }
+
+ /* Play some final catchup with the progress meter */
+ end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE);
+ if (last_cluster + 1 < end) {
+ job->common.offset += ((end - last_cluster - 1) * BACKUP_CLUSTER_SIZE);
+ }
+
+ return ret;
+}
+
static void coroutine_fn backup_run(void *opaque)
{
BackupBlockJob *job = opaque;
@@ -259,8 +346,7 @@ static void coroutine_fn backup_run(void *opaque)
qemu_co_rwlock_init(&job->flush_rwlock);
start = 0;
- end = DIV_ROUND_UP(job->common.len / BDRV_SECTOR_SIZE,
- BACKUP_SECTORS_PER_CLUSTER);
+ end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE);
job->bitmap = hbitmap_alloc(end, 0);
@@ -278,28 +364,13 @@ static void coroutine_fn backup_run(void *opaque)
qemu_coroutine_yield();
job->common.busy = true;
}
+ } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ ret = backup_run_incremental(job);
} else {
/* Both FULL and TOP SYNC_MODE's require copying.. */
for (; start < end; start++) {
bool error_is_read;
-
- if (block_job_is_cancelled(&job->common)) {
- break;
- }
-
- /* we need to yield so that qemu_aio_flush() returns.
- * (without, VM does not reboot)
- */
- if (job->common.speed) {
- uint64_t delay_ns = ratelimit_calculate_delay(
- &job->limit, job->sectors_read);
- job->sectors_read = 0;
- block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
- } else {
- block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
- }
-
- if (block_job_is_cancelled(&job->common)) {
+ if (yield_and_check(job)) {
break;
}
@@ -357,6 +428,18 @@ static void coroutine_fn backup_run(void *opaque)
qemu_co_rwlock_wrlock(&job->flush_rwlock);
qemu_co_rwlock_unlock(&job->flush_rwlock);
+ if (job->sync_bitmap) {
+ BdrvDirtyBitmap *bm;
+ if (ret < 0) {
+ /* Merge the successor back into the parent, delete nothing. */
+ bm = bdrv_reclaim_dirty_bitmap(bs, job->sync_bitmap, NULL);
+ assert(bm);
+ } else {
+ /* Everything is fine, delete this bitmap and install the backup. */
+ bm = bdrv_dirty_bitmap_abdicate(bs, job->sync_bitmap, NULL);
+ assert(bm);
+ }
+ }
hbitmap_free(job->bitmap);
bdrv_iostatus_disable(target);
@@ -369,6 +452,7 @@ static void coroutine_fn backup_run(void *opaque)
void backup_start(BlockDriverState *bs, BlockDriverState *target,
int64_t speed, MirrorSyncMode sync_mode,
+ BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
@@ -412,17 +496,36 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ if (sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ if (!sync_bitmap) {
+ error_setg(errp, "must provide a valid bitmap name for "
+ "\"dirty-bitmap\" sync mode");
+ return;
+ }
+
+ /* Create a new bitmap, and freeze/disable this one. */
+ if (bdrv_dirty_bitmap_create_successor(bs, sync_bitmap, errp) < 0) {
+ return;
+ }
+ } else if (sync_bitmap) {
+ error_setg(errp,
+ "a sync_bitmap was provided to backup_run, "
+ "but received an incompatible sync_mode (%s)",
+ MirrorSyncMode_lookup[sync_mode]);
+ return;
+ }
+
len = bdrv_getlength(bs);
if (len < 0) {
error_setg_errno(errp, -len, "unable to get length for '%s'",
bdrv_get_device_name(bs));
- return;
+ goto error;
}
BackupBlockJob *job = block_job_create(&backup_job_driver, bs, speed,
cb, opaque, errp);
if (!job) {
- return;
+ goto error;
}
bdrv_op_block_all(target, job->common.blocker);
@@ -431,7 +534,15 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
job->on_target_error = on_target_error;
job->target = target;
job->sync_mode = sync_mode;
+ job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP ?
+ sync_bitmap : NULL;
job->common.len = len;
job->common.co = qemu_coroutine_create(backup_run);
qemu_coroutine_enter(job->common.co, job);
+ return;
+
+ error:
+ if (sync_bitmap) {
+ bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
+ }
}
diff --git a/block/mirror.c b/block/mirror.c
index 1cb700e..f89eccf 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -718,6 +718,10 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
bool is_none_mode;
BlockDriverState *base;
+ if (mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ error_setg(errp, "Sync mode 'dirty-bitmap' not supported");
+ return;
+ }
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
mirror_start_job(bs, target, replaces,
diff --git a/blockdev.c b/blockdev.c
index 373d19a..90ba5b6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1584,6 +1584,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
backup->sync,
backup->has_mode, backup->mode,
backup->has_speed, backup->speed,
+ backup->has_bitmap, backup->bitmap,
backup->has_on_source_error, backup->on_source_error,
backup->has_on_target_error, backup->on_target_error,
&local_err);
@@ -2394,6 +2395,7 @@ void qmp_drive_backup(const char *device, const char *target,
enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
+ bool has_bitmap, const char *bitmap,
bool has_on_source_error, BlockdevOnError on_source_error,
bool has_on_target_error, BlockdevOnError on_target_error,
Error **errp)
@@ -2402,6 +2404,7 @@ void qmp_drive_backup(const char *device, const char *target,
BlockDriverState *bs;
BlockDriverState *target_bs;
BlockDriverState *source = NULL;
+ BdrvDirtyBitmap *bmap = NULL;
AioContext *aio_context;
BlockDriver *drv = NULL;
Error *local_err = NULL;
@@ -2501,7 +2504,16 @@ void qmp_drive_backup(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ if (has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, bitmap);
+ if (!bmap) {
+ error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+ goto out;
+ }
+ }
+
+ backup_start(bs, target_bs, speed, sync, bmap,
+ on_source_error, on_target_error,
block_job_cb, bs, &local_err);
if (local_err != NULL) {
bdrv_unref(target_bs);
@@ -2562,8 +2574,8 @@ void qmp_blockdev_backup(const char *device, const char *target,
bdrv_ref(target_bs);
bdrv_set_aio_context(target_bs, aio_context);
- backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
- block_job_cb, bs, &local_err);
+ backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
+ on_target_error, block_job_cb, bs, &local_err);
if (local_err != NULL) {
bdrv_unref(target_bs);
error_propagate(errp, local_err);
diff --git a/hmp.c b/hmp.c
index f142d36..a38874a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1062,7 +1062,8 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
qmp_drive_backup(device, filename, !!format, format,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
- true, mode, false, 0, false, 0, false, 0, &err);
+ true, mode, false, 0, false, NULL,
+ false, 0, false, 0, &err);
hmp_handle_error(mon, &err);
}
diff --git a/include/block/block.h b/include/block/block.h
index b02c670..80ac2cc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -480,6 +480,7 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
+void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index fb9e100..e0d5561 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -602,6 +602,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
* @target: Block device to write to.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
* @sync_mode: What parts of the disk image should be copied to the destination.
+ * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_DIRTY_BITMAP.
* @on_source_error: The action to take upon error reading from the source.
* @on_target_error: The action to take upon error writing to the target.
* @cb: Completion function for the job.
@@ -612,6 +613,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
*/
void backup_start(BlockDriverState *bs, BlockDriverState *target,
int64_t speed, MirrorSyncMode sync_mode,
+ BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 710be68..a4e2897 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -512,10 +512,12 @@
#
# @none: only copy data written from now on
#
+# @dirty-bitmap: only copy data described by the dirty bitmap. Since: 2.4
+#
# Since: 1.3
##
{ 'enum': 'MirrorSyncMode',
- 'data': ['top', 'full', 'none'] }
+ 'data': ['top', 'full', 'none', 'dirty-bitmap'] }
##
# @BlockJobType:
@@ -690,14 +692,18 @@
# probe if @mode is 'existing', else the format of the source
#
# @sync: what parts of the disk image should be copied to the destination
-# (all the disk, only the sectors allocated in the topmost image, or
-# only new I/O).
+# (all the disk, only the sectors allocated in the topmost image, from a
+# dirty bitmap, or only new I/O).
#
# @mode: #optional whether and how QEMU should create a new image, default is
# 'absolute-paths'.
#
# @speed: #optional the maximum speed, in bytes per second
#
+# @bitmap: #optional the name of dirty bitmap if sync is "dirty-bitmap".
+# Must be present if sync is "dirty-bitmap", must NOT be present
+# otherwise. (Since 2.4)
+#
# @on-source-error: #optional the action to take on an error on the source,
# default 'report'. 'stop' and 'enospc' can only be used
# if the block device supports io-status (see BlockInfo).
@@ -715,7 +721,7 @@
{ 'type': 'DriveBackup',
'data': { 'device': 'str', 'target': 'str', '*format': 'str',
'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
- '*speed': 'int',
+ '*speed': 'int', '*bitmap': 'str',
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b6bd455..eb54dcd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1074,7 +1074,7 @@ EQMP
{
.name = "drive-backup",
.args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "on-source-error:s?,on-target-error:s?",
+ "bitmap:s?,on-source-error:s?,on-target-error:s?",
.mhandler.cmd_new = qmp_marshal_input_drive_backup,
},
@@ -1101,8 +1101,10 @@ Arguments:
(json-string, optional)
- "sync": what parts of the disk image should be copied to the destination;
possibilities include "full" for all the disk, "top" for only the sectors
- allocated in the topmost image, or "none" to only replicate new I/O
- (MirrorSyncMode).
+ allocated in the topmost image, "dirty-bitmap" for only the dirty sectors in
+ the bitmap, or "none" to only replicate new I/O (MirrorSyncMode).
+- "bitmap": dirty bitmap name for sync==dirty-bitmap. Must be present if sync
+ is "dirty-bitmap", must NOT be present otherwise.
- "mode": whether and how QEMU should create a new image
(NewImageMode, optional, default 'absolute-paths')
- "speed": the maximum speed, in bytes per second (json-int, optional)
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
@ 2015-04-22 16:33 ` Max Reitz
2015-04-22 22:11 ` Eric Blake
2015-04-23 13:47 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2 siblings, 0 replies; 52+ messages in thread
From: Max Reitz @ 2015-04-22 16:33 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha
On 18.04.2015 01:49, John Snow wrote:
> For "dirty-bitmap" sync mode, the block job will iterate through the
> given dirty bitmap to decide if a sector needs backup (backup all the
> dirty clusters and skip clean ones), just as allocation conditions of
> "top" sync mode.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 9 +++
> block/backup.c | 155 +++++++++++++++++++++++++++++++++++++++-------
> block/mirror.c | 4 ++
> blockdev.c | 18 +++++-
> hmp.c | 3 +-
> include/block/block.h | 1 +
> include/block/block_int.h | 2 +
> qapi/block-core.json | 14 +++--
> qmp-commands.hx | 8 ++-
> 9 files changed, 181 insertions(+), 33 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
2015-04-22 16:33 ` Max Reitz
@ 2015-04-22 22:11 ` Eric Blake
2015-04-23 13:47 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2 siblings, 0 replies; 52+ messages in thread
From: Eric Blake @ 2015-04-22 22:11 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 542 bytes --]
On 04/17/2015 05:49 PM, John Snow wrote:
> For "dirty-bitmap" sync mode, the block job will iterate through the
> given dirty bitmap to decide if a sector needs backup (backup all the
> dirty clusters and skip clean ones), just as allocation conditions of
> "top" sync mode.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
2015-04-22 16:33 ` Max Reitz
2015-04-22 22:11 ` Eric Blake
@ 2015-04-23 13:47 ` Stefan Hajnoczi
2 siblings, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 13:47 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 914 bytes --]
On Fri, Apr 17, 2015 at 07:49:58PM -0400, John Snow wrote:
> For "dirty-bitmap" sync mode, the block job will iterate through the
> given dirty bitmap to decide if a sector needs backup (backup all the
> dirty clusters and skip clean ones), just as allocation conditions of
> "top" sync mode.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 9 +++
> block/backup.c | 155 +++++++++++++++++++++++++++++++++++++++-------
> block/mirror.c | 4 ++
> blockdev.c | 18 +++++-
> hmp.c | 3 +-
> include/block/block.h | 1 +
> include/block/block_int.h | 2 +
> qapi/block-core.json | 14 +++--
> qmp-commands.hx | 8 ++-
> 9 files changed, 181 insertions(+), 33 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 11/21] qmp: add block-dirty-bitmap-clear
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (9 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
@ 2015-04-17 23:49 ` John Snow
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block John Snow
` (11 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:49 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Add bdrv_clear_dirty_bitmap and a matching QMP command,
qmp_block_dirty_bitmap_clear that enables a user to reset
the bitmap attached to a drive.
This allows us to reset a bitmap in the event of a full
drive backup.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 8 ++++++++
blockdev.c | 34 ++++++++++++++++++++++++++++++++++
include/block/block.h | 1 +
qapi/block-core.json | 14 ++++++++++++++
qmp-commands.hx | 29 +++++++++++++++++++++++++++++
5 files changed, 86 insertions(+)
diff --git a/block.c b/block.c
index 185cd7f..679991b 100644
--- a/block.c
+++ b/block.c
@@ -62,6 +62,7 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
BdrvDirtyBitmap *successor;
+ int64_t size;
char *name;
bool disabled;
QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -5491,6 +5492,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
+ bitmap->size = bitmap_size;
bitmap->name = g_strdup(name);
bitmap->disabled = false;
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
@@ -5693,6 +5695,12 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
}
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ assert(bdrv_dirty_bitmap_enabled(bitmap));
+ hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
+}
+
static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors)
{
diff --git a/blockdev.c b/blockdev.c
index 90ba5b6..df96959 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2078,6 +2078,40 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
aio_context_release(aio_context);
}
+/**
+ * Completely clear a bitmap, for the purposes of synchronizing a bitmap
+ * immediately after a full backup operation.
+ */
+void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+
+ bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
+ if (!bitmap || !bs) {
+ return;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently frozen and cannot be modified",
+ name);
+ goto out;
+ } else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently disabled and cannot be cleared",
+ name);
+ goto out;
+ }
+
+ bdrv_clear_dirty_bitmap(bitmap);
+
+ out:
+ aio_context_release(aio_context);
+}
+
int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index 80ac2cc..0961b1e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -478,6 +478,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a4e2897..fc9ca04 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1022,6 +1022,20 @@
'data': 'BlockDirtyBitmap' }
##
+# @block-dirty-bitmap-clear
+#
+# Clear (reset) a dirty bitmap on the device
+#
+# Returns: nothing on success
+# If @node is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explanation
+#
+# Since 2.4
+##
+{ 'command': 'block-dirty-bitmap-clear',
+ 'data': 'BlockDirtyBitmap' }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index eb54dcd..e7db3a3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1328,6 +1328,35 @@ Example:
EQMP
{
+ .name = "block-dirty-bitmap-clear",
+ .args_type = "node:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_clear,
+ },
+
+SQMP
+
+block-dirty-bitmap-clear
+------------------------
+Since 2.4
+
+Reset the dirty bitmap associated with a node so that an incremental backup
+from this point in time forward will only backup clusters modified after this
+clear operation.
+
+Arguments:
+
+- "node": device/node on which to remove dirty bitmap (json-string)
+- "name": name of the dirty bitmap to remove (json-string)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-clear", "arguments": { "node": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "blockdev-snapshot-sync",
.args_type = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
.mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (10 preceding siblings ...)
2015-04-17 23:49 ` [Qemu-devel] [PATCH v6 11/21] qmp: add block-dirty-bitmap-clear John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-22 22:18 ` Eric Blake
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 13/21] block: add BdrvDirtyBitmap documentation John Snow
` (10 subsequent siblings)
22 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Add the "frozen" status booleans, to inform clients
when a bitmap is occupied doing a task.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 1 +
qapi/block-core.json | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 679991b..ca73a6a 100644
--- a/block.c
+++ b/block.c
@@ -5633,6 +5633,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->granularity = bdrv_dirty_bitmap_granularity(bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
+ info->frozen = bdrv_dirty_bitmap_frozen(bm);
entry->value = info;
*plist = entry;
plist = &entry->next;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index fc9ca04..cae995b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -336,10 +336,13 @@
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
#
+# @frozen: whether the dirty bitmap is frozen (Since 2.4)
+#
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
+ 'frozen': 'bool'} }
##
# @BlockInfo:
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block John Snow
@ 2015-04-22 22:18 ` Eric Blake
2015-04-22 22:22 ` John Snow
0 siblings, 1 reply; 52+ messages in thread
From: Eric Blake @ 2015-04-22 22:18 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]
On 04/17/2015 05:50 PM, John Snow wrote:
> Add the "frozen" status booleans, to inform clients
> when a bitmap is occupied doing a task.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block.c | 1 +
> qapi/block-core.json | 5 ++++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
> +++ b/qapi/block-core.json
> @@ -336,10 +336,13 @@
> #
> # @granularity: granularity of the dirty bitmap in bytes (since 1.4)
> #
> +# @frozen: whether the dirty bitmap is frozen (Since 2.4)
> +#
> # Since: 1.3
> ##
> { 'type': 'BlockDirtyInfo',
> - 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} }
> + 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
> + 'frozen': 'bool'} }
Just thinking aloud here - internally, we have a tri-state situation
(enabled, disabled, or frozen). I know we aren't exposing disabled to
the end user yet (no use case for that), but would it be better to make
this output an enum type (two values for now, 'enabled' and 'frozen') to
make it easier to add a third value later, without having to add yet
another boolean?
But it's not the end of the world to expose a single boolean now (we'd
just have to maintain it forever, even if we add later states), and
adding an enum now just adds complexity that we may not need, so:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block
2015-04-22 22:18 ` Eric Blake
@ 2015-04-22 22:22 ` John Snow
0 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-22 22:22 UTC (permalink / raw)
To: Eric Blake, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha, mreitz
On 04/22/2015 06:18 PM, Eric Blake wrote:
> On 04/17/2015 05:50 PM, John Snow wrote:
>> Add the "frozen" status booleans, to inform clients
>> when a bitmap is occupied doing a task.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> block.c | 1 +
>> qapi/block-core.json | 5 ++++-
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>
>> +++ b/qapi/block-core.json
>> @@ -336,10 +336,13 @@
>> #
>> # @granularity: granularity of the dirty bitmap in bytes (since 1.4)
>> #
>> +# @frozen: whether the dirty bitmap is frozen (Since 2.4)
>> +#
>> # Since: 1.3
>> ##
>> { 'type': 'BlockDirtyInfo',
>> - 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} }
>> + 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
>> + 'frozen': 'bool'} }
>
> Just thinking aloud here - internally, we have a tri-state situation
> (enabled, disabled, or frozen). I know we aren't exposing disabled to
> the end user yet (no use case for that), but would it be better to make
> this output an enum type (two values for now, 'enabled' and 'frozen') to
> make it easier to add a third value later, without having to add yet
> another boolean?
>
> But it's not the end of the world to expose a single boolean now (we'd
> just have to maintain it forever, even if we add later states), and
> adding an enum now just adds complexity that we may not need, so:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Hmm..., you have a point!
If there are no further objections to this series as-is, I will author a
quick follow-up patch to implement that -- possibly when we merge in the
bitmap migration code.
--js
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 13/21] block: add BdrvDirtyBitmap documentation
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (11 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 12/21] qmp: Add dirty bitmap status field in query-block John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 14/21] block: Ensure consistent bitmap function prototypes John Snow
` (9 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index ca73a6a..30c8568 100644
--- a/block.c
+++ b/block.c
@@ -60,11 +60,11 @@
* or enabled. A frozen bitmap can only abdicate() or reclaim().
*/
struct BdrvDirtyBitmap {
- HBitmap *bitmap;
- BdrvDirtyBitmap *successor;
- int64_t size;
- char *name;
- bool disabled;
+ HBitmap *bitmap; /* Dirty sector bitmap implementation */
+ BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
+ char *name; /* Optional non-empty unique ID */
+ int64_t size; /* Size of the bitmap (Number of sectors) */
+ bool disabled; /* Bitmap is read-only */
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 14/21] block: Ensure consistent bitmap function prototypes
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (12 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 13/21] block: add BdrvDirtyBitmap documentation John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate John Snow
` (8 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
We often don't need the BlockDriverState for functions
that operate on bitmaps. Remove it.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 13 ++++++-------
block/backup.c | 2 +-
block/mirror.c | 26 ++++++++++----------------
blockdev.c | 2 +-
include/block/block.h | 11 +++++------
migration/block.c | 7 +++----
6 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/block.c b/block.c
index 30c8568..735acff 100644
--- a/block.c
+++ b/block.c
@@ -5460,7 +5460,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
return NULL;
}
-void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
{
assert(!bdrv_dirty_bitmap_frozen(bitmap));
g_free(bitmap->name);
@@ -5629,7 +5629,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
- info->count = bdrv_get_dirty_count(bs, bm);
+ info->count = bdrv_get_dirty_count(bm);
info->granularity = bdrv_dirty_bitmap_granularity(bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
@@ -5676,20 +5676,19 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
}
-void bdrv_dirty_iter_init(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
+void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
hbitmap_iter_init(hbi, bitmap->bitmap, 0);
}
-void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
{
assert(bdrv_dirty_bitmap_enabled(bitmap));
hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
}
-void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
{
assert(bdrv_dirty_bitmap_enabled(bitmap));
@@ -5735,7 +5734,7 @@ void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
hbitmap_iter_init(hbi, hbi->hb, offset);
}
-int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
{
return hbitmap_count(bitmap->bitmap);
}
diff --git a/block/backup.c b/block/backup.c
index e77f7e8..a297df6 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -284,7 +284,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
clusters_per_iter = MAX((granularity / BACKUP_CLUSTER_SIZE), 1);
- bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+ bdrv_dirty_iter_init(job->sync_bitmap, &hbi);
/* Find the next dirty sector(s) */
while ((sector = hbitmap_iter_next(&hbi)) != -1) {
diff --git a/block/mirror.c b/block/mirror.c
index f89eccf..dcd6f65 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -125,11 +125,9 @@ static void mirror_write_complete(void *opaque, int ret)
MirrorOp *op = opaque;
MirrorBlockJob *s = op->s;
if (ret < 0) {
- BlockDriverState *source = s->common.bs;
BlockErrorAction action;
- bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
- op->nb_sectors);
+ bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors);
action = mirror_error_action(s, false, -ret);
if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
s->ret = ret;
@@ -143,11 +141,9 @@ static void mirror_read_complete(void *opaque, int ret)
MirrorOp *op = opaque;
MirrorBlockJob *s = op->s;
if (ret < 0) {
- BlockDriverState *source = s->common.bs;
BlockErrorAction action;
- bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
- op->nb_sectors);
+ bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors);
action = mirror_error_action(s, true, -ret);
if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
s->ret = ret;
@@ -170,10 +166,9 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
s->sector_num = hbitmap_iter_next(&s->hbi);
if (s->sector_num < 0) {
- bdrv_dirty_iter_init(source, s->dirty_bitmap, &s->hbi);
+ bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
s->sector_num = hbitmap_iter_next(&s->hbi);
- trace_mirror_restart_iter(s,
- bdrv_get_dirty_count(source, s->dirty_bitmap));
+ trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
assert(s->sector_num >= 0);
}
@@ -288,8 +283,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
next_sector += sectors_per_chunk;
}
- bdrv_reset_dirty_bitmap(source, s->dirty_bitmap, sector_num,
- nb_sectors);
+ bdrv_reset_dirty_bitmap(s->dirty_bitmap, sector_num, nb_sectors);
/* Copy the dirty cluster. */
s->in_flight++;
@@ -446,7 +440,7 @@ static void coroutine_fn mirror_run(void *opaque)
assert(n > 0);
if (ret == 1) {
- bdrv_set_dirty_bitmap(bs, s->dirty_bitmap, sector_num, n);
+ bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
sector_num = next;
} else {
sector_num += n;
@@ -454,7 +448,7 @@ static void coroutine_fn mirror_run(void *opaque)
}
}
- bdrv_dirty_iter_init(bs, s->dirty_bitmap, &s->hbi);
+ bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
for (;;) {
uint64_t delay_ns = 0;
@@ -466,7 +460,7 @@ static void coroutine_fn mirror_run(void *opaque)
goto immediate_exit;
}
- cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap);
+ cnt = bdrv_get_dirty_count(s->dirty_bitmap);
/* s->common.offset contains the number of bytes already processed so
* far, cnt is the number of dirty sectors remaining and
* s->sectors_in_flight is the number of sectors currently being
@@ -516,7 +510,7 @@ static void coroutine_fn mirror_run(void *opaque)
should_complete = s->should_complete ||
block_job_is_cancelled(&s->common);
- cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap);
+ cnt = bdrv_get_dirty_count(s->dirty_bitmap);
}
}
@@ -531,7 +525,7 @@ static void coroutine_fn mirror_run(void *opaque)
*/
trace_mirror_before_drain(s, cnt);
bdrv_drain(bs);
- cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap);
+ cnt = bdrv_get_dirty_count(s->dirty_bitmap);
}
ret = 0;
diff --git a/blockdev.c b/blockdev.c
index df96959..b9c79ed 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2071,7 +2071,7 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
name);
goto out;
}
- bdrv_dirty_bitmap_make_anon(bs, bitmap);
+ bdrv_dirty_bitmap_make_anon(bitmap);
bdrv_release_dirty_bitmap(bs, bitmap);
out:
diff --git a/include/block/block.h b/include/block/block.h
index 0961b1e..36bdf04 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -464,7 +464,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
Error **errp);
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
-void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
@@ -474,15 +474,14 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
-void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
-void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
-void bdrv_dirty_iter_init(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
+void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
-int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
void bdrv_disable_copy_on_read(BlockDriverState *bs);
diff --git a/migration/block.c b/migration/block.c
index 02a7d26..ddb59cc 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -304,7 +304,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds)
blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
nr_sectors, blk_mig_read_cb, blk);
- bdrv_reset_dirty_bitmap(bs, bmds->dirty_bitmap, cur_sector, nr_sectors);
+ bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector, nr_sectors);
qemu_mutex_unlock_iothread();
bmds->cur_sector = cur_sector + nr_sectors;
@@ -497,8 +497,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
g_free(blk);
}
- bdrv_reset_dirty_bitmap(bmds->bs, bmds->dirty_bitmap, sector,
- nr_sectors);
+ bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
break;
}
sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
@@ -584,7 +583,7 @@ static int64_t get_remaining_dirty(void)
int64_t dirty = 0;
QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
- dirty += bdrv_get_dirty_count(bmds->bs, bmds->dirty_bitmap);
+ dirty += bdrv_get_dirty_count(bmds->dirty_bitmap);
}
return dirty << BDRV_SECTOR_BITS;
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (13 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 14/21] block: Ensure consistent bitmap function prototypes John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-22 16:35 ` Max Reitz
2015-04-23 13:51 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 16/21] hbitmap: truncate tests John Snow
` (7 subsequent siblings)
22 siblings, 2 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 18 ++++++++++++++++++
include/qemu/hbitmap.h | 10 ++++++++++
util/hbitmap.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+)
diff --git a/block.c b/block.c
index 735acff..b29aafe 100644
--- a/block.c
+++ b/block.c
@@ -113,6 +113,7 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors);
static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors);
+static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@@ -3583,6 +3584,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
ret = drv->bdrv_truncate(bs, offset);
if (ret == 0) {
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+ bdrv_dirty_bitmap_truncate(bs);
if (bs->blk) {
blk_dev_resize_cb(bs->blk);
}
@@ -5593,6 +5595,22 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
return parent;
}
+/**
+ * Truncates _all_ bitmaps attached to a BDS.
+ */
+static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
+{
+ BdrvDirtyBitmap *bitmap;
+ uint64_t size = bdrv_nb_sectors(bs);
+
+ QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ continue;
+ }
+ hbitmap_truncate(bitmap->bitmap, size);
+ }
+}
+
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
BdrvDirtyBitmap *bm, *next;
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 6cb2d0e..f0a85f8 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,16 @@ struct HBitmapIter {
HBitmap *hbitmap_alloc(uint64_t size, int granularity);
/**
+ * hbitmap_truncate:
+ * @hb: The bitmap to change the size of.
+ * @size: The number of elements to change the bitmap to accommodate.
+ *
+ * truncate or grow an existing bitmap to accommodate a new number of elements.
+ * This may invalidate existing HBitmapIterators.
+ */
+void hbitmap_truncate(HBitmap *hb, uint64_t size);
+
+/**
* hbitmap_merge:
* @a: The bitmap to store the result in.
* @b: The bitmap to merge into @a.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 150d6e9..a10c7ae 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -400,6 +400,54 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
return hb;
}
+void hbitmap_truncate(HBitmap *hb, uint64_t size)
+{
+ bool shrink;
+ unsigned i;
+ uint64_t num_elements = size;
+ uint64_t old;
+
+ /* Size comes in as logical elements, adjust for granularity. */
+ size = (size + (1ULL << hb->granularity) - 1) >> hb->granularity;
+ assert(size <= ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE));
+ shrink = size < hb->size;
+
+ /* bit sizes are identical; nothing to do. */
+ if (size == hb->size) {
+ return;
+ }
+
+ /* If we're losing bits, let's clear those bits before we invalidate all of
+ * our invariants. This helps keep the bitcount consistent, and will prevent
+ * us from carrying around garbage bits beyond the end of the map.
+ */
+ if (shrink) {
+ /* Don't clear partial granularity groups;
+ * start at the first full one. */
+ uint64_t start = QEMU_ALIGN_UP(num_elements, 1 << hb->granularity);
+ uint64_t fix_count = (hb->size << hb->granularity) - start;
+
+ assert(fix_count);
+ hbitmap_reset(hb, start, fix_count);
+ }
+
+ hb->size = size;
+ for (i = HBITMAP_LEVELS; i-- > 0; ) {
+ size = MAX(BITS_TO_LONGS(size), 1);
+ if (hb->sizes[i] == size) {
+ break;
+ }
+ old = hb->sizes[i];
+ hb->sizes[i] = size;
+ hb->levels[i] = g_realloc(hb->levels[i], size * sizeof(unsigned long));
+ if (!shrink) {
+ memset(&hb->levels[i][old], 0x00,
+ (size - old) * sizeof(*hb->levels[i]));
+ }
+ }
+}
+
+
/**
* Given HBitmaps A and B, let A := A (BITOR) B.
* Bitmap B will not be modified.
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate John Snow
@ 2015-04-22 16:35 ` Max Reitz
2015-04-23 13:51 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
1 sibling, 0 replies; 52+ messages in thread
From: Max Reitz @ 2015-04-22 16:35 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha
On 18.04.2015 01:50, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 18 ++++++++++++++++++
> include/qemu/hbitmap.h | 10 ++++++++++
> util/hbitmap.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate John Snow
2015-04-22 16:35 ` Max Reitz
@ 2015-04-23 13:51 ` Stefan Hajnoczi
1 sibling, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 13:51 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 372 bytes --]
On Fri, Apr 17, 2015 at 07:50:03PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 18 ++++++++++++++++++
> include/qemu/hbitmap.h | 10 ++++++++++
> util/hbitmap.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 16/21] hbitmap: truncate tests
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (14 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 15/21] block: Resize bitmaps on bdrv_truncate John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 17/21] iotests: add invalid input incremental backup tests John Snow
` (6 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
The general approach is to set bits close to the boundaries of
where we are truncating and ensure that everything appears to
have gone OK.
We test growing and shrinking by different amounts:
- Less than the granularity
- Less than the granularity, but across a boundary
- Less than sizeof(unsigned long)
- Less than sizeof(unsigned long), but across a ulong boundary
- More than sizeof(unsigned long)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/test-hbitmap.c | 255 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 255 insertions(+)
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 8c902f2..9f41b5f 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -11,6 +11,8 @@
#include <glib.h>
#include <stdarg.h>
+#include <string.h>
+#include <sys/types.h>
#include "qemu/hbitmap.h"
#define LOG_BITS_PER_LONG (BITS_PER_LONG == 32 ? 5 : 6)
@@ -23,6 +25,7 @@ typedef struct TestHBitmapData {
HBitmap *hb;
unsigned long *bits;
size_t size;
+ size_t old_size;
int granularity;
} TestHBitmapData;
@@ -91,6 +94,44 @@ static void hbitmap_test_init(TestHBitmapData *data,
}
}
+static inline size_t hbitmap_test_array_size(size_t bits)
+{
+ size_t n = (bits + BITS_PER_LONG - 1) / BITS_PER_LONG;
+ return n ? n : 1;
+}
+
+static void hbitmap_test_truncate_impl(TestHBitmapData *data,
+ size_t size)
+{
+ size_t n;
+ size_t m;
+ data->old_size = data->size;
+ data->size = size;
+
+ if (data->size == data->old_size) {
+ return;
+ }
+
+ n = hbitmap_test_array_size(size);
+ m = hbitmap_test_array_size(data->old_size);
+ data->bits = g_realloc(data->bits, sizeof(unsigned long) * n);
+ if (n > m) {
+ memset(&data->bits[m], 0x00, sizeof(unsigned long) * (n - m));
+ }
+
+ /* If we shrink to an uneven multiple of sizeof(unsigned long),
+ * scrub the leftover memory. */
+ if (data->size < data->old_size) {
+ m = size % (sizeof(unsigned long) * 8);
+ if (m) {
+ unsigned long mask = (1ULL << m) - 1;
+ data->bits[n-1] &= mask;
+ }
+ }
+
+ hbitmap_truncate(data->hb, size);
+}
+
static void hbitmap_test_teardown(TestHBitmapData *data,
const void *unused)
{
@@ -369,6 +410,198 @@ static void test_hbitmap_iter_granularity(TestHBitmapData *data,
g_assert_cmpint(hbitmap_iter_next(&hbi), <, 0);
}
+static void hbitmap_test_set_boundary_bits(TestHBitmapData *data, ssize_t diff)
+{
+ size_t size = data->size;
+
+ /* First bit */
+ hbitmap_test_set(data, 0, 1);
+ if (diff < 0) {
+ /* Last bit in new, shortened map */
+ hbitmap_test_set(data, size + diff - 1, 1);
+
+ /* First bit to be truncated away */
+ hbitmap_test_set(data, size + diff, 1);
+ }
+ /* Last bit */
+ hbitmap_test_set(data, size - 1, 1);
+ if (data->granularity == 0) {
+ hbitmap_test_check_get(data);
+ }
+}
+
+static void hbitmap_test_check_boundary_bits(TestHBitmapData *data)
+{
+ size_t size = MIN(data->size, data->old_size);
+
+ if (data->granularity == 0) {
+ hbitmap_test_check_get(data);
+ hbitmap_test_check(data, 0);
+ } else {
+ /* If a granularity was set, note that every distinct
+ * (bit >> granularity) value that was set will increase
+ * the bit pop count by 2^granularity, not just 1.
+ *
+ * The hbitmap_test_check facility does not currently tolerate
+ * non-zero granularities, so test the boundaries and the population
+ * count manually.
+ */
+ g_assert(hbitmap_get(data->hb, 0));
+ g_assert(hbitmap_get(data->hb, size - 1));
+ g_assert_cmpint(2 << data->granularity, ==, hbitmap_count(data->hb));
+ }
+}
+
+/* Generic truncate test. */
+static void hbitmap_test_truncate(TestHBitmapData *data,
+ size_t size,
+ ssize_t diff,
+ int granularity)
+{
+ hbitmap_test_init(data, size, granularity);
+ hbitmap_test_set_boundary_bits(data, diff);
+ hbitmap_test_truncate_impl(data, size + diff);
+ hbitmap_test_check_boundary_bits(data);
+}
+
+static void test_hbitmap_truncate_nop(TestHBitmapData *data,
+ const void *unused)
+{
+ hbitmap_test_truncate(data, L2, 0, 0);
+}
+
+/**
+ * Grow by an amount smaller than the granularity, without crossing
+ * a granularity alignment boundary. Effectively a NOP.
+ */
+static void test_hbitmap_truncate_grow_negligible(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2 - 1;
+ size_t diff = 1;
+ int granularity = 1;
+
+ hbitmap_test_truncate(data, size, diff, granularity);
+}
+
+/**
+ * Shrink by an amount smaller than the granularity, without crossing
+ * a granularity alignment boundary. Effectively a NOP.
+ */
+static void test_hbitmap_truncate_shrink_negligible(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2;
+ ssize_t diff = -1;
+ int granularity = 1;
+
+ hbitmap_test_truncate(data, size, diff, granularity);
+}
+
+/**
+ * Grow by an amount smaller than the granularity, but crossing over
+ * a granularity alignment boundary.
+ */
+static void test_hbitmap_truncate_grow_tiny(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2 - 2;
+ ssize_t diff = 1;
+ int granularity = 1;
+
+ hbitmap_test_truncate(data, size, diff, granularity);
+}
+
+/**
+ * Shrink by an amount smaller than the granularity, but crossing over
+ * a granularity alignment boundary.
+ */
+static void test_hbitmap_truncate_shrink_tiny(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2 - 1;
+ ssize_t diff = -1;
+ int granularity = 1;
+
+ hbitmap_test_truncate(data, size, diff, granularity);
+}
+
+/**
+ * Grow by an amount smaller than sizeof(long), and not crossing over
+ * a sizeof(long) alignment boundary.
+ */
+static void test_hbitmap_truncate_grow_small(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2 + 1;
+ size_t diff = sizeof(long) / 2;
+
+ hbitmap_test_truncate(data, size, diff, 0);
+}
+
+/**
+ * Shrink by an amount smaller than sizeof(long), and not crossing over
+ * a sizeof(long) alignment boundary.
+ */
+static void test_hbitmap_truncate_shrink_small(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2;
+ size_t diff = sizeof(long) / 2;
+
+ hbitmap_test_truncate(data, size, -diff, 0);
+}
+
+/**
+ * Grow by an amount smaller than sizeof(long), while crossing over
+ * a sizeof(long) alignment boundary.
+ */
+static void test_hbitmap_truncate_grow_medium(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2 - 1;
+ size_t diff = sizeof(long) / 2;
+
+ hbitmap_test_truncate(data, size, diff, 0);
+}
+
+/**
+ * Shrink by an amount smaller than sizeof(long), while crossing over
+ * a sizeof(long) alignment boundary.
+ */
+static void test_hbitmap_truncate_shrink_medium(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2 + 1;
+ size_t diff = sizeof(long) / 2;
+
+ hbitmap_test_truncate(data, size, -diff, 0);
+}
+
+/**
+ * Grow by an amount larger than sizeof(long).
+ */
+static void test_hbitmap_truncate_grow_large(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2;
+ size_t diff = 8 * sizeof(long);
+
+ hbitmap_test_truncate(data, size, diff, 0);
+}
+
+/**
+ * Shrink by an amount larger than sizeof(long).
+ */
+static void test_hbitmap_truncate_shrink_large(TestHBitmapData *data,
+ const void *unused)
+{
+ size_t size = L2;
+ size_t diff = 8 * sizeof(long);
+
+ hbitmap_test_truncate(data, size, -diff, 0);
+}
+
static void hbitmap_test_add(const char *testpath,
void (*test_func)(TestHBitmapData *data, const void *user_data))
{
@@ -395,6 +628,28 @@ int main(int argc, char **argv)
hbitmap_test_add("/hbitmap/reset/empty", test_hbitmap_reset_empty);
hbitmap_test_add("/hbitmap/reset/general", test_hbitmap_reset);
hbitmap_test_add("/hbitmap/granularity", test_hbitmap_granularity);
+
+ hbitmap_test_add("/hbitmap/truncate/nop", test_hbitmap_truncate_nop);
+ hbitmap_test_add("/hbitmap/truncate/grow/negligible",
+ test_hbitmap_truncate_grow_negligible);
+ hbitmap_test_add("/hbitmap/truncate/shrink/negligible",
+ test_hbitmap_truncate_shrink_negligible);
+ hbitmap_test_add("/hbitmap/truncate/grow/tiny",
+ test_hbitmap_truncate_grow_tiny);
+ hbitmap_test_add("/hbitmap/truncate/shrink/tiny",
+ test_hbitmap_truncate_shrink_tiny);
+ hbitmap_test_add("/hbitmap/truncate/grow/small",
+ test_hbitmap_truncate_grow_small);
+ hbitmap_test_add("/hbitmap/truncate/shrink/small",
+ test_hbitmap_truncate_shrink_small);
+ hbitmap_test_add("/hbitmap/truncate/grow/medium",
+ test_hbitmap_truncate_grow_medium);
+ hbitmap_test_add("/hbitmap/truncate/shrink/medium",
+ test_hbitmap_truncate_shrink_medium);
+ hbitmap_test_add("/hbitmap/truncate/grow/large",
+ test_hbitmap_truncate_grow_large);
+ hbitmap_test_add("/hbitmap/truncate/shrink/large",
+ test_hbitmap_truncate_shrink_large);
g_test_run();
return 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 17/21] iotests: add invalid input incremental backup tests
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (15 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 16/21] hbitmap: truncate tests John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 18/21] iotests: add QMP event waiting queue John Snow
` (5 subsequent siblings)
22 siblings, 0 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/qemu-iotests/124 | 104 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/124.out | 5 +++
tests/qemu-iotests/group | 1 +
3 files changed, 110 insertions(+)
create mode 100644 tests/qemu-iotests/124
create mode 100644 tests/qemu-iotests/124.out
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
new file mode 100644
index 0000000..85675ec
--- /dev/null
+++ b/tests/qemu-iotests/124
@@ -0,0 +1,104 @@
+#!/usr/bin/env python
+#
+# Tests for incremental drive-backup
+#
+# Copyright (C) 2015 John Snow for Red Hat, Inc.
+#
+# Based on 056.
+#
+# 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 <http://www.gnu.org/licenses/>.
+#
+
+import os
+import iotests
+
+
+def io_write_patterns(img, patterns):
+ for pattern in patterns:
+ iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
+
+
+class TestIncrementalBackup(iotests.QMPTestCase):
+ def setUp(self):
+ self.bitmaps = list()
+ self.files = list()
+ self.drives = list()
+ self.vm = iotests.VM()
+ self.err_img = os.path.join(iotests.test_dir, 'err.%s' % iotests.imgfmt)
+
+ # Create a base image with a distinctive patterning
+ drive0 = self.add_node('drive0')
+ self.img_create(drive0['file'], drive0['fmt'])
+ self.vm.add_drive(drive0['file'])
+ io_write_patterns(drive0['file'], (('0x41', 0, 512),
+ ('0xd5', '1M', '32k'),
+ ('0xdc', '32M', '124k')))
+ self.vm.launch()
+
+
+ def add_node(self, node_id, fmt=iotests.imgfmt, path=None, backup=None):
+ if path is None:
+ path = os.path.join(iotests.test_dir, '%s.%s' % (node_id, fmt))
+ if backup is None:
+ backup = os.path.join(iotests.test_dir,
+ '%s.full.backup.%s' % (node_id, fmt))
+
+ self.drives.append({
+ 'id': node_id,
+ 'file': path,
+ 'backup': backup,
+ 'fmt': fmt })
+ return self.drives[-1]
+
+
+ def img_create(self, img, fmt=iotests.imgfmt, size='64M',
+ parent=None, parentFormat=None):
+ if parent:
+ if parentFormat is None:
+ parentFormat = fmt
+ iotests.qemu_img('create', '-f', fmt, img, size,
+ '-b', parent, '-F', parentFormat)
+ else:
+ iotests.qemu_img('create', '-f', fmt, img, size)
+ self.files.append(img)
+
+ def test_sync_dirty_bitmap_missing(self):
+ self.assert_no_active_block_jobs()
+ self.files.append(self.err_img)
+ result = self.vm.qmp('drive-backup', device=self.drives[0]['id'],
+ sync='dirty-bitmap', format=self.drives[0]['fmt'],
+ target=self.err_img)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+
+ def test_sync_dirty_bitmap_not_found(self):
+ self.assert_no_active_block_jobs()
+ self.files.append(self.err_img)
+ result = self.vm.qmp('drive-backup', device=self.drives[0]['id'],
+ sync='dirty-bitmap', bitmap='unknown',
+ format=self.drives[0]['fmt'], target=self.err_img)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+
+ def tearDown(self):
+ self.vm.shutdown()
+ for filename in self.files:
+ try:
+ os.remove(filename)
+ except OSError:
+ pass
+
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
new file mode 100644
index 0000000..fbc63e6
--- /dev/null
+++ b/tests/qemu-iotests/124.out
@@ -0,0 +1,5 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index bcf2578..f9830d2 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -123,5 +123,6 @@
116 rw auto quick
121 rw auto
123 rw auto quick
+124 rw auto backing
128 rw auto quick
130 rw auto quick
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [PATCH v6 18/21] iotests: add QMP event waiting queue
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (16 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 17/21] iotests: add invalid input incremental backup tests John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-22 16:50 ` Max Reitz
2015-04-23 14:24 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case John Snow
` (4 subsequent siblings)
22 siblings, 2 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
A filter is added to allow callers to request very specific
events to be pulled from the event queue, while leaving undesired
events still in the stream.
This allows us to poll for completion data for multiple asynchronous
events in any arbitrary order.
A new timeout context is added to the qmp pull_event method's
wait parameter to allow tests to fail if they do not complete
within some expected period of time.
Also fixed is a bug in qmp.pull_event where we try to retrieve an event
from an empty list if we attempt to retrieve an event with wait=False
but no events have occurred.
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qmp/qmp.py | 95 +++++++++++++++++++++++++++++--------------
tests/qemu-iotests/iotests.py | 38 +++++++++++++++++
2 files changed, 103 insertions(+), 30 deletions(-)
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 20b6ec7..1d38e3e 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -21,6 +21,9 @@ class QMPConnectError(QMPError):
class QMPCapabilitiesError(QMPError):
pass
+class QMPTimeoutError(QMPError):
+ pass
+
class QEMUMonitorProtocol:
def __init__(self, address, server=False):
"""
@@ -72,6 +75,44 @@ class QEMUMonitorProtocol:
error = socket.error
+ def __get_events(self, wait=False):
+ """
+ Check for new events in the stream and cache them in __events.
+
+ @param wait (bool): block until an event is available.
+ @param wait (float): If wait is a float, treat it as a timeout value.
+
+ @raise QMPTimeoutError: If a timeout float is provided and the timeout
+ period elapses.
+ @raise QMPConnectError: If wait is True but no events could be retrieved
+ or if some other error occurred.
+ """
+
+ # Check for new events regardless and pull them into the cache:
+ self.__sock.setblocking(0)
+ try:
+ self.__json_read()
+ except socket.error, err:
+ if err[0] == errno.EAGAIN:
+ # No data available
+ pass
+ self.__sock.setblocking(1)
+
+ # Wait for new events, if needed.
+ # if wait is 0.0, this means "no wait" and is also implicitly false.
+ if not self.__events and wait:
+ if isinstance(wait, float):
+ self.__sock.settimeout(wait)
+ try:
+ ret = self.__json_read(only_event=True)
+ except socket.timeout:
+ raise QMPTimeoutError("Timeout waiting for event")
+ except:
+ raise QMPConnectError("Error while reading from socket")
+ if ret is None:
+ raise QMPConnectError("Error while reading from socket")
+ self.__sock.settimeout(None)
+
def connect(self, negotiate=True):
"""
Connect to the QMP Monitor and perform capabilities negotiation.
@@ -140,43 +181,37 @@ class QEMUMonitorProtocol:
"""
Get and delete the first available QMP event.
- @param wait: block until an event is available (bool)
+ @param wait (bool): block until an event is available.
+ @param wait (float): If wait is a float, treat it as a timeout value.
+
+ @raise QMPTimeoutError: If a timeout float is provided and the timeout
+ period elapses.
+ @raise QMPConnectError: If wait is True but no events could be retrieved
+ or if some other error occurred.
+
+ @return The first available QMP event, or None.
"""
- self.__sock.setblocking(0)
- try:
- self.__json_read()
- except socket.error, err:
- if err[0] == errno.EAGAIN:
- # No data available
- pass
- self.__sock.setblocking(1)
- if not self.__events and wait:
- self.__json_read(only_event=True)
- event = self.__events[0]
- del self.__events[0]
- return event
+ self.__get_events(wait)
+
+ if self.__events:
+ return self.__events.pop(0)
+ return None
def get_events(self, wait=False):
"""
Get a list of available QMP events.
- @param wait: block until an event is available (bool)
+ @param wait (bool): block until an event is available.
+ @param wait (float): If wait is a float, treat it as a timeout value.
+
+ @raise QMPTimeoutError: If a timeout float is provided and the timeout
+ period elapses.
+ @raise QMPConnectError: If wait is True but no events could be retrieved
+ or if some other error occurred.
+
+ @return The list of available QMP events.
"""
- self.__sock.setblocking(0)
- try:
- self.__json_read()
- except socket.error, err:
- if err[0] == errno.EAGAIN:
- # No data available
- pass
- self.__sock.setblocking(1)
- if not self.__events and wait:
- ret = self.__json_read(only_event=True)
- if ret == None:
- # We are in blocking mode, if don't get anything, something
- # went wrong
- raise QMPConnectError("Error while reading from socket")
-
+ self.__get_events(wait)
return self.__events
def clear_events(self):
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1402854..e93e623 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -78,6 +78,23 @@ def create_image(name, size):
i = i + 512
file.close()
+# Test if 'match' is a recursive subset of 'event'
+def event_match(event, match=None):
+ if match is None:
+ return True
+
+ for key in match:
+ if key in event:
+ if isinstance(event[key], dict):
+ if not event_match(event[key], match[key]):
+ return False
+ elif event[key] != match[key]:
+ return False
+ else:
+ return False
+
+ return True
+
class VM(object):
'''A QEMU VM'''
@@ -92,6 +109,7 @@ class VM(object):
'-machine', 'accel=qtest',
'-display', 'none', '-vga', 'none']
self._num_drives = 0
+ self._events = []
# This can be used to add an unused monitor instance.
def add_monitor_telnet(self, ip, port):
@@ -202,14 +220,34 @@ class VM(object):
def get_qmp_event(self, wait=False):
'''Poll for one queued QMP events and return it'''
+ if len(self._events) > 0:
+ return self._events.pop(0)
return self._qmp.pull_event(wait=wait)
def get_qmp_events(self, wait=False):
'''Poll for queued QMP events and return a list of dicts'''
events = self._qmp.get_events(wait=wait)
+ events.extend(self._events)
+ del self._events[:]
self._qmp.clear_events()
return events
+ def event_wait(self, name='BLOCK_JOB_COMPLETED', timeout=60.0, match=None):
+ # Search cached events
+ for event in self._events:
+ if (event['event'] == name) and event_match(event, match):
+ self._events.remove(event)
+ return event
+
+ # Poll for new events
+ while True:
+ event = self._qmp.pull_event(wait=timeout)
+ if (event['event'] == name) and event_match(event, match):
+ return event
+ self._events.append(event)
+
+ return None
+
index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
class QMPTestCase(unittest.TestCase):
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 18/21] iotests: add QMP event waiting queue
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 18/21] iotests: add QMP event waiting queue John Snow
@ 2015-04-22 16:50 ` Max Reitz
2015-04-23 14:24 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
1 sibling, 0 replies; 52+ messages in thread
From: Max Reitz @ 2015-04-22 16:50 UTC (permalink / raw)
To: John Snow, qemu-block
Cc: kwolf, famz, qemu-devel, armbru, vsementsov, stefanha
On 18.04.2015 01:50, John Snow wrote:
> A filter is added to allow callers to request very specific
> events to be pulled from the event queue, while leaving undesired
> events still in the stream.
>
> This allows us to poll for completion data for multiple asynchronous
> events in any arbitrary order.
>
> A new timeout context is added to the qmp pull_event method's
> wait parameter to allow tests to fail if they do not complete
> within some expected period of time.
>
> Also fixed is a bug in qmp.pull_event where we try to retrieve an event
> from an empty list if we attempt to retrieve an event with wait=False
> but no events have occurred.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qmp/qmp.py | 95 +++++++++++++++++++++++++++++--------------
> tests/qemu-iotests/iotests.py | 38 +++++++++++++++++
> 2 files changed, 103 insertions(+), 30 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 18/21] iotests: add QMP event waiting queue
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 18/21] iotests: add QMP event waiting queue John Snow
2015-04-22 16:50 ` Max Reitz
@ 2015-04-23 14:24 ` Stefan Hajnoczi
1 sibling, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 14:24 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 999 bytes --]
On Fri, Apr 17, 2015 at 07:50:06PM -0400, John Snow wrote:
> A filter is added to allow callers to request very specific
> events to be pulled from the event queue, while leaving undesired
> events still in the stream.
>
> This allows us to poll for completion data for multiple asynchronous
> events in any arbitrary order.
>
> A new timeout context is added to the qmp pull_event method's
> wait parameter to allow tests to fail if they do not complete
> within some expected period of time.
>
> Also fixed is a bug in qmp.pull_event where we try to retrieve an event
> from an empty list if we attempt to retrieve an event with wait=False
> but no events have occurred.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qmp/qmp.py | 95 +++++++++++++++++++++++++++++--------------
> tests/qemu-iotests/iotests.py | 38 +++++++++++++++++
> 2 files changed, 103 insertions(+), 30 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (17 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 18/21] iotests: add QMP event waiting queue John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-23 14:28 ` Stefan Hajnoczi
2015-05-22 15:02 ` Kevin Wolf
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test John Snow
` (3 subsequent siblings)
22 siblings, 2 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/124 | 174 +++++++++++++++++++++++++++++++++++++++++++--
tests/qemu-iotests/124.out | 4 +-
2 files changed, 172 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 85675ec..5c3b434 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -29,6 +29,51 @@ def io_write_patterns(img, patterns):
iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
+def try_remove(img):
+ try:
+ os.remove(img)
+ except OSError:
+ pass
+
+
+class Bitmap:
+ def __init__(self, name, drive):
+ self.name = name
+ self.drive = drive
+ self.num = 0
+ self.backups = list()
+
+ def base_target(self):
+ return (self.drive['backup'], None)
+
+ def new_target(self, num=None):
+ if num is None:
+ num = self.num
+ self.num = num + 1
+ base = os.path.join(iotests.test_dir,
+ "%s.%s." % (self.drive['id'], self.name))
+ suff = "%i.%s" % (num, self.drive['fmt'])
+ target = base + "inc" + suff
+ reference = base + "ref" + suff
+ self.backups.append((target, reference))
+ return (target, reference)
+
+ def last_target(self):
+ if self.backups:
+ return self.backups[-1]
+ return self.base_target()
+
+ def del_target(self):
+ for image in self.backups.pop():
+ try_remove(image)
+ self.num -= 1
+
+ def cleanup(self):
+ for backup in self.backups:
+ for image in backup:
+ try_remove(image)
+
+
class TestIncrementalBackup(iotests.QMPTestCase):
def setUp(self):
self.bitmaps = list()
@@ -73,6 +118,128 @@ class TestIncrementalBackup(iotests.QMPTestCase):
iotests.qemu_img('create', '-f', fmt, img, size)
self.files.append(img)
+
+ def do_qmp_backup(self, error='Input/output error', **kwargs):
+ res = self.vm.qmp('drive-backup', **kwargs)
+ self.assert_qmp(res, 'return', {})
+
+ event = self.vm.event_wait(name="BLOCK_JOB_COMPLETED",
+ match={'data': {'device': kwargs['device']}})
+ self.assertIsNotNone(event)
+
+ try:
+ failure = self.dictpath(event, 'data/error')
+ except AssertionError:
+ # Backup succeeded.
+ self.assert_qmp(event, 'data/offset', event['data']['len'])
+ return True
+ else:
+ # Backup failed.
+ self.assert_qmp(event, 'data/error', error)
+ return False
+
+
+ def create_anchor_backup(self, drive=None):
+ if drive is None:
+ drive = self.drives[-1]
+ res = self.do_qmp_backup(device=drive['id'], sync='full',
+ format=drive['fmt'], target=drive['backup'])
+ self.assertTrue(res)
+ self.files.append(drive['backup'])
+ return drive['backup']
+
+
+ def make_reference_backup(self, bitmap=None):
+ if bitmap is None:
+ bitmap = self.bitmaps[-1]
+ _, reference = bitmap.last_target()
+ res = self.do_qmp_backup(device=bitmap.drive['id'], sync='full',
+ format=bitmap.drive['fmt'], target=reference)
+ self.assertTrue(res)
+
+
+ def add_bitmap(self, name, drive):
+ bitmap = Bitmap(name, drive)
+ self.bitmaps.append(bitmap)
+ result = self.vm.qmp('block-dirty-bitmap-add', node=drive['id'],
+ name=bitmap.name)
+ self.assert_qmp(result, 'return', {})
+ return bitmap
+
+
+ def prepare_backup(self, bitmap=None, parent=None):
+ if bitmap is None:
+ bitmap = self.bitmaps[-1]
+ if parent is None:
+ parent, _ = bitmap.last_target()
+
+ target, _ = bitmap.new_target()
+ self.img_create(target, bitmap.drive['fmt'], parent=parent)
+ return target
+
+
+ def create_incremental(self, bitmap=None, parent=None,
+ parentFormat=None, validate=True):
+ if bitmap is None:
+ bitmap = self.bitmaps[-1]
+ if parent is None:
+ parent, _ = bitmap.last_target()
+
+ target = self.prepare_backup(bitmap, parent)
+ res = self.do_qmp_backup(device=bitmap.drive['id'],
+ sync='dirty-bitmap', bitmap=bitmap.name,
+ format=bitmap.drive['fmt'], target=target,
+ mode='existing')
+ if not res:
+ bitmap.del_target();
+ self.assertFalse(validate)
+ else:
+ self.make_reference_backup(bitmap)
+ return res
+
+
+ def check_backups(self):
+ for bitmap in self.bitmaps:
+ for incremental, reference in bitmap.backups:
+ self.assertTrue(iotests.compare_images(incremental, reference))
+ last = bitmap.last_target()[0]
+ self.assertTrue(iotests.compare_images(last, bitmap.drive['file']))
+
+
+ def hmp_io_writes(self, drive, patterns):
+ for pattern in patterns:
+ self.vm.hmp_qemu_io(drive, 'write -P%s %s %s' % pattern)
+ self.vm.hmp_qemu_io(drive, 'flush')
+
+
+ def test_incremental_simple(self):
+ '''
+ Test: Create and verify three incremental backups.
+
+ Create a bitmap and a full backup before VM execution begins,
+ then create a series of three incremental backups "during execution,"
+ i.e.; after IO requests begin modifying the drive.
+ '''
+ self.create_anchor_backup()
+ self.add_bitmap('bitmap0', self.drives[0])
+
+ # Sanity: Create a "hollow" incremental backup
+ self.create_incremental()
+ # Three writes: One complete overwrite, one new segment,
+ # and one partial overlap.
+ self.hmp_io_writes(self.drives[0]['id'], (('0xab', 0, 512),
+ ('0xfe', '16M', '256k'),
+ ('0x64', '32736k', '64k')))
+ self.create_incremental()
+ # Three more writes, one of each kind, like above
+ self.hmp_io_writes(self.drives[0]['id'], (('0x9a', 0, 512),
+ ('0x55', '8M', '352k'),
+ ('0x78', '15872k', '1M')))
+ self.create_incremental()
+ self.vm.shutdown()
+ self.check_backups()
+
+
def test_sync_dirty_bitmap_missing(self):
self.assert_no_active_block_jobs()
self.files.append(self.err_img)
@@ -93,11 +260,10 @@ class TestIncrementalBackup(iotests.QMPTestCase):
def tearDown(self):
self.vm.shutdown()
+ for bitmap in self.bitmaps:
+ bitmap.cleanup()
for filename in self.files:
- try:
- os.remove(filename)
- except OSError:
- pass
+ try_remove(filename)
if __name__ == '__main__':
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index fbc63e6..8d7e996 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-..
+...
----------------------------------------------------------------------
-Ran 2 tests
+Ran 3 tests
OK
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case John Snow
@ 2015-04-23 14:28 ` Stefan Hajnoczi
2015-05-22 15:02 ` Kevin Wolf
1 sibling, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 14:28 UTC (permalink / raw)
To: John Snow
Cc: kwolf, famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 383 bytes --]
On Fri, Apr 17, 2015 at 07:50:07PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/124 | 174 +++++++++++++++++++++++++++++++++++++++++++--
> tests/qemu-iotests/124.out | 4 +-
> 2 files changed, 172 insertions(+), 6 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case John Snow
2015-04-23 14:28 ` Stefan Hajnoczi
@ 2015-05-22 15:02 ` Kevin Wolf
2015-05-22 15:29 ` John Snow
1 sibling, 1 reply; 52+ messages in thread
From: Kevin Wolf @ 2015-05-22 15:02 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Am 18.04.2015 um 01:50 hat John Snow geschrieben:
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/124 | 174 +++++++++++++++++++++++++++++++++++++++++++--
> tests/qemu-iotests/124.out | 4 +-
> 2 files changed, 172 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> index 85675ec..5c3b434 100644
> --- a/tests/qemu-iotests/124
> +++ b/tests/qemu-iotests/124
> @@ -29,6 +29,51 @@ def io_write_patterns(img, patterns):
> iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
>
>
> +def try_remove(img):
> + try:
> + os.remove(img)
> + except OSError:
> + pass
> +
> +
> +class Bitmap:
> + def __init__(self, name, drive):
> + self.name = name
> + self.drive = drive
> + self.num = 0
> + self.backups = list()
> +
> + def base_target(self):
> + return (self.drive['backup'], None)
> +
> + def new_target(self, num=None):
> + if num is None:
> + num = self.num
> + self.num = num + 1
> + base = os.path.join(iotests.test_dir,
> + "%s.%s." % (self.drive['id'], self.name))
> + suff = "%i.%s" % (num, self.drive['fmt'])
> + target = base + "inc" + suff
> + reference = base + "ref" + suff
> + self.backups.append((target, reference))
> + return (target, reference)
> +
> + def last_target(self):
> + if self.backups:
> + return self.backups[-1]
> + return self.base_target()
> +
> + def del_target(self):
> + for image in self.backups.pop():
> + try_remove(image)
> + self.num -= 1
> +
> + def cleanup(self):
> + for backup in self.backups:
> + for image in backup:
> + try_remove(image)
> +
> +
> class TestIncrementalBackup(iotests.QMPTestCase):
> def setUp(self):
> self.bitmaps = list()
> @@ -73,6 +118,128 @@ class TestIncrementalBackup(iotests.QMPTestCase):
> iotests.qemu_img('create', '-f', fmt, img, size)
> self.files.append(img)
>
> +
> + def do_qmp_backup(self, error='Input/output error', **kwargs):
> + res = self.vm.qmp('drive-backup', **kwargs)
> + self.assert_qmp(res, 'return', {})
> +
> + event = self.vm.event_wait(name="BLOCK_JOB_COMPLETED",
> + match={'data': {'device': kwargs['device']}})
> + self.assertIsNotNone(event)
RHEL 6 doesn't have self.assertIsNotNone(), so 124 fails there now. Can
you send a follow-up patch to fix this?
Kevin
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case
2015-05-22 15:02 ` Kevin Wolf
@ 2015-05-22 15:29 ` John Snow
2015-05-22 15:37 ` Kevin Wolf
0 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-05-22 15:29 UTC (permalink / raw)
To: Kevin Wolf
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
On 05/22/2015 11:02 AM, Kevin Wolf wrote:
> Am 18.04.2015 um 01:50 hat John Snow geschrieben:
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/124 | 174 +++++++++++++++++++++++++++++++++++++++++++--
>> tests/qemu-iotests/124.out | 4 +-
>> 2 files changed, 172 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
>> index 85675ec..5c3b434 100644
>> --- a/tests/qemu-iotests/124
>> +++ b/tests/qemu-iotests/124
>> @@ -29,6 +29,51 @@ def io_write_patterns(img, patterns):
>> iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
>>
>>
>> +def try_remove(img):
>> + try:
>> + os.remove(img)
>> + except OSError:
>> + pass
>> +
>> +
>> +class Bitmap:
>> + def __init__(self, name, drive):
>> + self.name = name
>> + self.drive = drive
>> + self.num = 0
>> + self.backups = list()
>> +
>> + def base_target(self):
>> + return (self.drive['backup'], None)
>> +
>> + def new_target(self, num=None):
>> + if num is None:
>> + num = self.num
>> + self.num = num + 1
>> + base = os.path.join(iotests.test_dir,
>> + "%s.%s." % (self.drive['id'], self.name))
>> + suff = "%i.%s" % (num, self.drive['fmt'])
>> + target = base + "inc" + suff
>> + reference = base + "ref" + suff
>> + self.backups.append((target, reference))
>> + return (target, reference)
>> +
>> + def last_target(self):
>> + if self.backups:
>> + return self.backups[-1]
>> + return self.base_target()
>> +
>> + def del_target(self):
>> + for image in self.backups.pop():
>> + try_remove(image)
>> + self.num -= 1
>> +
>> + def cleanup(self):
>> + for backup in self.backups:
>> + for image in backup:
>> + try_remove(image)
>> +
>> +
>> class TestIncrementalBackup(iotests.QMPTestCase):
>> def setUp(self):
>> self.bitmaps = list()
>> @@ -73,6 +118,128 @@ class TestIncrementalBackup(iotests.QMPTestCase):
>> iotests.qemu_img('create', '-f', fmt, img, size)
>> self.files.append(img)
>>
>> +
>> + def do_qmp_backup(self, error='Input/output error', **kwargs):
>> + res = self.vm.qmp('drive-backup', **kwargs)
>> + self.assert_qmp(res, 'return', {})
>> +
>> + event = self.vm.event_wait(name="BLOCK_JOB_COMPLETED",
>> + match={'data': {'device': kwargs['device']}})
>> + self.assertIsNotNone(event)
>
> RHEL 6 doesn't have self.assertIsNotNone(), so 124 fails there now. Can
> you send a follow-up patch to fix this?
>
> Kevin
>
What's the minimum version requirement for RHEL6, for future information?
--js
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case
2015-05-22 15:29 ` John Snow
@ 2015-05-22 15:37 ` Kevin Wolf
0 siblings, 0 replies; 52+ messages in thread
From: Kevin Wolf @ 2015-05-22 15:37 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Am 22.05.2015 um 17:29 hat John Snow geschrieben:
>
>
> On 05/22/2015 11:02 AM, Kevin Wolf wrote:
> > Am 18.04.2015 um 01:50 hat John Snow geschrieben:
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> >> Reviewed-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >> tests/qemu-iotests/124 | 174 +++++++++++++++++++++++++++++++++++++++++++--
> >> tests/qemu-iotests/124.out | 4 +-
> >> 2 files changed, 172 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> >> index 85675ec..5c3b434 100644
> >> --- a/tests/qemu-iotests/124
> >> +++ b/tests/qemu-iotests/124
> >> @@ -29,6 +29,51 @@ def io_write_patterns(img, patterns):
> >> iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
> >>
> >>
> >> +def try_remove(img):
> >> + try:
> >> + os.remove(img)
> >> + except OSError:
> >> + pass
> >> +
> >> +
> >> +class Bitmap:
> >> + def __init__(self, name, drive):
> >> + self.name = name
> >> + self.drive = drive
> >> + self.num = 0
> >> + self.backups = list()
> >> +
> >> + def base_target(self):
> >> + return (self.drive['backup'], None)
> >> +
> >> + def new_target(self, num=None):
> >> + if num is None:
> >> + num = self.num
> >> + self.num = num + 1
> >> + base = os.path.join(iotests.test_dir,
> >> + "%s.%s." % (self.drive['id'], self.name))
> >> + suff = "%i.%s" % (num, self.drive['fmt'])
> >> + target = base + "inc" + suff
> >> + reference = base + "ref" + suff
> >> + self.backups.append((target, reference))
> >> + return (target, reference)
> >> +
> >> + def last_target(self):
> >> + if self.backups:
> >> + return self.backups[-1]
> >> + return self.base_target()
> >> +
> >> + def del_target(self):
> >> + for image in self.backups.pop():
> >> + try_remove(image)
> >> + self.num -= 1
> >> +
> >> + def cleanup(self):
> >> + for backup in self.backups:
> >> + for image in backup:
> >> + try_remove(image)
> >> +
> >> +
> >> class TestIncrementalBackup(iotests.QMPTestCase):
> >> def setUp(self):
> >> self.bitmaps = list()
> >> @@ -73,6 +118,128 @@ class TestIncrementalBackup(iotests.QMPTestCase):
> >> iotests.qemu_img('create', '-f', fmt, img, size)
> >> self.files.append(img)
> >>
> >> +
> >> + def do_qmp_backup(self, error='Input/output error', **kwargs):
> >> + res = self.vm.qmp('drive-backup', **kwargs)
> >> + self.assert_qmp(res, 'return', {})
> >> +
> >> + event = self.vm.event_wait(name="BLOCK_JOB_COMPLETED",
> >> + match={'data': {'device': kwargs['device']}})
> >> + self.assertIsNotNone(event)
> >
> > RHEL 6 doesn't have self.assertIsNotNone(), so 124 fails there now. Can
> > you send a follow-up patch to fix this?
> >
> > Kevin
> >
>
> What's the minimum version requirement for RHEL6, for future information?
$ python --version
Python 2.6.5
Kevin
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (18 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 19/21] iotests: add simple incremental backup case John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-23 14:28 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-11-27 17:14 ` [Qemu-devel] " Kevin Wolf
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 21/21] iotests: add incremental backup granularity tests John Snow
` (2 subsequent siblings)
22 siblings, 2 replies; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Test the failure case for incremental backups.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/124 | 57 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/124.out | 4 ++--
2 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 5c3b434..95f6de5 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -240,6 +240,63 @@ class TestIncrementalBackup(iotests.QMPTestCase):
self.check_backups()
+ def test_incremental_failure(self):
+ '''Test: Verify backups made after a failure are correct.
+
+ Simulate a failure during an incremental backup block job,
+ emulate additional writes, then create another incremental backup
+ afterwards and verify that the backup created is correct.
+ '''
+
+ # Create a blkdebug interface to this img as 'drive1',
+ # but don't actually create a new image.
+ drive1 = self.add_node('drive1', self.drives[0]['fmt'],
+ path=self.drives[0]['file'],
+ backup=self.drives[0]['backup'])
+ result = self.vm.qmp('blockdev-add', options={
+ 'id': drive1['id'],
+ 'driver': drive1['fmt'],
+ 'file': {
+ 'driver': 'blkdebug',
+ 'image': {
+ 'driver': 'file',
+ 'filename': drive1['file']
+ },
+ 'set-state': [{
+ 'event': 'flush_to_disk',
+ 'state': 1,
+ 'new_state': 2
+ }],
+ 'inject-error': [{
+ 'event': 'read_aio',
+ 'errno': 5,
+ 'state': 2,
+ 'immediately': False,
+ 'once': True
+ }],
+ }
+ })
+ self.assert_qmp(result, 'return', {})
+
+ self.create_anchor_backup(self.drives[0])
+ self.add_bitmap('bitmap0', drive1)
+ # Note: at this point, during a normal execution,
+ # Assume that the VM resumes and begins issuing IO requests here.
+
+ self.hmp_io_writes(drive1['id'], (('0xab', 0, 512),
+ ('0xfe', '16M', '256k'),
+ ('0x64', '32736k', '64k')))
+
+ result = self.create_incremental(validate=False)
+ self.assertFalse(result)
+ self.hmp_io_writes(drive1['id'], (('0x9a', 0, 512),
+ ('0x55', '8M', '352k'),
+ ('0x78', '15872k', '1M')))
+ self.create_incremental()
+ self.vm.shutdown()
+ self.check_backups()
+
+
def test_sync_dirty_bitmap_missing(self):
self.assert_no_active_block_jobs()
self.files.append(self.err_img)
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 8d7e996..89968f3 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-...
+....
----------------------------------------------------------------------
-Ran 3 tests
+Ran 4 tests
OK
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH v6 20/21] iotests: add incremental backup failure recovery test
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test John Snow
@ 2015-04-23 14:28 ` Stefan Hajnoczi
2015-11-27 17:14 ` [Qemu-devel] " Kevin Wolf
1 sibling, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 14:28 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 446 bytes --]
On Fri, Apr 17, 2015 at 07:50:08PM -0400, John Snow wrote:
> Test the failure case for incremental backups.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/124 | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/124.out | 4 ++--
> 2 files changed, 59 insertions(+), 2 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test John Snow
2015-04-23 14:28 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2015-11-27 17:14 ` Kevin Wolf
2015-11-30 17:17 ` John Snow
1 sibling, 1 reply; 52+ messages in thread
From: Kevin Wolf @ 2015-11-27 17:14 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Am 18.04.2015 um 01:50 hat John Snow geschrieben:
> Test the failure case for incremental backups.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/124 | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/124.out | 4 ++--
> 2 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> index 5c3b434..95f6de5 100644
> --- a/tests/qemu-iotests/124
> +++ b/tests/qemu-iotests/124
> @@ -240,6 +240,63 @@ class TestIncrementalBackup(iotests.QMPTestCase):
> self.check_backups()
>
>
> + def test_incremental_failure(self):
> + '''Test: Verify backups made after a failure are correct.
> +
> + Simulate a failure during an incremental backup block job,
> + emulate additional writes, then create another incremental backup
> + afterwards and verify that the backup created is correct.
> + '''
> +
> + # Create a blkdebug interface to this img as 'drive1',
> + # but don't actually create a new image.
> + drive1 = self.add_node('drive1', self.drives[0]['fmt'],
> + path=self.drives[0]['file'],
> + backup=self.drives[0]['backup'])
> + result = self.vm.qmp('blockdev-add', options={
> + 'id': drive1['id'],
> + 'driver': drive1['fmt'],
> + 'file': {
> + 'driver': 'blkdebug',
> + 'image': {
> + 'driver': 'file',
> + 'filename': drive1['file']
> + },
> + 'set-state': [{
> + 'event': 'flush_to_disk',
> + 'state': 1,
> + 'new_state': 2
> + }],
> + 'inject-error': [{
> + 'event': 'read_aio',
> + 'errno': 5,
> + 'state': 2,
> + 'immediately': False,
> + 'once': True
> + }],
> + }
> + })
> + self.assert_qmp(result, 'return', {})
John, how naughty of you!
It's interesting how many tests break now that I tried to add some
advisory qcow2 locking so that people don't constantly break their
images with 'qemu-img snapshot' while the VM is running.
I think this one is a bug in the test case. I'm not completely sure how
to fix it, though. Can we move the blkdebug layer to the top level? I
think reusing the same qcow2 BDS (using a node name reference) would be
okay. We just need to avoid opening the qcow2 layer twice for the same
image.
Kevin
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test
2015-11-27 17:14 ` [Qemu-devel] " Kevin Wolf
@ 2015-11-30 17:17 ` John Snow
2015-12-01 9:31 ` Kevin Wolf
0 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-11-30 17:17 UTC (permalink / raw)
To: Kevin Wolf
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
On 11/27/2015 12:14 PM, Kevin Wolf wrote:
> Am 18.04.2015 um 01:50 hat John Snow geschrieben:
>> Test the failure case for incremental backups.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/124 | 57 ++++++++++++++++++++++++++++++++++++++++++++++
>> tests/qemu-iotests/124.out | 4 ++--
>> 2 files changed, 59 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
>> index 5c3b434..95f6de5 100644
>> --- a/tests/qemu-iotests/124
>> +++ b/tests/qemu-iotests/124
>> @@ -240,6 +240,63 @@ class TestIncrementalBackup(iotests.QMPTestCase):
>> self.check_backups()
>>
>>
>> + def test_incremental_failure(self):
>> + '''Test: Verify backups made after a failure are correct.
>> +
>> + Simulate a failure during an incremental backup block job,
>> + emulate additional writes, then create another incremental backup
>> + afterwards and verify that the backup created is correct.
>> + '''
>> +
>> + # Create a blkdebug interface to this img as 'drive1',
>> + # but don't actually create a new image.
>> + drive1 = self.add_node('drive1', self.drives[0]['fmt'],
>> + path=self.drives[0]['file'],
>> + backup=self.drives[0]['backup'])
>> + result = self.vm.qmp('blockdev-add', options={
>> + 'id': drive1['id'],
>> + 'driver': drive1['fmt'],
>> + 'file': {
>> + 'driver': 'blkdebug',
>> + 'image': {
>> + 'driver': 'file',
>> + 'filename': drive1['file']
>> + },
>> + 'set-state': [{
>> + 'event': 'flush_to_disk',
>> + 'state': 1,
>> + 'new_state': 2
>> + }],
>> + 'inject-error': [{
>> + 'event': 'read_aio',
>> + 'errno': 5,
>> + 'state': 2,
>> + 'immediately': False,
>> + 'once': True
>> + }],
>> + }
>> + })
>> + self.assert_qmp(result, 'return', {})
>
> John, how naughty of you!
>
And here I thought it was OK because nobody yelled!
The yell was just delayed.
> It's interesting how many tests break now that I tried to add some
> advisory qcow2 locking so that people don't constantly break their
> images with 'qemu-img snapshot' while the VM is running.
>
> I think this one is a bug in the test case. I'm not completely sure how
> to fix it, though. Can we move the blkdebug layer to the top level? I
> think reusing the same qcow2 BDS (using a node name reference) would be
> okay. We just need to avoid opening the qcow2 layer twice for the same
> image.
>
I can either do that, or just fall back to fully allocating two images
and modify the test accordingly.
> Kevin
>
Is this for 2.5?
--js
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test
2015-11-30 17:17 ` John Snow
@ 2015-12-01 9:31 ` Kevin Wolf
0 siblings, 0 replies; 52+ messages in thread
From: Kevin Wolf @ 2015-12-01 9:31 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Am 30.11.2015 um 18:17 hat John Snow geschrieben:
> On 11/27/2015 12:14 PM, Kevin Wolf wrote:
> > Am 18.04.2015 um 01:50 hat John Snow geschrieben:
> >> Test the failure case for incremental backups.
> >>
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> >> Reviewed-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >> tests/qemu-iotests/124 | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> >> tests/qemu-iotests/124.out | 4 ++--
> >> 2 files changed, 59 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> >> index 5c3b434..95f6de5 100644
> >> --- a/tests/qemu-iotests/124
> >> +++ b/tests/qemu-iotests/124
> >> @@ -240,6 +240,63 @@ class TestIncrementalBackup(iotests.QMPTestCase):
> >> self.check_backups()
> >>
> >>
> >> + def test_incremental_failure(self):
> >> + '''Test: Verify backups made after a failure are correct.
> >> +
> >> + Simulate a failure during an incremental backup block job,
> >> + emulate additional writes, then create another incremental backup
> >> + afterwards and verify that the backup created is correct.
> >> + '''
> >> +
> >> + # Create a blkdebug interface to this img as 'drive1',
> >> + # but don't actually create a new image.
> >> + drive1 = self.add_node('drive1', self.drives[0]['fmt'],
> >> + path=self.drives[0]['file'],
> >> + backup=self.drives[0]['backup'])
> >> + result = self.vm.qmp('blockdev-add', options={
> >> + 'id': drive1['id'],
> >> + 'driver': drive1['fmt'],
> >> + 'file': {
> >> + 'driver': 'blkdebug',
> >> + 'image': {
> >> + 'driver': 'file',
> >> + 'filename': drive1['file']
> >> + },
> >> + 'set-state': [{
> >> + 'event': 'flush_to_disk',
> >> + 'state': 1,
> >> + 'new_state': 2
> >> + }],
> >> + 'inject-error': [{
> >> + 'event': 'read_aio',
> >> + 'errno': 5,
> >> + 'state': 2,
> >> + 'immediately': False,
> >> + 'once': True
> >> + }],
> >> + }
> >> + })
> >> + self.assert_qmp(result, 'return', {})
> >
> > John, how naughty of you!
> >
>
> And here I thought it was OK because nobody yelled!
>
> The yell was just delayed.
>
> > It's interesting how many tests break now that I tried to add some
> > advisory qcow2 locking so that people don't constantly break their
> > images with 'qemu-img snapshot' while the VM is running.
> >
> > I think this one is a bug in the test case. I'm not completely sure how
> > to fix it, though. Can we move the blkdebug layer to the top level? I
> > think reusing the same qcow2 BDS (using a node name reference) would be
> > okay. We just need to avoid opening the qcow2 layer twice for the same
> > image.
> >
>
> I can either do that, or just fall back to fully allocating two images
> and modify the test accordingly.
Whatever is the easiest (and works).
> Is this for 2.5?
No, that's definitely 2.6. But if possible, I want to merge the qcow2
locking relatively early in the cycle because I want to see how
surprising the new behaviour would be for users and whether it's too
restrictive before we enable it in a release.
Kevin
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [PATCH v6 21/21] iotests: add incremental backup granularity tests
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (19 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 20/21] iotests: add incremental backup failure recovery test John Snow
@ 2015-04-17 23:50 ` John Snow
2015-04-23 14:29 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-04-23 13:19 ` [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series Stefan Hajnoczi
2015-04-24 14:02 ` [Qemu-devel] " Stefan Hajnoczi
22 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-04-17 23:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, famz, John Snow, qemu-devel, armbru, vsementsov, stefanha,
mreitz
Test what happens if you fiddle with the granularity.
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/124 | 58 +++++++++++++++++++++++++++++++++++++---------
tests/qemu-iotests/124.out | 4 ++--
2 files changed, 49 insertions(+), 13 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 95f6de5..3ee78cd 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -158,11 +158,11 @@ class TestIncrementalBackup(iotests.QMPTestCase):
self.assertTrue(res)
- def add_bitmap(self, name, drive):
+ def add_bitmap(self, name, drive, **kwargs):
bitmap = Bitmap(name, drive)
self.bitmaps.append(bitmap)
result = self.vm.qmp('block-dirty-bitmap-add', node=drive['id'],
- name=bitmap.name)
+ name=bitmap.name, **kwargs)
self.assert_qmp(result, 'return', {})
return bitmap
@@ -212,16 +212,9 @@ class TestIncrementalBackup(iotests.QMPTestCase):
self.vm.hmp_qemu_io(drive, 'flush')
- def test_incremental_simple(self):
- '''
- Test: Create and verify three incremental backups.
-
- Create a bitmap and a full backup before VM execution begins,
- then create a series of three incremental backups "during execution,"
- i.e.; after IO requests begin modifying the drive.
- '''
+ def do_incremental_simple(self, **kwargs):
self.create_anchor_backup()
- self.add_bitmap('bitmap0', self.drives[0])
+ self.add_bitmap('bitmap0', self.drives[0], **kwargs)
# Sanity: Create a "hollow" incremental backup
self.create_incremental()
@@ -240,6 +233,37 @@ class TestIncrementalBackup(iotests.QMPTestCase):
self.check_backups()
+ def test_incremental_simple(self):
+ '''
+ Test: Create and verify three incremental backups.
+
+ Create a bitmap and a full backup before VM execution begins,
+ then create a series of three incremental backups "during execution,"
+ i.e.; after IO requests begin modifying the drive.
+ '''
+ return self.do_incremental_simple()
+
+
+ def test_small_granularity(self):
+ '''
+ Test: Create and verify backups made with a small granularity bitmap.
+
+ Perform the same test as test_incremental_simple, but with a granularity
+ of only 32KiB instead of the present default of 64KiB.
+ '''
+ return self.do_incremental_simple(granularity=32768)
+
+
+ def test_large_granularity(self):
+ '''
+ Test: Create and verify backups made with a large granularity bitmap.
+
+ Perform the same test as test_incremental_simple, but with a granularity
+ of 128KiB instead of the present default of 64KiB.
+ '''
+ return self.do_incremental_simple(granularity=131072)
+
+
def test_incremental_failure(self):
'''Test: Verify backups made after a failure are correct.
@@ -315,6 +339,18 @@ class TestIncrementalBackup(iotests.QMPTestCase):
self.assert_qmp(result, 'error/class', 'GenericError')
+ def test_sync_dirty_bitmap_bad_granularity(self):
+ '''
+ Test: Test what happens if we provide an improper granularity.
+
+ The granularity must always be a power of 2.
+ '''
+ self.assert_no_active_block_jobs()
+ self.assertRaises(AssertionError, self.add_bitmap,
+ 'bitmap0', self.drives[0],
+ granularity=64000)
+
+
def tearDown(self):
self.vm.shutdown()
for bitmap in self.bitmaps:
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 89968f3..2f7d390 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-....
+.......
----------------------------------------------------------------------
-Ran 4 tests
+Ran 7 tests
OK
--
2.1.0
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH v6 21/21] iotests: add incremental backup granularity tests
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 21/21] iotests: add incremental backup granularity tests John Snow
@ 2015-04-23 14:29 ` Stefan Hajnoczi
0 siblings, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 14:29 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 454 bytes --]
On Fri, Apr 17, 2015 at 07:50:09PM -0400, John Snow wrote:
> Test what happens if you fiddle with the granularity.
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/124 | 58 +++++++++++++++++++++++++++++++++++++---------
> tests/qemu-iotests/124.out | 4 ++--
> 2 files changed, 49 insertions(+), 13 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (20 preceding siblings ...)
2015-04-17 23:50 ` [Qemu-devel] [PATCH v6 21/21] iotests: add incremental backup granularity tests John Snow
@ 2015-04-23 13:19 ` Stefan Hajnoczi
2015-04-23 14:41 ` John Snow
2015-04-24 14:02 ` [Qemu-devel] " Stefan Hajnoczi
22 siblings, 1 reply; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-23 13:19 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]
On Fri, Apr 17, 2015 at 07:49:48PM -0400, John Snow wrote:
> ===
> v6:
> ===
>
> 01: s/underlaying/underlying/
> Removed a reference to 'disabled' bitmaps.
> Touching up inconsistent list indentation.
> Added FreeBSD Documentation License, primarily to be difficult
Please stick to the currently used set of licenses in the future, unless
you have a strong reason. It's not a good use of anyone's time to fuss
with licenses when we have enough of them in the codebase already.
In my non-lawyer opinion the license you chose seems okay but I'd rather
avoid the risk and hassle.
Thanks,
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series
2015-04-23 13:19 ` [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series Stefan Hajnoczi
@ 2015-04-23 14:41 ` John Snow
2015-04-23 19:18 ` Eric Blake
0 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-04-23 14:41 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
On 04/23/2015 09:19 AM, Stefan Hajnoczi wrote:
> On Fri, Apr 17, 2015 at 07:49:48PM -0400, John Snow wrote:
>> ===
>> v6:
>> ===
>>
>> 01: s/underlaying/underlying/
>> Removed a reference to 'disabled' bitmaps.
>> Touching up inconsistent list indentation.
>> Added FreeBSD Documentation License, primarily to be difficult
>
> Please stick to the currently used set of licenses in the future, unless
> you have a strong reason. It's not a good use of anyone's time to fuss
> with licenses when we have enough of them in the codebase already.
>
> In my non-lawyer opinion the license you chose seems okay but I'd rather
> avoid the risk and hassle.
>
> Thanks,
> Stefan
>
I know I said "primarily to be difficult" but I was just being
facetious. I didn't find the GPL2+ to be suitable for documentation,
strictly, so I went to read up on the documentation licenses that the
fsf support/recommend.
There's the GNU documentation license, but I found that unsuitable for a
couple reasons -- one of them was that you are forbidden(!) from
changing the text of the license, and there are some provisions in there
I didn't like, such as requiring the full text of the license to be
included with compiled copies of the document. That's not something I
care about -- a reference in source, for instance, is sufficient, or a
copy of the license being distributed *with* the compiled source is
fine, but I have no need for the full license to be copied with the
compiled version.
The other documentation license the fsf recommends is the FreeBSD one,
and that one looked appealing, short, and to the point, so it is the one
I chose. It is essentially the FreeBSD license with words altered to
clarify what "code" and "source" means with respect to a document.
Sorry for /actually/ being difficult; but Eric Blake was urging me to
select a license instead of relying on the implicit GPL, so I did go out
of my way to choose one I found appropriate.
I stand by my pick.
--js
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series
2015-04-23 14:41 ` John Snow
@ 2015-04-23 19:18 ` Eric Blake
2015-04-23 19:40 ` John Snow
0 siblings, 1 reply; 52+ messages in thread
From: Eric Blake @ 2015-04-23 19:18 UTC (permalink / raw)
To: John Snow, Stefan Hajnoczi
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]
On 04/23/2015 08:41 AM, John Snow wrote:
> I know I said "primarily to be difficult" but I was just being
> facetious. I didn't find the GPL2+ to be suitable for documentation,
> strictly, so I went to read up on the documentation licenses that the
> fsf support/recommend.
>
> There's the GNU documentation license, but I found that unsuitable for a
> couple reasons -- one of them was that you are forbidden(!) from
> changing the text of the license,
Note that it is usually only the license text proper that is locked
down; the rest of the documentation is not under the same restriction
unless you declare specific invariant sections such as a cover page. But
I know that the Debian project has typically frowned upon any use of FDL
with invariant sections, and the FDL has therefore earned a somewhat
questionable reputation outside of FSF projects.
> and there are some provisions in there
> I didn't like, such as requiring the full text of the license to be
> included with compiled copies of the document. That's not something I
> care about -- a reference in source, for instance, is sufficient, or a
> copy of the license being distributed *with* the compiled source is
> fine, but I have no need for the full license to be copied with the
> compiled version.
Yes, I like those benefits of the FreeBSD Documentation License.
>
> The other documentation license the fsf recommends is the FreeBSD one,
> and that one looked appealing, short, and to the point, so it is the one
> I chose. It is essentially the FreeBSD license with words altered to
> clarify what "code" and "source" means with respect to a document.
In particular, according to the FSF, the FreeBSD Documentation License
_should be_ acceptable for use with a GPLv2 program:
https://www.gnu.org/philosophy/license-list.html#FreeDocumentationLicenses
although this is probably not the right list to get a definitive answer
from a lawyer familiar with the various copyright licenses and laws.
>
> Sorry for /actually/ being difficult; but Eric Blake was urging me to
> select a license instead of relying on the implicit GPL, so I did go out
> of my way to choose one I found appropriate.
>
> I stand by my pick.
I also agree with the pick; I think that GPLv2+ on documentation is a
bit questionable - if someone else implements the same interface using
just the documentation, is their code required to be under the GPL by
virtue of "using" the documentation? Using a more permissive
documentation license feels nicer to me, as it would allow non-GPL
implementations if someone is so inclined. Sorry if encouraging the
issue has made matters more difficult.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series
2015-04-23 19:18 ` Eric Blake
@ 2015-04-23 19:40 ` John Snow
2015-04-24 8:37 ` Stefan Hajnoczi
0 siblings, 1 reply; 52+ messages in thread
From: John Snow @ 2015-04-23 19:40 UTC (permalink / raw)
To: Eric Blake, Stefan Hajnoczi
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
On 04/23/2015 03:18 PM, Eric Blake wrote:
> On 04/23/2015 08:41 AM, John Snow wrote:
>
>> I know I said "primarily to be difficult" but I was just being
>> facetious. I didn't find the GPL2+ to be suitable for documentation,
>> strictly, so I went to read up on the documentation licenses that the
>> fsf support/recommend.
>>
>> There's the GNU documentation license, but I found that unsuitable for a
>> couple reasons -- one of them was that you are forbidden(!) from
>> changing the text of the license,
>
> Note that it is usually only the license text proper that is locked
> down; the rest of the documentation is not under the same restriction
> unless you declare specific invariant sections such as a cover page. But
> I know that the Debian project has typically frowned upon any use of FDL
> with invariant sections, and the FDL has therefore earned a somewhat
> questionable reputation outside of FSF projects.
>
Understood; however the GNU FDL specifies within the license where and
how the GNU FDL must be displayed. I didn't like these requirements, and
might've used the FDL, but you are prohibited from altering the license,
so I chose against this license.
It's too restrictive for me.
>> and there are some provisions in there
>> I didn't like, such as requiring the full text of the license to be
>> included with compiled copies of the document. That's not something I
>> care about -- a reference in source, for instance, is sufficient, or a
>> copy of the license being distributed *with* the compiled source is
>> fine, but I have no need for the full license to be copied with the
>> compiled version.
>
> Yes, I like those benefits of the FreeBSD Documentation License.
>
>>
>> The other documentation license the fsf recommends is the FreeBSD one,
>> and that one looked appealing, short, and to the point, so it is the one
>> I chose. It is essentially the FreeBSD license with words altered to
>> clarify what "code" and "source" means with respect to a document.
>
> In particular, according to the FSF, the FreeBSD Documentation License
> _should be_ acceptable for use with a GPLv2 program:
>
> https://www.gnu.org/philosophy/license-list.html#FreeDocumentationLicenses
>
> although this is probably not the right list to get a definitive answer
> from a lawyer familiar with the various copyright licenses and laws.
>
>>
>> Sorry for /actually/ being difficult; but Eric Blake was urging me to
>> select a license instead of relying on the implicit GPL, so I did go out
>> of my way to choose one I found appropriate.
>>
>> I stand by my pick.
>
> I also agree with the pick; I think that GPLv2+ on documentation is a
> bit questionable - if someone else implements the same interface using
> just the documentation, is their code required to be under the GPL by
> virtue of "using" the documentation? Using a more permissive
> documentation license feels nicer to me, as it would allow non-GPL
> implementations if someone is so inclined. Sorry if encouraging the
> issue has made matters more difficult.
>
It's too late! You've opened Pandora's Box!
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series
2015-04-23 19:40 ` John Snow
@ 2015-04-24 8:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-24 8:37 UTC (permalink / raw)
To: John Snow
Cc: famz, qemu-block, qemu-devel, armbru, vsementsov, stefanha,
mreitz
[-- Attachment #1: Type: text/plain, Size: 170 bytes --]
On Thu, Apr 23, 2015 at 03:40:03PM -0400, John Snow wrote:
> It's too late! You've opened Pandora's Box!
This is why it's a waste of time to play with licenses.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series
2015-04-17 23:49 [Qemu-devel] [PATCH v6 00/21] block: transactionless incremental backup series John Snow
` (21 preceding siblings ...)
2015-04-23 13:19 ` [Qemu-devel] [Qemu-block] [PATCH v6 00/21] block: transactionless incremental backup series Stefan Hajnoczi
@ 2015-04-24 14:02 ` Stefan Hajnoczi
22 siblings, 0 replies; 52+ messages in thread
From: Stefan Hajnoczi @ 2015-04-24 14:02 UTC (permalink / raw)
To: John Snow; +Cc: kwolf, famz, qemu-block, qemu-devel, armbru, vsementsov, mreitz
[-- Attachment #1: Type: text/plain, Size: 10230 bytes --]
On Fri, Apr 17, 2015 at 07:49:48PM -0400, John Snow wrote:
> It's spring! The winter snow has thawed and so here is a new
> patch series to enter your life and warm your heart.
>
> This patchset enables the in-memory part of the incremental backup
> feature, without transactional support.
>
> Support for transactions was separated into a separate series which
> is also now available on-list. Getting this portion of the series
> committed will help stabilize work on bitmap persistence and bitmap
> migration.
>
> Thanks to Fam Zheng for the original versions of this patchset,
> And thanks to Max and Eric for reviewing 2,396 versions of it since.
>
> ===
> v6:
> ===
>
> 01: s/underlaying/underlying/
> Removed a reference to 'disabled' bitmaps.
> Touching up inconsistent list indentation.
> Added FreeBSD Documentation License, primarily to be difficult
> 07: More in-line documentation for hbitmap_merge, for return value.
> Fix size cache index to be uint64_t.
> 09: Grammar fixes from Eric Blake, kept R-Bs.
> 10: Moved yield into the do{}while(). Now we check to see if we should
> yield/cancel after each unsuccessful sector we transfer.
> Some documentation additions for Eric Blake.
> 15: corrected 'num_elements' to 'start'
> 18: Refactored qmp.py event functions,
> Added in more explicit exception classes.
> No changes to iotests.py, just qmp.py.
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/21:[0057] [FC] 'docs: incremental backup documentation'
> 002/21:[----] [--] 'qapi: Add optional field "name" to block dirty bitmap'
> 003/21:[----] [--] 'qmp: Ensure consistent granularity type'
> 004/21:[----] [--] 'qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove'
> 005/21:[----] [--] 'block: Introduce bdrv_dirty_bitmap_granularity()'
> 006/21:[----] [--] 'hbitmap: cache array lengths'
> 007/21:[0008] [FC] 'hbitmap: add hbitmap_merge'
> 008/21:[----] [--] 'block: Add bitmap disabled status'
> 009/21:[0008] [FC] 'block: Add bitmap successors'
> 010/21:[0013] [FC] 'qmp: Add support of "dirty-bitmap" sync mode for drive-backup'
> 011/21:[----] [--] 'qmp: add block-dirty-bitmap-clear'
> 012/21:[----] [--] 'qmp: Add dirty bitmap status field in query-block'
> 013/21:[----] [--] 'block: add BdrvDirtyBitmap documentation'
> 014/21:[----] [--] 'block: Ensure consistent bitmap function prototypes'
> 015/21:[0002] [FC] 'block: Resize bitmaps on bdrv_truncate'
> 016/21:[----] [--] 'hbitmap: truncate tests'
> 017/21:[----] [-C] 'iotests: add invalid input incremental backup tests'
> 018/21:[0095] [FC] 'iotests: add QMP event waiting queue'
> 019/21:[----] [--] 'iotests: add simple incremental backup case'
> 020/21:[----] [--] 'iotests: add incremental backup failure recovery test'
> 021/21:[----] [--] 'iotests: add incremental backup granularity tests'
>
> ===
> v5:
> ===
>
> 10: Code has been moved into backup_run_incremental()
> 'polyrhythm' check is removed,
> clusters_per_iter variable is introduced instead.
> If the bitmap granularity is larger than the backup granularity,
> loop over the backup_do_cow call multiple times.
> If the bitmap granularity is smaller, skip the iterator ahead as
> we had been doing previously.
> 14: Only whitespace changes caused by patch 10.
> 15: Changed my approach for clearing out data for the hbitmap
> truncate shrink case, as suggested by Stefan
> 18: Added a proper timeout mechanism to qmp.pull_event():
> wait=False or wait=0.0 implies non-blocking.
> wait=True implies blocking.
> wait=60.0 implies a 60 second timeout.
> VM.event_wait() now uses a 60 second timeout by default.
> 19: Many things:
> The big picture is to add a set of full backups alongside the
> incremental backups created during the test to be able to test
> the validity of each incremental at the conclusion of the test
> when we can shut the VM down.
>
> To do this, there are two basic changes:
> (1) Keep a list of pairs of backup filenames (incremental, reference);
> create a full reference backup for every incremental created.
> (2) Refactor the backup helper functions a bit.
> 20: Naming fallout from 19
> Added calls to vm.shutdown() and check_backups().
> 21: NEW, adds granularity tests that cover the changes in patch 10.
>
> ===
> v4:
> ===
>
> 04: Some in-line documentation for block_dirty_bitmap_lookup
> Changed behavior with respect to aio_context
> (always acquire, release if pbs == null, give to user otherwise)
> 10: Removed vestigial (currently nop) bdrv_enable_dirty_bitmap call
> Kept R-B.
> 16: Added some comments to test_check_boundary_bits.
> Kept R-B.
> 17: Folded in refactor from "incremental transactions v1" (Poor Kitty)
> 18: Pulled forward from "incremental transactions v1"
> Kept R-B from that series.
> 19: Folded in refactor from "incremental transactions v1"
> Added offset assertions into wait_incremental
> 20: Removed error tolerance from wait_until_completed, as
> these patches no longer make use of that feature.
>
> ===
> v3:
> ===
>
> 01: Removed enabled/disabled modes information.
> Elaborated on events that can occur during error cases.
> 04: Added an AioContext out parameter to block_dirty_bitmap_lookup.
> 06: NEW:
> Cache the array lengths for hbitmap.
> 07: hbitmap_merge now uses the cached array lengths.
> 11: block-dirty-bitmap-clear is edited for the new block_dirty_bitmap_lookup.
> 12: Removed the "disabled" status, leaving just "Frozen."
> 15: Moved bdrv_truncate_dirty_bitmap to be static local
> Inlined dirty_bitmap_truncate function.
> Removed size[] caching into new patch (06, above)
> hbitmap_truncate now keeps correct bit population count
> hbitmap_truncate now uses hbitmap_reset BEFORE the truncate,
> to avoid tricky out-of-bounds usages.
> Remove g_realloc_n call that is not available in glib 2.12 (or 2.22)
> Renamed "truncate" to "shrink" to make that more clear
> to people who aren't me (at last count: 7+ billion)
> 16 NEW:
> hbitmap_truncate tests.
>
> ===
> v2:
> ===
>
> 01: Added a new opening blurb.
> Adjusted codeblock indentations to be 4 spaces instead of 3,
> so it works as MD or GFMD.
> Adjusted errors explanation.
> Make visual separations between json data and shell commands
> Eliminate any ligering single quotes
>
> 07: Remember that memset takes bytes, not n_items ...
>
> ===
> v1:
> ===
>
> Deletions:
> - Removed Transactions, to be added later.
> - Removed Transaction tests, as above.
>
> Changes:
> 01: Indentation fixes.
> Removed enable/disable documentation.
> Added a note that transactions aren't implemented yet.
> Removed my needless commas
> Added error case documentation.
>
> 07: QMP enable/disable commands are deleted.
>
> 14: Some comments concerning assertions.
> Scrub re-alloc memory if we expand the array.
> Do not attempt to scrub memory if fix_count is 0
>
> Changes made with Reviews kept:
>
> 02: Since 2.4
> 04: Since 2.4
> Demingled the QMP command documentation.
> 08: Additions to what was qmp_block_dirty_enable/disable
> are no longer present as those function no longer exist.
> 09: Since 2.4
> 10: Since 2.4
> Demingled QMP command documentation.
> 11: Since 2.4
> 15: Test 112 --> 124
> 17: Number of tests altered. (Only 4, now.)
>
> Fam Zheng (1):
> qapi: Add optional field "name" to block dirty bitmap
>
> John Snow (20):
> docs: incremental backup documentation
> qmp: Ensure consistent granularity type
> qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
> block: Introduce bdrv_dirty_bitmap_granularity()
> hbitmap: cache array lengths
> hbitmap: add hbitmap_merge
> block: Add bitmap disabled status
> block: Add bitmap successors
> qmp: Add support of "dirty-bitmap" sync mode for drive-backup
> qmp: add block-dirty-bitmap-clear
> qmp: Add dirty bitmap status field in query-block
> block: add BdrvDirtyBitmap documentation
> block: Ensure consistent bitmap function prototypes
> block: Resize bitmaps on bdrv_truncate
> hbitmap: truncate tests
> iotests: add invalid input incremental backup tests
> iotests: add QMP event waiting queue
> iotests: add simple incremental backup case
> iotests: add incremental backup failure recovery test
> iotests: add incremental backup granularity tests
>
> block.c | 243 ++++++++++++++++++++++++++--
> block/backup.c | 155 +++++++++++++++---
> block/mirror.c | 46 +++---
> blockdev.c | 176 +++++++++++++++++++-
> docs/bitmaps.md | 352 ++++++++++++++++++++++++++++++++++++++++
> hmp.c | 3 +-
> include/block/block.h | 33 +++-
> include/block/block_int.h | 4 +-
> include/qemu/hbitmap.h | 23 +++
> migration/block.c | 9 +-
> qapi/block-core.json | 91 ++++++++++-
> qmp-commands.hx | 93 ++++++++++-
> scripts/qmp/qmp.py | 95 +++++++----
> tests/qemu-iotests/124 | 363 ++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/124.out | 5 +
> tests/qemu-iotests/group | 1 +
> tests/qemu-iotests/iotests.py | 38 +++++
> tests/test-hbitmap.c | 255 +++++++++++++++++++++++++++++
> util/hbitmap.c | 85 ++++++++++
> 19 files changed, 1953 insertions(+), 117 deletions(-)
> create mode 100644 docs/bitmaps.md
> create mode 100644 tests/qemu-iotests/124
> create mode 100644 tests/qemu-iotests/124.out
>
> --
> 2.1.0
>
Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread