On 06.04.2015 23:49, John Snow wrote:


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 <js...@redhat.com>
---
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.

I'm fine with it (and consciously accepted it while reviewing), because the worst I can see is the test breaking. You're modifying the image and you're checking whether the modifications are appearing in the image. I cannot really see how qemu could not have made the modifications or made them differently than expected, and they still appear in the image file, just as expected.

Second, we are already relying on internal things pretty much in the iotests (just take qemu-io, it's nothing users should ever use). So because relying on things users shouldn't do is already a part of the iotests and because if it doesn't work, the test will most likely just break, I'm fine with it.


In anticipation of an "Yes, *you* can't see how the behavior can break, making the test pass when it's actually failed, but it's not impossible, you're relying on undefined behavior after all! Doing this even in just a test might consume your HDD in a dragon fire!",

Max

Reply via email to