| Message ID | 1482503344-6424-9-git-send-email-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 12/23 17:28, Vladimir Sementsov-Ogievskiy wrote: > diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 > index 1d3fd04..388b7b2 100755 > --- a/tests/qemu-iotests/055 > +++ b/tests/qemu-iotests/055 > @@ -30,6 +30,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img') > blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img') > > image_len = 64 * 1024 * 1024 # MB > +pause_write = '3M' > > def setUpModule(): > qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len)) > @@ -39,6 +40,7 @@ def setUpModule(): > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img) > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img) > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img) > + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 40M ' + pause_write, test_img) What does this iotest change do? Fam > > def tearDownModule(): > os.remove(test_img) > -- > 1.8.3.1 >
24.01.2017 10:59, Fam Zheng wrote: > On Fri, 12/23 17:28, Vladimir Sementsov-Ogievskiy wrote: >> diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 >> index 1d3fd04..388b7b2 100755 >> --- a/tests/qemu-iotests/055 >> +++ b/tests/qemu-iotests/055 >> @@ -30,6 +30,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img') >> blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img') >> >> image_len = 64 * 1024 * 1024 # MB >> +pause_write = '3M' >> >> def setUpModule(): >> qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len)) >> @@ -39,6 +40,7 @@ def setUpModule(): >> qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img) >> qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img) >> qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img) >> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 40M ' + pause_write, test_img) > What does this iotest change do? Without this backup block-job finishes before the next query to it from test and test fails. This is a wide problem I suffer of on the way of backup improvement and it should have better solution then adjusting amount of data being copied... > > Fam > >> >> def tearDownModule(): >> os.remove(test_img) >> -- >> 1.8.3.1 >>
On Tue, 01/24 12:18, Vladimir Sementsov-Ogievskiy wrote: > 24.01.2017 10:59, Fam Zheng wrote: > > On Fri, 12/23 17:28, Vladimir Sementsov-Ogievskiy wrote: > > > diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 > > > index 1d3fd04..388b7b2 100755 > > > --- a/tests/qemu-iotests/055 > > > +++ b/tests/qemu-iotests/055 > > > @@ -30,6 +30,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img') > > > blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img') > > > image_len = 64 * 1024 * 1024 # MB > > > +pause_write = '3M' > > > def setUpModule(): > > > qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len)) > > > @@ -39,6 +40,7 @@ def setUpModule(): > > > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img) > > > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img) > > > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img) > > > + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 40M ' + pause_write, test_img) > > What does this iotest change do? > > Without this backup block-job finishes before the next query to it from test > and test fails. This is a wide problem I suffer of on the way of backup > improvement and it should have better solution then adjusting amount of data > being copied... You can use blkdebug to pause I/O before querying the block job, or simply throttle it down. Fam
24.01.2017 12:36, Fam Zheng wrote: > On Tue, 01/24 12:18, Vladimir Sementsov-Ogievskiy wrote: >> 24.01.2017 10:59, Fam Zheng wrote: >>> On Fri, 12/23 17:28, Vladimir Sementsov-Ogievskiy wrote: >>>> diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 >>>> index 1d3fd04..388b7b2 100755 >>>> --- a/tests/qemu-iotests/055 >>>> +++ b/tests/qemu-iotests/055 >>>> @@ -30,6 +30,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img') >>>> blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img') >>>> image_len = 64 * 1024 * 1024 # MB >>>> +pause_write = '3M' >>>> def setUpModule(): >>>> qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len)) >>>> @@ -39,6 +40,7 @@ def setUpModule(): >>>> qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img) >>>> qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img) >>>> qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img) >>>> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 40M ' + pause_write, test_img) >>> What does this iotest change do? >> Without this backup block-job finishes before the next query to it from test >> and test fails. This is a wide problem I suffer of on the way of backup >> improvement and it should have better solution then adjusting amount of data >> being copied... > You can use blkdebug to pause I/O before querying the block job, or simply > throttle it down. > > Fam throttling down is not better I thing then just copying additional 3M, it is environment-dependent hack too. And we can't use throttling in all tests - throttling should be itself tested separately. In the other thread there was an idea to not delete block jobs on their finish, but maintain them in some 'finished' state. This will be great for the case and will be generic.
On Tue, 01/24 13:13, Vladimir Sementsov-Ogievskiy wrote: > 24.01.2017 12:36, Fam Zheng wrote: > > On Tue, 01/24 12:18, Vladimir Sementsov-Ogievskiy wrote: > > > 24.01.2017 10:59, Fam Zheng wrote: > > > > On Fri, 12/23 17:28, Vladimir Sementsov-Ogievskiy wrote: > > > > > diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 > > > > > index 1d3fd04..388b7b2 100755 > > > > > --- a/tests/qemu-iotests/055 > > > > > +++ b/tests/qemu-iotests/055 > > > > > @@ -30,6 +30,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img') > > > > > blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img') > > > > > image_len = 64 * 1024 * 1024 # MB > > > > > +pause_write = '3M' > > > > > def setUpModule(): > > > > > qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len)) > > > > > @@ -39,6 +40,7 @@ def setUpModule(): > > > > > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img) > > > > > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img) > > > > > qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img) > > > > > + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 40M ' + pause_write, test_img) > > > > What does this iotest change do? > > > Without this backup block-job finishes before the next query to it from test > > > and test fails. This is a wide problem I suffer of on the way of backup > > > improvement and it should have better solution then adjusting amount of data > > > being copied... > > You can use blkdebug to pause I/O before querying the block job, or simply > > throttle it down. > > > > Fam > > throttling down is not better I thing then just copying additional 3M, it is > environment-dependent hack too. And we can't use throttling in all tests - > throttling should be itself tested separately. Then maybe opt to blkdebug? I didn't mean the speed parameter of block jobs, but the block_set_io_throttle. It is not perfect, but still better than simply enlarging the data size, and is slightly simpler than blkdebug pause/resume. Fam
On Fri, Dec 23, 2016 at 05:28:51PM +0300, Vladimir Sementsov-Ogievskiy wrote: > In case of full backup we can skip unallocated clusters if the target > disk is already zero-initialized. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/backup.c | 8 ++++++-- > tests/qemu-iotests/055 | 2 ++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index 2afd1b6..4ef8daf 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -562,9 +562,13 @@ static void coroutine_fn backup_run(void *opaque) > if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) { > backup_incremental_init_copy_bitmap(job); > } else { > + /* top or full mode */ > + bool is_top = job->sync_mode == MIRROR_SYNC_MODE_TOP; > + BlockDriverState *base = > + is_top ? backing_bs(blk_bs(job->common.blk)) : NULL; > hbitmap_set(job->copy_bitmap, 0, end); > - if (job->sync_mode == MIRROR_SYNC_MODE_TOP) { > - backup_skip_loop(job, backing_bs(blk_bs(job->common.blk))); > + if (is_top || bdrv_has_zero_init(blk_bs(job->target))) { > + backup_skip_loop(job, base); > } An alternative that I find easier to read: hbitmap_set(job->copy_bitmap, 0, end); if (job->sync_mode == MIRROR_SYNC_MODE_TOP) { backup_skip_loop(job, backing_bs(blk_bs(job->common.blk))); } else if (bdrv_has_zero_init(blk_bs(job->target))) { backup_skip_loop(job, NULL); }
On Fri, Dec 23, 2016 at 05:28:51PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> + if (is_top || bdrv_has_zero_init(blk_bs(job->target))) {
/*
* Returns 1 if newly created images are guaranteed to contain
* only zeros, 0 otherwise.
*/
int (*bdrv_has_zero_init)(BlockDriverState *bs);
This function does not cover arbitrary images opened with bdrv_open(),
only those created with bdrv_img_create() immediately before opening.
This means that the check is only valid with backup->mode !=
NEW_IMAGE_MODE_EXISTING.
diff --git a/block/backup.c b/block/backup.c index 2afd1b6..4ef8daf 100644 --- a/block/backup.c +++ b/block/backup.c @@ -562,9 +562,13 @@ static void coroutine_fn backup_run(void *opaque) if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) { backup_incremental_init_copy_bitmap(job); } else { + /* top or full mode */ + bool is_top = job->sync_mode == MIRROR_SYNC_MODE_TOP; + BlockDriverState *base = + is_top ? backing_bs(blk_bs(job->common.blk)) : NULL; hbitmap_set(job->copy_bitmap, 0, end); - if (job->sync_mode == MIRROR_SYNC_MODE_TOP) { - backup_skip_loop(job, backing_bs(blk_bs(job->common.blk))); + if (is_top || bdrv_has_zero_init(blk_bs(job->target))) { + backup_skip_loop(job, base); } } ret = backup_loop(job); diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index 1d3fd04..388b7b2 100755 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -30,6 +30,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img') blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img') image_len = 64 * 1024 * 1024 # MB +pause_write = '3M' def setUpModule(): qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len)) @@ -39,6 +40,7 @@ def setUpModule(): qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img) qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img) qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img) + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 40M ' + pause_write, test_img) def tearDownModule(): os.remove(test_img)
In case of full backup we can skip unallocated clusters if the target disk is already zero-initialized. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/backup.c | 8 ++++++-- tests/qemu-iotests/055 | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-)