* [Qemu-devel] [PATCH 0/2] block: add 'bitmap' argument to blockdev-backup
@ 2018-08-30 21:16 John Snow
2018-08-30 21:16 ` [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument John Snow
2018-08-30 21:16 ` [Qemu-devel] [PATCH 2/2] HACK: test blockdev-backup instead of drive-backup John Snow
0 siblings, 2 replies; 4+ messages in thread
From: John Snow @ 2018-08-30 21:16 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Markus Armbruster, Kevin Wolf, Max Reitz, Eric Blake, John Snow
Eric brought to my attention that we don't currently allow
incremental backups made to a node. There's no reason we can't,
though, so enable this.
A better test in en-route, this is mostly a PoC chainsaw job
on the second patch to see if patchew knows something I don't.
--js
John Snow (2):
blockdev-backup: add bitmap argument
HACK: test blockdev-backup instead of drive-backup
blockdev.c | 16 +++++++-
qapi/block-core.json | 7 +++-
tests/qemu-iotests/124 | 99 ++++++++++++++++++++++++++++++++++----------------
3 files changed, 88 insertions(+), 34 deletions(-)
--
2.14.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument
2018-08-30 21:16 [Qemu-devel] [PATCH 0/2] block: add 'bitmap' argument to blockdev-backup John Snow
@ 2018-08-30 21:16 ` John Snow
2018-09-13 19:52 ` John Snow
2018-08-30 21:16 ` [Qemu-devel] [PATCH 2/2] HACK: test blockdev-backup instead of drive-backup John Snow
1 sibling, 1 reply; 4+ messages in thread
From: John Snow @ 2018-08-30 21:16 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Markus Armbruster, Kevin Wolf, Max Reitz, Eric Blake, John Snow
It is only an oversight that we don't allow incremental backup with
blockdev-backup. Add the bitmap argument which enables this.
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 16 +++++++++++++++-
qapi/block-core.json | 7 ++++++-
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 72f5347df5..b9f19afacc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3491,6 +3491,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
BlockDriverState *bs;
BlockDriverState *target_bs;
Error *local_err = NULL;
+ BdrvDirtyBitmap *bmap = NULL;
AioContext *aio_context;
BlockJob *job = NULL;
int job_flags = JOB_DEFAULT;
@@ -3541,6 +3542,19 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
goto out;
}
}
+ if (backup->has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
+ if (!bmap) {
+ error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
+ goto out;
+ }
+ if (bdrv_dirty_bitmap_qmp_locked(bmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently locked and cannot be used for "
+ "backup", backup->bitmap);
+ goto out;
+ }
+ }
if (!backup->auto_finalize) {
job_flags |= JOB_MANUAL_FINALIZE;
}
@@ -3548,7 +3562,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
job_flags |= JOB_MANUAL_DISMISS;
}
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
- backup->sync, NULL, backup->compress,
+ backup->sync, bmap, backup->compress,
backup->on_source_error, backup->on_target_error,
job_flags, NULL, NULL, txn, &local_err);
if (local_err != NULL) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4c7a37afdc..d3ea281039 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1315,6 +1315,10 @@
# @speed: the maximum speed, in bytes per second. The default is 0,
# for unlimited.
#
+# @bitmap: the name of dirty bitmap if sync is "incremental".
+# Must be present if sync is "incremental", must NOT be present
+# otherwise. (Since 3.1)
+#
# @compress: true to compress data, if the target format supports it.
# (default: false) (since 2.8)
#
@@ -1346,7 +1350,8 @@
##
{ 'struct': 'BlockdevBackup',
'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
- 'sync': 'MirrorSyncMode', '*speed': 'int', '*compress': 'bool',
+ 'sync': 'MirrorSyncMode', '*speed': 'int',
+ '*bitmap': 'str', '*compress': 'bool',
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
--
2.14.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 2/2] HACK: test blockdev-backup instead of drive-backup
2018-08-30 21:16 [Qemu-devel] [PATCH 0/2] block: add 'bitmap' argument to blockdev-backup John Snow
2018-08-30 21:16 ` [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument John Snow
@ 2018-08-30 21:16 ` John Snow
1 sibling, 0 replies; 4+ messages in thread
From: John Snow @ 2018-08-30 21:16 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: Markus Armbruster, Kevin Wolf, Max Reitz, Eric Blake, John Snow
This is just a real chainsaw job on 124 to prove that we can
indeed use blockdev-backup interchangeably with drive-backup
for incremental backups.
A nicer test will follow once I refactor this a bit to look
a little less like the Texas Chainsaw Massacre.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/124 | 99 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 67 insertions(+), 32 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 3ea4ac53f5..b5a1f5e248 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -49,7 +49,7 @@ def transaction_bitmap_clear(node, name, **kwargs):
def transaction_drive_backup(device, target, **kwargs):
- return transaction_action('drive-backup', job_id=device, device=device,
+ return transaction_action('blockdev-backup', job_id=device, device=device,
target=target, **kwargs)
@@ -146,10 +146,22 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
def do_qmp_backup(self, error='Input/output error', **kwargs):
- res = self.vm.qmp('drive-backup', **kwargs)
+ res = self.vm.qmp('blockdev-backup', **kwargs)
self.assert_qmp(res, 'return', {})
return self.wait_qmp_backup(kwargs['device'], error)
+ def _FIXME_new_target(self, name, fmt, filename):
+ result = self.vm.qmp('blockdev-add',
+ node_name=name,
+ driver=fmt,
+ file={"driver": "file",
+ "filename": filename})
+ self.assert_qmp(result, 'return', {})
+
+ def _FIXME_del_target(self, name):
+ result = self.vm.qmp('blockdev-del',
+ node_name=name)
+ self.assert_qmp(result, 'return', {})
def ignore_job_status_change_events(self):
while True:
@@ -185,11 +197,13 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
def create_anchor_backup(self, drive=None):
if drive is None:
drive = self.drives[-1]
+ self.img_create(drive['backup'], drive['fmt'])
+ self._FIXME_new_target('target', drive['fmt'], drive['backup'])
res = self.do_qmp_backup(job_id=drive['id'],
device=drive['id'], sync='full',
- format=drive['fmt'], target=drive['backup'])
+ target='target')
+ self._FIXME_del_target('target')
self.assertTrue(res)
- self.files.append(drive['backup'])
return drive['backup']
@@ -197,9 +211,12 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
if bitmap is None:
bitmap = self.bitmaps[-1]
_, reference = bitmap.last_target()
+ self.img_create(reference, bitmap.drive['fmt'])
+ self._FIXME_new_target('target', bitmap.drive['fmt'], reference)
res = self.do_qmp_backup(job_id=bitmap.drive['id'],
device=bitmap.drive['id'], sync='full',
- format=bitmap.drive['fmt'], target=reference)
+ target='target')
+ self._FIXME_del_target('target')
self.assertTrue(res)
@@ -223,19 +240,19 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
return target
- def create_incremental(self, bitmap=None, parent=None,
- parentFormat=None, validate=True):
+ def create_incremental(self, bitmap=None, parent=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)
+ self._FIXME_new_target('target', bitmap.drive['fmt'], target)
res = self.do_qmp_backup(job_id=bitmap.drive['id'],
device=bitmap.drive['id'],
sync='incremental', bitmap=bitmap.name,
- format=bitmap.drive['fmt'], target=target,
- mode='existing')
+ target='target')
+ self._FIXME_del_target('target')
if not res:
bitmap.del_target();
self.assertFalse(validate)
@@ -331,10 +348,11 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
# Create a cluster_size=128k full backup / "anchor" backup
self.img_create(drive0['backup'], cluster_size='128k')
+ self.img_create(drive0['backup'], drive0['fmt'])
+ self._FIXME_new_target('target', drive0['fmt'], drive0['backup'])
self.assertTrue(self.do_qmp_backup(device=drive0['id'], sync='full',
- format=drive0['fmt'],
- target=drive0['backup'],
- mode='existing'))
+ target='target'))
+ self._FIXME_del_target('target')
# Create bitmap and dirty it with some new writes.
# overwrite [32736, 32799] which will dirty bitmap clusters at
@@ -356,12 +374,12 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.img_create(target, bitmap0.drive['fmt'], cluster_size='128k')
# Perform Incremental Backup
+ self._FIXME_new_target('target', bitmap0.drive['fmt'], target)
self.assertTrue(self.do_qmp_backup(device=bitmap0.drive['id'],
sync='incremental',
bitmap=bitmap0.name,
- format=bitmap0.drive['fmt'],
- target=target,
- mode='existing'))
+ target='target'))
+ self._FIXME_del_target('target')
self.make_reference_backup(bitmap0)
# Add the backing file, then compare and exit.
@@ -388,15 +406,18 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
('0x64', '32736k', '64k')))
bitmap1 = self.add_bitmap('bitmap1', drive0)
+ # Create and mount image
+ self.img_create(drive0['backup'], drive0['fmt'])
+ self._FIXME_new_target('target', drive0['fmt'], drive0['backup'])
+
result = self.vm.qmp('transaction', actions=[
transaction_bitmap_clear(bitmap0.drive['id'], bitmap0.name),
transaction_bitmap_clear(bitmap1.drive['id'], bitmap1.name),
- transaction_drive_backup(drive0['id'], drive0['backup'],
- sync='full', format=drive0['fmt'])
+ transaction_drive_backup(drive0['id'], 'target', sync='full')
])
self.assert_qmp(result, 'return', {})
self.wait_until_completed(drive0['id'])
- self.files.append(drive0['backup'])
+ self._FIXME_del_target('target')
self.hmp_io_writes(drive0['id'], (('0x9a', 0, 512),
('0x55', '8M', '352k'),
@@ -464,16 +485,18 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
target0 = self.prepare_backup(dr0bm0)
target1 = self.prepare_backup(dr1bm0)
+ # Mount targets
+ self._FIXME_new_target('target0', drive0['fmt'], target0)
+ self._FIXME_new_target('target1', drive1['fmt'], target1)
+
# Ask for a new incremental backup per-each drive,
# expecting drive1's backup to fail. In the 'race' test,
# we expect drive1 to attempt to cancel the empty drive0 job.
transaction = [
- transaction_drive_backup(drive0['id'], target0, sync='incremental',
- format=drive0['fmt'], mode='existing',
- bitmap=dr0bm0.name),
- transaction_drive_backup(drive1['id'], target1, sync='incremental',
- format=drive1['fmt'], mode='existing',
- bitmap=dr1bm0.name)
+ transaction_drive_backup(drive0['id'], 'target0',
+ sync='incremental', bitmap=dr0bm0.name),
+ transaction_drive_backup(drive1['id'], 'target1',
+ sync='incremental', bitmap=dr1bm0.name)
]
result = self.vm.qmp('transaction', actions=transaction,
properties={'completion-mode': 'grouped'} )
@@ -490,6 +513,10 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.assertFalse(self.vm.get_qmp_events(wait=False))
self.assert_no_active_block_jobs()
+ # Unmount targets
+ self._FIXME_del_target('target0')
+ self._FIXME_del_target('target1')
+
# Delete drive0's successful target and eliminate our record of the
# unsuccessful drive1 target.
dr0bm0.del_target()
@@ -499,9 +526,11 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.vm.shutdown()
return
- # Re-run the same transaction:
+ # Re-run the same transaction; create and mount images
target0 = self.prepare_backup(dr0bm0)
target1 = self.prepare_backup(dr1bm0)
+ self._FIXME_new_target('target0', drive0['fmt'], target0)
+ self._FIXME_new_target('target1', drive1['fmt'], target1)
# Re-run the exact same transaction.
result = self.vm.qmp('transaction', actions=transaction,
@@ -516,6 +545,9 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.assertFalse(self.vm.get_qmp_events(wait=False))
self.assert_no_active_block_jobs()
+ self._FIXME_del_target('target0')
+ self._FIXME_del_target('target1')
+
# And the images should of course validate.
self.vm.shutdown()
self.check_backups()
@@ -544,19 +576,22 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
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='incremental', format=self.drives[0]['fmt'],
- target=self.err_img)
+ self.img_create(self.err_img, iotests.imgfmt)
+ self._FIXME_new_target('err0', iotests.imgfmt, self.err_img)
+ result = self.vm.qmp('blockdev-backup', device=self.drives[0]['id'],
+ sync='incremental', target='err0')
+ self._FIXME_del_target('err0')
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'],
+ self.img_create(self.err_img, iotests.imgfmt)
+ self._FIXME_new_target('err0', iotests.imgfmt, self.err_img)
+ result = self.vm.qmp('blockdev-backup', device=self.drives[0]['id'],
sync='incremental', bitmap='unknown',
- format=self.drives[0]['fmt'], target=self.err_img)
+ target='err0')
+ self._FIXME_del_target('err0')
self.assert_qmp(result, 'error/class', 'GenericError')
--
2.14.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument
2018-08-30 21:16 ` [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument John Snow
@ 2018-09-13 19:52 ` John Snow
0 siblings, 0 replies; 4+ messages in thread
From: John Snow @ 2018-09-13 19:52 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: Kevin Wolf, Markus Armbruster, Max Reitz
On 08/30/2018 05:16 PM, John Snow wrote:
> It is only an oversight that we don't allow incremental backup with
> blockdev-backup. Add the bitmap argument which enables this.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 16 +++++++++++++++-
> qapi/block-core.json | 7 ++++++-
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 72f5347df5..b9f19afacc 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3491,6 +3491,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
> BlockDriverState *bs;
> BlockDriverState *target_bs;
> Error *local_err = NULL;
> + BdrvDirtyBitmap *bmap = NULL;
> AioContext *aio_context;
> BlockJob *job = NULL;
> int job_flags = JOB_DEFAULT;
> @@ -3541,6 +3542,19 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
> goto out;
> }
> }
> + if (backup->has_bitmap) {
> + bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
> + if (!bmap) {
> + error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
> + goto out;
> + }
> + if (bdrv_dirty_bitmap_qmp_locked(bmap)) {
> + error_setg(errp,
> + "Bitmap '%s' is currently locked and cannot be used for "
> + "backup", backup->bitmap);
> + goto out;
> + }
> + }
> if (!backup->auto_finalize) {
> job_flags |= JOB_MANUAL_FINALIZE;
> }
> @@ -3548,7 +3562,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
> job_flags |= JOB_MANUAL_DISMISS;
> }
> job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
> - backup->sync, NULL, backup->compress,
> + backup->sync, bmap, backup->compress,
> backup->on_source_error, backup->on_target_error,
> job_flags, NULL, NULL, txn, &local_err);
> if (local_err != NULL) {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4c7a37afdc..d3ea281039 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1315,6 +1315,10 @@
> # @speed: the maximum speed, in bytes per second. The default is 0,
> # for unlimited.
> #
> +# @bitmap: the name of dirty bitmap if sync is "incremental".
> +# Must be present if sync is "incremental", must NOT be present
> +# otherwise. (Since 3.1)
> +#
> # @compress: true to compress data, if the target format supports it.
> # (default: false) (since 2.8)
> #
> @@ -1346,7 +1350,8 @@
> ##
> { 'struct': 'BlockdevBackup',
> 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
> - 'sync': 'MirrorSyncMode', '*speed': 'int', '*compress': 'bool',
> + 'sync': 'MirrorSyncMode', '*speed': 'int',
> + '*bitmap': 'str', '*compress': 'bool',
> '*on-source-error': 'BlockdevOnError',
> '*on-target-error': 'BlockdevOnError',
> '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
>
I'll be staging this patch (and not the hack that follows it), if there
are no objections.
Thanks, applied to my bitmaps tree:
https://github.com/jnsnow/qemu/commits/bitmaps
https://github.com/jnsnow/qemu.git
--js
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-13 19:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 21:16 [Qemu-devel] [PATCH 0/2] block: add 'bitmap' argument to blockdev-backup John Snow
2018-08-30 21:16 ` [Qemu-devel] [PATCH 1/2] blockdev-backup: add bitmap argument John Snow
2018-09-13 19:52 ` John Snow
2018-08-30 21:16 ` [Qemu-devel] [PATCH 2/2] HACK: test blockdev-backup instead of drive-backup John Snow
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.