qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Cc: Markus Armbruster <armbru@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>,
	vsementsov@virtuozzo.com, Juan Quintela <quintela@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	libvir-list@redhat.com,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>
Subject: Re: [Qemu-devel] [PATCH v3 10/10] iotests: add busy/recording bit test to 124
Date: Sat, 23 Feb 2019 16:06:51 -0600	[thread overview]
Message-ID: <39df9fc7-6458-759b-e452-3bfdbf39c94d@redhat.com> (raw)
In-Reply-To: <20190223000614.13894-11-jsnow@redhat.com>

On 2/22/19 6:06 PM, John Snow wrote:
> This adds a simple test that ensures the busy bit works for push backups,
> as well as doubling as bonus test for incremental backups that get interrupted
> by EIO errors.

This makes 124 longer to run, but it is not in the 'quick' group, so
that's okay.

The easy part: I compiled the series, and validated that ./check still
passes with this applied, so:

Tested-by: Eric Blake <eblake@redhat.com>

Now for the fun part (I know from IRC that you had some interesting
challenges coming up with scenarios to even provoke the states you
wanted shown in the output):

> 
> Recording bit tests are already handled sufficiently by 236.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/124     | 110 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/124.out |   4 +-
>  2 files changed, 112 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> index 5aa1bf1bd6..30f12a2202 100755
> --- a/tests/qemu-iotests/124
> +++ b/tests/qemu-iotests/124
> @@ -634,6 +634,116 @@ class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
>          self.vm.shutdown()
>          self.check_backups()
>  
> +    def test_incremental_pause(self):
> +        """
> +        Test an incremental backup that errors into a pause and is resumed.
> +        """
> +
> +        drive0 = self.drives[0]
> +        result = self.vm.qmp('blockdev-add',
> +                             node_name=drive0['id'],
> +                             driver=drive0['fmt'],
> +                             file={
> +                                 'driver': 'blkdebug',
> +                                 'image': {
> +                                     'driver': 'file',
> +                                     'filename': drive0['file']
> +                                 },
> +                                 'set-state': [{
> +                                     'event': 'flush_to_disk',
> +                                     'state': 1,
> +                                     'new_state': 2
> +                                 },{
> +                                     'event': 'read_aio',
> +                                     'state': 2,
> +                                     'new_state': 3
> +                                 }],
> +                                 'inject-error': [{
> +                                     'event': 'read_aio',
> +                                     'errno': 5,
> +                                     'state': 3,
> +                                     'immediately': False,
> +                                     'once': True
> +                                 }],

Yeah, it's hairy to come up with sequences that will pause at the right
place, and this may be sensitive enough that we have to revisit it in
the future, but for now it works and I don't have any better suggestions.

> +                             })
> +        self.assert_qmp(result, 'return', {})
> +        self.create_anchor_backup(drive0)
> +        bitmap = self.add_bitmap('bitmap0', drive0)
> +
> +        # Emulate guest activity
> +        self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
> +                                          ('0xfe', '16M', '256k'),
> +                                          ('0x64', '32736k', '64k')))
> +
> +        # For the purposes of query-block visibility of bitmaps, add a drive
> +        # frontend after we've written data; otherwise we can't use hmp-io
> +        result = self.vm.qmp("device_add",
> +                             id="device0",
> +                             drive=drive0['id'],
> +                             driver="virtio-blk")
> +        self.assert_qmp(result, 'return', {})
> +
> +        # Bitmap Status Check
> +        query = self.vm.qmp('query-block')
> +        ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
> +               if bmap.get('name') == bitmap.name][0]
> +        self.assert_qmp(ret, 'count', 458752)
> +        self.assert_qmp(ret, 'granularity', 65536)
> +        self.assert_qmp(ret, 'status', 'active')
> +        self.assert_qmp(ret, 'busy', False)
> +        self.assert_qmp(ret, 'recording', True)

So far, nothing too different from we've seen elsewhere, but then we get
to the fun part:

> +
> +        # Start backup
> +        parent, _ = bitmap.last_target()
> +        target = self.prepare_backup(bitmap, parent)
> +        res = self.vm.qmp('drive-backup',
> +                          job_id=bitmap.drive['id'],
> +                          device=bitmap.drive['id'],
> +                          sync='incremental',
> +                          bitmap=bitmap.name,
> +                          format=bitmap.drive['fmt'],
> +                          target=target,
> +                          mode='existing',
> +                          on_source_error='stop')
> +        self.assert_qmp(res, 'return', {})
> +
> +        # Wait for the error
> +        event = self.vm.event_wait(name="BLOCK_JOB_ERROR",
> +                                   match={"data":{"device":bitmap.drive['id']}})
> +        self.assert_qmp(event, 'data', {'device': bitmap.drive['id'],
> +                                        'action': 'stop',
> +                                        'operation': 'read'})
> +
> +        # Bitmap Status Check
> +        query = self.vm.qmp('query-block')
> +        ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
> +               if bmap.get('name') == bitmap.name][0]
> +        self.assert_qmp(ret, 'count', 458752)
> +        self.assert_qmp(ret, 'granularity', 65536)
> +        self.assert_qmp(ret, 'status', 'frozen')
> +        self.assert_qmp(ret, 'busy', True)
> +        self.assert_qmp(ret, 'recording', True)

