All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Sam Eiderman <shmuel.eiderman@oracle.com>,
	Eric Blake <eblake@redhat.com>
Cc: fam@euphon.net, kwolf@redhat.com, qemu-block@nongnu.org,
	arbel.moshe@oracle.com, QEMU <qemu-devel@nongnu.org>,
	mreitz@redhat.com, liran.alon@oracle.com,
	yoav.elnekave@oracle.com
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] vmdk: Set vmdk parent backing_format to vmdk
Date: Thu, 2 May 2019 11:31:27 +0200	[thread overview]
Message-ID: <af928e13-bde2-a9ae-de74-853d9bfc5e65@redhat.com> (raw)
In-Reply-To: <AC82FB5B-34EF-453F-B4CB-33F1BBF58623@oracle.com>

On 27/03/2019 10.28, Sam Eiderman wrote:
> Thanks,
> 
> Also please notice that the mentioned commit in the commit message has it’s commit id changed to b69864e.
> 
> Sam
> 
>> On 27 Mar 2019, at 2:35, Eric Blake <eblake@redhat.com> wrote:
>>
>> On 3/26/19 2:58 PM, Sam Eiderman wrote:
>>> Commit fb2105b ("vmdk: Support version=3 in VMDK descriptor files") fixed
>>> the probe function to correctly guess vmdk descriptors with version=3.
>>
>> All patches need to cc qemu-devel in addition to their maintainers (I've
>> added that now).
>>
>>>
>>> This solves the issue where vmdk snapshot with parent vmdk descriptor
>>> containing "version=3" would be treated as raw instead vmdk.
>>>
>>> In the future case where a new vmdk version is introduced, we will again
>>> experience this issue, even if the user will provide "-f vmdk" it will
>>> only apply to the tip image and not to the underlying "misprobed" parent
>>> image.
>>>
>>> The code in vmdk.c already assumes that the backing file of vmdk must be
>>> vmdk (see vmdk_is_cid_valid which returns 0 if backing file is not
>>> vmdk).
>>>
>>> So let's make it official by supplying the backing_format as vmdk.
>>>
>>> Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
>>> Reviewed-By: Liran Alon <liran.alon@oracle.com>
>>> Reviewed-by: Arbel Moshe <arbel.moshe@oracle.com>
>>> Signed-off-by: Shmuel Eiderman <shmuel.eiderman@oracle.com>
>>> ---
>>> block/vmdk.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/block/vmdk.c b/block/vmdk.c
>>> index 8dec6ef767..de8cb859f8 100644
>>> --- a/block/vmdk.c
>>> +++ b/block/vmdk.c
>>> @@ -397,6 +397,8 @@ static int vmdk_parent_open(BlockDriverState *bs)
>>>         pstrcpy(bs->auto_backing_file, end_name - p_name + 1, p_name);
>>>         pstrcpy(bs->backing_file, sizeof(bs->backing_file),
>>>                 bs->auto_backing_file);
>>> +        pstrcpy(bs->backing_format, sizeof(bs->backing_format),
>>> +                "vmdk");
>>>     }

 Hi,

this patch broke the vmdk qemu-iotests:

$ cd tests/qemu-iotests/ ; ./check -vmdk 110 126 ; cd ../..
[...]
110 0s ... - output mismatch (see 110.out.bad)
--- /home/thuth/devel/qemu/tests/qemu-iotests/110.out	2019-05-02
11:27:41.000000000 +0200
+++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/110.out.bad	2019-05-02
11:27:54.000000000 +0200
@@ -8,6 +8,7 @@
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
 backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
+backing file format: IMGFMT

 === Non-reconstructable filename ===

@@ -15,6 +16,7 @@
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
 backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
+backing file format: IMGFMT

 === Backing name is always relative to the backed image ===

@@ -26,4 +28,5 @@
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
 backing file: t.IMGFMT.base (cannot determine actual path)
+backing file format: IMGFMT
 *** done
126 0s ... - output mismatch (see 126.out.bad)
--- /home/thuth/devel/qemu/tests/qemu-iotests/126.out	2019-05-02
11:27:41.000000000 +0200
+++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/126.out.bad	2019-05-02
11:27:55.000000000 +0200
@@ -13,6 +13,7 @@
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
 backing file: ./image:base.IMGFMT (actual path:
TEST_DIR/./image:base.IMGFMT)
+backing file format: IMGFMT

 Formatting 'base.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'file:image:top.IMGFMT', fmt=IMGFMT size=67108864
backing_file=base.IMGFMT
@@ -20,4 +21,5 @@
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
 backing file: base.IMGFMT (actual path: ./base.IMGFMT)
+backing file format: IMGFMT
 *** done
Failures: 110 126
Failed 2 of 2 tests

Could you please send a patch to fix this?

 Thanks,
  Thomas

       reply	other threads:[~2019-05-02  9:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190326195837.8416-1-shmuel.eiderman@oracle.com>
     [not found] ` <891af075-0e1d-de0b-b874-738b0c8f34ae@redhat.com>
     [not found]   ` <AC82FB5B-34EF-453F-B4CB-33F1BBF58623@oracle.com>
2019-05-02  9:31     ` Thomas Huth [this message]
2019-05-02 13:08       ` [Qemu-devel] [PATCH v2] vmdk: Set vmdk parent backing_format to vmdk Sam Eiderman
2019-05-02 13:08         ` Sam Eiderman
2019-05-03 11:34           ` Thomas Huth
2019-05-03 14:32             ` Max Reitz
2019-05-03 14:32               ` Max Reitz
2019-05-04 12:43               ` Sam
2019-05-04 12:43                 ` Sam
2019-05-07 13:26                 ` Max Reitz

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=af928e13-bde2-a9ae-de74-853d9bfc5e65@redhat.com \
    --to=thuth@redhat.com \
    --cc=arbel.moshe@oracle.com \
    --cc=eblake@redhat.com \
    --cc=fam@euphon.net \
    --cc=kwolf@redhat.com \
    --cc=liran.alon@oracle.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=shmuel.eiderman@oracle.com \
    --cc=yoav.elnekave@oracle.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.