qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"stefanha@redhat.com" <stefanha@redhat.com>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
	"mreitz@redhat.com" <mreitz@redhat.com>
Subject: Re: [PATCH 6/6] iotests: Test committing to short backing file
Date: Wed, 20 Nov 2019 17:11:06 +0100	[thread overview]
Message-ID: <20191120161106.GE5779@linux.fritz.box> (raw)
In-Reply-To: <b5f3fb13-7467-d87e-07db-45bd014c6464@virtuozzo.com>

Am 20.11.2019 um 16:41 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 20.11.2019 17:03, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   tests/qemu-iotests/274        | 131 +++++++++++++++++++++++++++++
> >   tests/qemu-iotests/274.out    | 150 ++++++++++++++++++++++++++++++++++
> >   tests/qemu-iotests/group      |   1 +
> >   tests/qemu-iotests/iotests.py |   2 +-
> >   4 files changed, 283 insertions(+), 1 deletion(-)
> >   create mode 100755 tests/qemu-iotests/274
> >   create mode 100644 tests/qemu-iotests/274.out
> > 
> > diff --git a/tests/qemu-iotests/274 b/tests/qemu-iotests/274
> > new file mode 100755
> > index 0000000000..f3b71e2859
> > --- /dev/null
> > +++ b/tests/qemu-iotests/274
> > @@ -0,0 +1,131 @@
> > +#!/usr/bin/env python
> > +#
> > +# Copyright (C) 2019 Red Hat, Inc.
> > +#
> > +# 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/>.
> > +#
> > +# Creator/Owner: Kevin Wolf <kwolf@redhat.com>
> > +#
> > +# Some tests for short backing files and short overlays
> > +
> > +import iotests
> > +import os
> > +
> > +iotests.verify_image_format(supported_fmts=['qcow2'])
> > +iotests.verify_platform(['linux'])
> > +
> > +size_short = 1 * 1024 * 1024
> > +size_long = 2 * 1024 * 1024
> > +size_diff = size_long - size_short
> > +
> > +def create_chain():
> > +    iotests.qemu_img_log('create', '-f', iotests.imgfmt, base,
> > +                         str(size_long))
> > +    iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base, mid,
> > +                         str(size_short))
> > +    iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', mid, top,
> > +                         str(size_long))
> > +
> > +    iotests.qemu_io_log('-c', 'write -P 1 0 %d' % size_long, base)
> > +
> > +def create_vm():
> > +    vm = iotests.VM()
> > +    vm.add_blockdev('file,filename=%s,node-name=base-file' % (base))
> > +    vm.add_blockdev('%s,file=base-file,node-name=base' % (iotests.imgfmt))
> > +    vm.add_blockdev('file,filename=%s,node-name=mid-file' % (mid))
> > +    vm.add_blockdev('%s,file=mid-file,node-name=mid,backing=base' % (iotests.imgfmt))
> > +    vm.add_drive(top, 'backing=mid,node-name=top')
> > +    return vm
> > +
> > +with iotests.FilePath('base') as base, \
> > +     iotests.FilePath('mid') as mid, \
> > +     iotests.FilePath('top') as top:
> > +
> > +    iotests.log('== Commit tests ==')
> > +
> > +    create_chain()
> > +
> > +    iotests.log('=== Check visible data ===')
> > +
> > +    iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, top)
> > +    iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), top)
> > +
> > +    iotests.log('=== Checking allocation status ===')
> > +
> > +    iotests.qemu_io_log('-c', 'alloc 0 %d' % size_short,
> > +                        '-c', 'alloc %d %d' % (size_short, size_diff),
> > +                        base)
> > +
> > +    iotests.qemu_io_log('-c', 'alloc 0 %d' % size_short,
> > +                        '-c', 'alloc %d %d' % (size_short, size_diff),
> > +                        mid)
> > +
> > +    iotests.qemu_io_log('-c', 'alloc 0 %d' % size_short,
> > +                        '-c', 'alloc %d %d' % (size_short, size_diff),
> > +                        top)
> > +
> > +    iotests.log('=== Checking map ===')
> > +
> > +    iotests.qemu_img_log('map', '--output=json', base)
> > +    iotests.qemu_img_log('map', '--output=human', base)
> > +    iotests.qemu_img_log('map', '--output=json', mid)
> > +    iotests.qemu_img_log('map', '--output=human', mid)
> > +    iotests.qemu_img_log('map', '--output=json', top)
> > +    iotests.qemu_img_log('map', '--output=human', top)
> > +
> > +    iotests.log('=== Testing qemu-img commit (top -> mid) ===')
> > +
> > +    iotests.qemu_img_log('commit', top)
> > +    iotests.img_info_log(mid)
> > +    iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid)
> > +    iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid)
> > +
> > +    iotests.log('=== Testing HMP commit (top -> mid) ===')
> > +
> > +    create_chain()
> > +    with create_vm() as vm:
> > +        vm.launch()
> > +        vm.qmp_log('human-monitor-command', command_line='commit drive0')
> > +
> > +    iotests.img_info_log(mid)
> > +    iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid)
> > +    iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid)
> > +
> > +    iotests.log('=== Testing QMP active commit (top -> mid) ===')
> > +
> > +    create_chain()
> > +    with create_vm() as vm:
> > +        vm.launch()
> > +        vm.qmp_log('block-commit', device='top', base_node='mid',
> > +                   job_id='job0', auto_dismiss=False)
> > +        vm.run_job('job0', wait=5)
> > +
> > +    iotests.img_info_log(mid)
> > +    iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid)
> > +    iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid)
> > +
> > +
> > +    iotests.log('== Resize tests ==')
> > +
> > +    iotests.qemu_img_log('create', '-f', iotests.imgfmt, base, '6G')
> > +    iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base, top, '1G')
> > +    iotests.qemu_io_log('-c', 'write -P 1 3G 64k', base)
> > +    iotests.qemu_io_log('-c', 'write -P 2 5G 64k', base)
> > +
> > +    # After this, 0 to 6 GB should be allocated/zeroed
> > +    # 6 GB to 8 BG should be unallocated
> 
> Hmm, the problem is that the following qemu-img map can't show it, as it merges
> 1G..6G and 6G..8G into one chunk..

