From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52647) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ydg6D-0002rG-AB for qemu-devel@nongnu.org; Thu, 02 Apr 2015 10:28:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ydg6C-00009Y-7Q for qemu-devel@nongnu.org; Thu, 02 Apr 2015 10:28:01 -0400 Date: Thu, 2 Apr 2015 15:27:47 +0100 From: Stefan Hajnoczi Message-ID: <20150402142747.GN25244@stefanha-thinkpad.redhat.com> References: <1426879023-18151-1-git-send-email-jsnow@redhat.com> <1426879023-18151-20-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="N/GrjenRD+RJfyz+" Content-Disposition: inline In-Reply-To: <1426879023-18151-20-git-send-email-jsnow@redhat.com> 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: John Snow 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 --N/GrjenRD+RJfyz+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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(-) >=20 > 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 > @@ -28,6 +28,42 @@ def io_write_patterns(img, patterns): > for pattern in patterns: > iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img) > =20 > +class Bitmap: > + def __init__(self, name, drive): > + self.name =3D name > + self.drive =3D drive > + self.pattern =3D os.path.join(iotests.test_dir.replace('%', '%%'= ), > + '%s.%s.backup.%%i.img' % (drive['id'= ], > + name)) drive['id'] and name aren't escaped. It is simpler and safer to format the string from scratch in new_target() and drop the Bitmap.pattern field: def new_target(self, num=3DNone): ... filename =3D '%s.%s.backup.%i.img' % (self.drive['id'], self.name, num) target =3D os.path.join(iotests.test_dir, filename) > + self.num =3D 0 > + self.backups =3D list() > + > + def base_target(self): > + return self.drive['backup'] > + > + def new_target(self, num=3DNone): > + if num is None: > + num =3D self.num > + self.num =3D num + 1 > + target =3D self.pattern % num > + self.backups.append(target) > + return target > + > + def last_target(self): > + if self.backups: > + return self.backups[-1] > + return self.base_target() > + > + def del_target(self): > + os.remove(self.backups.pop()) > + self.num -=3D 1 > + > + def __del__(self): > + for backup in self.backups: > + try: > + os.remove(backup) > + except OSError: > + pass =46rom the language reference: "It is not guaranteed that __del__() methods are called for objects that still exist when the interpreter exits." https://docs.python.org/2.7/reference/datamodel.html#object.__del__ Relying on reference counts is risky. The TestCase.tearDown() method is the place to clean up: https://docs.python.org/2.7/library/unittest.html#unittest.TestCase.tearDown It could iterate over self.bitmaps[] and call a cleanup() function. > =20 > 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) > =20 > + > + def create_full_backup(self, drive=3DNone): > + if drive is None: > + drive =3D self.drives[-1] > + > + res =3D self.vm.qmp('drive-backup', device=3Ddrive['id'], > + sync=3D'full', format=3Ddrive['fmt'], > + target=3Ddrive['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=3DNone): > + if drive is None: > + drive =3D self.drives[-1] > + self.assertTrue(iotests.compare_images(drive['file'], drive['bac= kup'])) 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(). --N/GrjenRD+RJfyz+ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVHVHjAAoJEJykq7OBq3PIEfUIAI6Rc8XFNkJIkvZaMTwrituM BWV5uFrYQwdJMnflZvXAvBZuUG8RUTythhVljq2xaPxSboSVbjcJliY6s/g+3epZ /urZ7I7gnPiT7+K1GQWwW8oC2TUdRymBFXpaK0GJITydFJYmecQh3XsCOJkYuVZc 4gK87B4U+s2NIFkYvESKUruUuckTy7qlBji4FGaCUcIeo0jrLaZd+d2EhCAuNKGw BQwt8Z1omw7Vmt7e4YdRzpIsvD3k/iZtkstQiVyVV83FFt9D/1uoLe7SPJEN8OvT wNYc26csMz5BGVfhXVmUckkIFQp/ORIrbut6qT1m3x23rwMX/kooi49jtucV788= =HVmZ -----END PGP SIGNATURE----- --N/GrjenRD+RJfyz+--