On 10.07.19 03:05, John Snow wrote: > This test needs support for non-bitmap backups and missing or > unspecified bitmap sync modes, so rewrite the helpers to be a little > more generic. > > Signed-off-by: John Snow <[email protected]> > --- > tests/qemu-iotests/257 | 46 +++++---- > tests/qemu-iotests/257.out | 192 ++++++++++++++++++------------------- > 2 files changed, 124 insertions(+), 114 deletions(-) > > diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257 > index 2ff4aa8695..2eb4f26c28 100755 > --- a/tests/qemu-iotests/257 > +++ b/tests/qemu-iotests/257
[...]
> -def bitmap_backup(drive, n, filepath, bitmap, bitmap_mode):
> - log("--- Bitmap Backup #{:d} ---\n".format(n))
> - target_id = "bitmap_target_{:d}".format(n)
> - job_id = "bitmap_backup_{:d}".format(n)
> +def backup(drive, n, filepath, bitmap, bitmap_mode, sync='bitmap'):
> + log("--- Test Backup #{:d} ---\n".format(n))
> + target_id = "backup_target_{:d}".format(n)
> + job_id = "backup_{:d}".format(n)
> target_drive = Drive(filepath, vm=drive.vm)
>
> target_drive.create_target(target_id, drive.fmt, drive.size)
> - drive.vm.qmp_log("blockdev-backup", job_id=job_id, device=drive.name,
> - target=target_id, sync="bitmap",
> - bitmap_mode=bitmap_mode,
> - bitmap=bitmap,
> - auto_finalize=False)
> +
> + kwargs = {
> + 'job_id': job_id,
> + 'auto_finalize': False,
> + 'bitmap': bitmap,
> + 'bitmap_mode': bitmap_mode,
> + }
> + kwargs = {key: val for key, val in kwargs.items() if val is not None}
I suppose this is to remove items that are None?
Very cute, but why not just
kwargs = {
'job_id': job_id,
'auto_finalize': False,
}
if bitmap is not None:
kwargs['bitmap'] = bitmap
kwargs['bitmap_mode'] = bitmap_mode
Exactly the same number of lines, but immediately makes it clear what’s
going on. Not as cute, I admit.
(Yes, I am indeed actively trying to train you not to write cute code.)
The rest looks good to me:
Reviewed-by: Max Reitz <[email protected]>
signature.asc
Description: OpenPGP digital signature