Hm, true, because it's more about the content of the blocks than about
the allocation status. I'll add a qemu-io 'map' call, which display the
actual allocation status:

1 GiB (0x40000000) bytes not allocated at offset 0 bytes (0x0)
5 GiB (0x140000000) bytes     allocated at offset 1 GiB (0x40000000)
2 GiB (0x80000000) bytes not allocated at offset 6 GiB (0x180000000)

> > +    iotests.qemu_img_log('resize', '-f', iotests.imgfmt, top, '8G')
> > +    iotests.qemu_io_log('-c', 'read -P 0 3G 64k', top)
> > +    iotests.qemu_io_log('-c', 'read -P 0 5G 64k', top)
> > +    iotests.qemu_img_log('map', '--output=json', top)
> > diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out
> > new file mode 100644
> > index 0000000000..def0ac7d27
> > --- /dev/null
> > +++ b/tests/qemu-iotests/274.out
> > @@ -0,0 +1,150 @@
> > +== Commit tests ==
> > +Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536 lazy_refcounts=off refcount_bits=16
> > +
> > +Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576 backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off refcount_bits=16
> > +
> > +Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152 backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off refcount_bits=16
> > +
> > +wrote 2097152/2097152 bytes at offset 0
> > +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +
> > +=== Check visible data ===
> > +read 1048576/1048576 bytes at offset 0
> > +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +
> > +read 1048576/1048576 bytes at offset 1048576
> > +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +
> > +=== Checking allocation status ===
> > +1048576/1048576 bytes allocated at offset 0 bytes
> > +1048576/1048576 bytes allocated at offset 1 MiB
> > +
> > +0/1048576 bytes allocated at offset 0 bytes
> > +0/0 bytes allocated at offset 1 MiB
> > +
> > +0/1048576 bytes allocated at offset 0 bytes
> > +0/1048576 bytes allocated at offset 1 MiB
> > +
> > +=== Checking map ===
> > +[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": 327680}]
> > +
> > +Offset          Length          Mapped to       File
> > +0               0x200000        0x50000         TEST_DIR/PID-base
> > +
> > +[{ "start": 0, "length": 1048576, "depth": 1, "zero": false, "data": true, "offset": 327680}]
> > +
> > +Offset          Length          Mapped to       File
> > +0               0x100000        0x50000         TEST_DIR/PID-base
> > +
> > +[{ "start": 0, "length": 1048576, "depth": 2, "zero": false, "data": true, "offset": 327680},
> > +{ "start": 1048576, "length": 1048576, "depth": 0, "zero": true, "data": false}]
> 
> I think depth of second chunk should be 1, not 0.. But this is for
> another fixing series.

The part from 1 GB to 6 GB should be 0 without any question, this is
where we wrote zeros into the overlay.

The part from 7 GB to 8 GB is a bit more open to interpretation because
this is unallocated in the overlay and reads zeros because the backing
file is shorter. I think 0 makes sense, but it's debatable.

Kevin



  reply	other threads:[~2019-11-20 16:15 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-20 14:03 [PATCH for-4.2? 0/6] block: Fix resize (extending) of short overlays Kevin Wolf
2019-11-20 14:03 ` [PATCH 1/6] block: bdrv_co_do_pwrite_zeroes: 64 bit 'bytes' parameter Kevin Wolf
2019-11-20 14:14   ` Eric Blake
2019-11-20 14:27   ` Vladimir Sementsov-Ogievskiy
2019-11-20 16:18   ` Alberto Garcia
2019-11-20 14:03 ` [PATCH 2/6] block: truncate: Don't make backing file data visible Kevin Wolf
2019-11-20 14:20   ` Eric Blake
2019-11-20 14:47   ` Vladimir Sementsov-Ogievskiy
2019-11-20 15:12     ` Kevin Wolf
2019-11-20 18:01   ` Vladimir Sementsov-Ogievskiy
2019-11-25 11:06     ` Kevin Wolf
2019-11-20 14:03 ` [PATCH 3/6] iotests: Add qemu_io_log() Kevin Wolf
2019-11-20 14:26   ` Eric Blake
2019-11-20 14:49   ` Vladimir Sementsov-Ogievskiy
2019-11-20 14:03 ` [PATCH 4/6] iotests: Fix timeout in run_job() Kevin Wolf
2019-11-20 14:29   ` Eric Blake
2019-11-20 14:51   ` Vladimir Sementsov-Ogievskiy
2019-11-20 14:03 ` [PATCH 5/6] iotests: Support job-complete " Kevin Wolf
2019-11-20 14:46   ` Eric Blake
2019-11-20 14:56   ` Vladimir Sementsov-Ogievskiy
2019-11-20 14:03 ` [PATCH 6/6] iotests: Test committing to short backing file Kevin Wolf
2019-11-20 14:52   ` Eric Blake
2019-11-20 15:41   ` Vladimir Sementsov-Ogievskiy
2019-11-20 16:11     ` Kevin Wolf [this message]
2019-11-20 16:29       ` Vladimir Sementsov-Ogievskiy
2019-11-20 16:10   ` Vladimir Sementsov-Ogievskiy
2019-11-20 15:49 ` [PATCH for-4.2? 0/6] block: Fix resize (extending) of short overlays Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191120161106.GE5779@linux.fritz.box \
    --to=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).