"status":"frozen", "busy":true - yay, you provoked the other QMP states
that we have not seen elsewhere in the testsuite.

> +
> +        # Resume and check incremental backup for consistency
> +        res = self.vm.qmp('block-job-resume', device=bitmap.drive['id'])
> +        self.assert_qmp(res, 'return', {})
> +        self.wait_qmp_backup(bitmap.drive['id'])
> +
> +        # Bitmap Status Check
> +        query = self.vm.qmp('query-block')
> +        ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
> +               if bmap.get('name') == bitmap.name][0]
> +        self.assert_qmp(ret, 'count', 0)
> +        self.assert_qmp(ret, 'granularity', 65536)
> +        self.assert_qmp(ret, 'status', 'active')
> +        self.assert_qmp(ret, 'busy', False)
> +        self.assert_qmp(ret, 'recording', True)

and even nicer, after the transient error goes away, it does not get in
the way of further bitmap edits.

> +
> +        # Finalize / Cleanup
> +        self.make_reference_backup(bitmap)
> +        self.vm.shutdown()
> +        self.check_backups()
> +
>  
>  if __name__ == '__main__':
>      iotests.main(supported_fmts=['qcow2'])
> diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
> index e56cae021b..281b69efea 100644
> --- a/tests/qemu-iotests/124.out
> +++ b/tests/qemu-iotests/124.out
> @@ -1,5 +1,5 @@
> -...........
> +............
>  ----------------------------------------------------------------------
> -Ran 11 tests
> +Ran 12 tests

Pre-existing, but this output is a bear to debug if the test ever starts
failing.  Not something you have to worry about today, though.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

  reply	other threads:[~2019-02-23 22:07 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-23  0:06 [Qemu-devel] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field John Snow
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 01/10] block/dirty-bitmap: add recording and busy properties John Snow
2019-02-23 20:06   ` Eric Blake
2019-02-25  6:23   ` Vladimir Sementsov-Ogievskiy
2019-02-25 15:01   ` Vladimir Sementsov-Ogievskiy
2019-02-25 15:08     ` Eric Blake
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 02/10] block/dirty-bitmaps: rename frozen predicate helper John Snow
2019-02-23 21:10   ` Eric Blake
2019-02-25  7:01   ` Vladimir Sementsov-Ogievskiy
2019-02-25 20:09     ` John Snow
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 03/10] block/dirty-bitmap: remove set/reset assertions against enabled bit John Snow
2019-02-23 21:11   ` Eric Blake
2019-02-25  7:09   ` Vladimir Sementsov-Ogievskiy
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 04/10] block/dirty-bitmap: change semantics of enabled predicate John Snow
2019-02-23 21:14   ` Eric Blake
2019-02-25  7:39   ` Vladimir Sementsov-Ogievskiy
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 05/10] nbd: change error checking order for bitmaps John Snow
2019-02-23 21:29   ` Eric Blake
2019-02-25  7:44   ` Vladimir Sementsov-Ogievskiy
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 06/10] block/dirty-bitmap: explicitly lock bitmaps with successors John Snow
2019-02-25  7:48   ` Vladimir Sementsov-Ogievskiy
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 07/10] block/dirty-bitmaps: unify qmp_locked and user_locked calls John Snow
2019-02-23 21:32   ` Eric Blake
2019-02-25 12:03   ` Vladimir Sementsov-Ogievskiy
2019-02-25 20:37     ` John Snow
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 08/10] block/dirty-bitmaps: move comment block John Snow
2019-02-23 21:32   ` Eric Blake
2019-02-25 12:11   ` Vladimir Sementsov-Ogievskiy
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 09/10] blockdev: remove unused paio parameter documentation John Snow
2019-02-23 21:33   ` Eric Blake
2019-02-25 12:20   ` Vladimir Sementsov-Ogievskiy
2019-02-23  0:06 ` [Qemu-devel] [PATCH v3 10/10] iotests: add busy/recording bit test to 124 John Snow
2019-02-23 22:06   ` Eric Blake [this message]
2019-02-25 20:29     ` John Snow
2019-02-25 22:08 ` [Qemu-devel] [PATCH v3 00/10] dirty-bitmaps: deprecate @status field John Snow
2019-02-27 17:45 ` no-reply

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=39df9fc7-6458-759b-e452-3bfdbf39c94d@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=fam@euphon.net \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --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).