| Message ID | 1453688783-11825-1-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 25.01.2016 um 03:26 hat Fam Zheng geschrieben: > Commit d62d9dc4b8 lifted streamOptimized images's version to 3, but we > now refuse to open version 3 images read-write. We need to make > streamOptimized an exception to allow converting to it. This fixes the > accidentally broken iotests case 059 for the same reason. > > Signed-off-by: Fam Zheng <famz@redhat.com> How different are version 3 images for other subformats? Are we arbitrarily restrictring their use or is it really that they don't work with our driver? And if they don't work with our driver, are we sure that streamOptimized images can't use the features we don't support? Or is the version defined per subformat and doesn't necessarily exist for other types? Kevin
On Mon, 01/25 12:16, Kevin Wolf wrote: > Am 25.01.2016 um 03:26 hat Fam Zheng geschrieben: > > Commit d62d9dc4b8 lifted streamOptimized images's version to 3, but we > > now refuse to open version 3 images read-write. We need to make > > streamOptimized an exception to allow converting to it. This fixes the > > accidentally broken iotests case 059 for the same reason. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > How different are version 3 images for other subformats? Are we > arbitrarily restrictring their use or is it really that they don't work > with our driver? And if they don't work with our driver, are we sure > that streamOptimized images can't use the features we don't support? > > Or is the version defined per subformat and doesn't necessarily exist > for other types? Version 3 images are undocumented except in the VMware KB article mentioned in the comment around this line (http://kb.vmware.com/kb/2064959). A few years ago, when users complained that QEMU doesn't support version 3 images, we presumed from the article that reading is okay, as the new feature is "persistent changed block tracking" (although it didn't say it is the only feature enabled by version 3), and went ahead enabling it. This time, it seems newer VMware products only accept version 3 if the subformat is streamOptimized. Again, without any documentation/specification update. Then our users complains again, so we add another exception to mitigate. As this subformat doesn't allow overwrite, the only use case is qemu-img converting to it. So this is pretty safe - it's always operating a new image - and the approach is tested by multiple users (both upstream and downstream). Fam
Am 26.01.2016 um 04:16 hat Fam Zheng geschrieben: > On Mon, 01/25 12:16, Kevin Wolf wrote: > > Am 25.01.2016 um 03:26 hat Fam Zheng geschrieben: > > > Commit d62d9dc4b8 lifted streamOptimized images's version to 3, but we > > > now refuse to open version 3 images read-write. We need to make > > > streamOptimized an exception to allow converting to it. This fixes the > > > accidentally broken iotests case 059 for the same reason. > > > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > > > How different are version 3 images for other subformats? Are we > > arbitrarily restrictring their use or is it really that they don't work > > with our driver? And if they don't work with our driver, are we sure > > that streamOptimized images can't use the features we don't support? > > > > Or is the version defined per subformat and doesn't necessarily exist > > for other types? > > Version 3 images are undocumented except in the VMware KB article mentioned in > the comment around this line (http://kb.vmware.com/kb/2064959). A few years > ago, when users complained that QEMU doesn't support version 3 images, we > presumed from the article that reading is okay, as the new feature is > "persistent changed block tracking" (although it didn't say it is the only > feature enabled by version 3), and went ahead enabling it. > > This time, it seems newer VMware products only accept version 3 if the > subformat is streamOptimized. Again, without any documentation/specification > update. Then our users complains again, so we add another exception to > mitigate. As this subformat doesn't allow overwrite, the only use case is > qemu-img converting to it. So this is pretty safe - it's always operating a > new image - and the approach is tested by multiple users (both upstream and > downstream). I see. Then I guess we can't do much else. Thanks, applied to the block branch. Kevin
diff --git a/block/vmdk.c b/block/vmdk.c index 698679d..4a5850b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -571,6 +571,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, VmdkExtent *extent; BDRVVmdkState *s = bs->opaque; int64_t l1_backup_offset = 0; + bool compressed; ret = bdrv_pread(file->bs, sizeof(magic), &header, sizeof(header)); if (ret < 0) { @@ -645,6 +646,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, header = footer.header; } + compressed = + le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE; if (le32_to_cpu(header.version) > 3) { char buf[64]; snprintf(buf, sizeof(buf), "VMDK version %" PRId32, @@ -652,7 +655,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, error_setg(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bdrv_get_device_or_node_name(bs), "vmdk", buf); return -ENOTSUP; - } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) { + } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR) && + !compressed) { /* VMware KB 2064959 explains that version 3 added support for * persistent changed block tracking (CBT), and backup software can * read it as version=1 if it doesn't care about the changed area
Commit d62d9dc4b8 lifted streamOptimized images's version to 3, but we now refuse to open version 3 images read-write. We need to make streamOptimized an exception to allow converting to it. This fixes the accidentally broken iotests case 059 for the same reason. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/vmdk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)