From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfEtU-00022o-CV for qemu-devel@nongnu.org; Mon, 06 Apr 2015 17:49:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfEtT-0000eu-92 for qemu-devel@nongnu.org; Mon, 06 Apr 2015 17:49:20 -0400 Message-ID: <5522FF56.90705@redhat.com> Date: Mon, 06 Apr 2015 17:49:10 -0400 From: John Snow MIME-Version: 1.0 References: <1426879023-18151-1-git-send-email-jsnow@redhat.com> <1426879023-18151-20-git-send-email-jsnow@redhat.com> <20150402142747.GN25244@stefanha-thinkpad.redhat.com> In-Reply-To: <20150402142747.GN25244@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 19/20] iotests: add simple incremental backup case List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, famz@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com, vsementsov@parallels.com, stefanha@redhat.com, mreitz@redhat.com On 04/02/2015 10:27 AM, Stefan Hajnoczi wrote: > On Fri, Mar 20, 2015 at 03:17:02PM -0400, John Snow wrote: >> Signed-off-by: John Snow >> --- >> tests/qemu-iotests/124 | 153 +++++++++++++++++++++++++++++++++++++++++++++ >> tests/qemu-iotests/124.out | 4 +- >> 2 files changed, 155 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124 >> index 85675ec..ce2cda7 100644 >> --- a/tests/qemu-iotests/124 >> +++ b/tests/qemu-iotests/124 [snip, snip, snip] >> >> class TestIncrementalBackup(iotests.QMPTestCase): >> def setUp(self): >> @@ -73,6 +109,123 @@ class TestIncrementalBackup(iotests.QMPTestCase): >> iotests.qemu_img('create', '-f', fmt, img, size) >> self.files.append(img) >> >> + >> + def create_full_backup(self, drive=None): >> + if drive is None: >> + drive = self.drives[-1] >> + >> + res = self.vm.qmp('drive-backup', device=drive['id'], >> + sync='full', format=drive['fmt'], >> + target=drive['backup']) >> + self.assert_qmp(res, 'return', {}) >> + self.wait_until_completed(drive['id']) >> + self.check_full_backup(drive) >> + self.files.append(drive['backup']) >> + return drive['backup'] >> + >> + >> + def check_full_backup(self, drive=None): >> + if drive is None: >> + drive = self.drives[-1] >> + self.assertTrue(iotests.compare_images(drive['file'], drive['backup'])) > > I think QEMU still has at least drive['file'] open? It's not safe to > access the file from another program while it is open. > > The simplest solution is to terminate the VM before calling > iotests.compare_images(). > Oh, that's unfortunate. I have been checking images after every creation as a nice incremental sanity check. That will be hard to do if I have to actually close QEMU. My reasoning was: (1) We're explicitly flushing after every write, (2) We're in a qtest mode so there is no guest activity, (3) Nobody is writing to this image by the time we call compare_images(). So it should be safe to /read/ the files while QEMU is occupied doing nothing. I could delay all tests until completion, but I'd lose out on the ability to test the equivalence of any incrementals that are not their most recent versions, unless I also start creating a lot of full backups, but I think this starts to get messy and makes the tests hard to follow. Is it really that unsafe? I could add in an explicit pause/resume barrier around the check if that would help inspire some confidence in the test. --js