qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kashyap Chamarthy <kchamart@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: Eric Blake <eblake@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Kevin Wolf <kwolf@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [Qemu-block] [RFC PATCH 2/2] iotests: add 222 to test basic fleecing
Date: Wed, 27 Jun 2018 14:28:15 +0200	[thread overview]
Message-ID: <20180627122815.GD1933@paraplu> (raw)
In-Reply-To: <d77341f6-cb36-3317-e748-64a205e668c1@redhat.com>

On Wed, Jun 27, 2018 at 01:51:59AM -0400, John Snow wrote:

[...]

FWIW, I would also add a little blurb in the commit message about what
"fleecing" exactly is.  Something along the lines of: 

    "It provides an NBD export that serves you a (read-only?)
    point-in-time (PIT) snapshot of a disk."  

I'm sure you can come up with better phrasing.

> > The test looks valid - you are definitely reading data over NBD from the
> > point in time that you started the blockdev-backup job, even while the
> > source image continues to be modified.
> > 
> >> +    for p in overwrite:
> >> +        cmd = "write -P%s %s %s" % p
> >> +        log(cmd)
> >> +        log(vm.hmp_qemu_io(srcNode, cmd))
> >> +
> >> +    log('')
> >> +    log('--- Verifying Data ---')
> >> +    log('')
> >> +
> >> +    for p in patterns:
> >> +        cmd = "read -P%s %s %s" % p
> >> +        log(cmd)
> >> +        assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri)
> >> == 0
> > 
> > Perhaps additional steps would be to then stop the NBD export, stop the
> > block job, delete the tgtNode fleecing file, then stop qemu, and finally
> > check that the overwritten patterns correctly show up in the source
> > image (that is, also prove that we can tear down a job, and that the
> > overwrites worked).  And we may want to enhance this test (or use it as
> > a starting point to copy into a new test) to play with persistent dirty
> > bitmaps thrown into the mix as well.  But what you have is already a
> > great start to prevent regressions, so:
> > 
> 
> Good suggestions. I'm working toward throwing bitmaps in now, but
> actually cleaning up the VM properly and stopping the NBD server and
> testing some of the latter-half paths would be nice. This was just a bit
> of an RFC to get the bits out there sooner rather than later.

Yeah, they can be added later on.  For this test, you can just get away
by cleaning up the VM and the NBD export: log(vm.qmp("nbd-server-stop").

With that: 

    Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>

[...]

-- 
/kashyap

      reply	other threads:[~2018-06-27 12:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-26 22:22 [Qemu-devel] [RFC PATCH 0/2] iotests: fleecing test John Snow
2018-06-26 22:22 ` [Qemu-devel] [RFC PATCH 1/2] block: allow blockdev-backup from any source John Snow
2018-06-27  1:38   ` Eric Blake
2018-06-27  6:00     ` John Snow
2018-06-26 22:22 ` [Qemu-devel] [RFC PATCH 2/2] iotests: add 222 to test basic fleecing John Snow
2018-06-27  1:49   ` Eric Blake
2018-06-27  5:51     ` John Snow
2018-06-27 12:28       ` Kashyap Chamarthy [this message]

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=20180627122815.GD1933@paraplu \
    --to=kchamart@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).