Message ID | 20190725155735.11872-3-mreitz@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | vmdk: Misc fixes | expand |
On 7/25/19 11:57 AM, Max Reitz wrote: > This makes iotest 033 pass with e.g. subformat=monolithicFlat. It also > turns a former error in 059 into success. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Seems roughly correct, but I only really gave it a cursory look; my trust in you knowing the exact semantics of filename and path variables because of those lengthy series is doing the heavy lifting here: Reviewed-by: John Snow <jsnow@redhat.com> (And if it breaks, it's for 4.2, and it's just vmdk, we'll figure it out.) > --- > block/vmdk.c | 54 ++++++++++++++++++++++++-------------- > tests/qemu-iotests/059 | 7 +++-- > tests/qemu-iotests/059.out | 4 ++- > 3 files changed, 42 insertions(+), 23 deletions(-) > > diff --git a/block/vmdk.c b/block/vmdk.c > index bd36ece125..db6acfc31e 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1076,8 +1076,7 @@ static const char *next_line(const char *s) > } > > static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > - const char *desc_file_path, QDict *options, > - Error **errp) > + QDict *options, Error **errp) > { > int ret; > int matches; > @@ -1087,6 +1086,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > const char *p, *np; > int64_t sectors = 0; > int64_t flat_offset; > + char *desc_file_dir = NULL; > char *extent_path; > BdrvChild *extent_file; > BDRVVmdkState *s = bs->opaque; > @@ -1130,16 +1130,23 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > continue; > } > > - if (!path_is_absolute(fname) && !path_has_protocol(fname) && > - !desc_file_path[0]) > - { > - bdrv_refresh_filename(bs->file->bs); > - error_setg(errp, "Cannot use relative extent paths with VMDK " > - "descriptor file '%s'", bs->file->bs->filename); > - return -EINVAL; > - } > + if (path_is_absolute(fname) || path_has_protocol(fname)) { > + extent_path = g_strdup(fname); > + } else { > + if (!desc_file_dir) { > + desc_file_dir = bdrv_dirname(bs->file->bs, errp); > + if (!desc_file_dir) { > + bdrv_refresh_filename(bs->file->bs); > + error_prepend(errp, "Cannot use relative paths with VMDK " > + "descriptor file '%s': ", > + bs->file->bs->filename); > + ret = -EINVAL; > + goto out; > + } > + } > > - extent_path = path_combine(desc_file_path, fname); > + extent_path = g_strconcat(desc_file_dir, fname, NULL); > + } > > ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents); > assert(ret < 32); > @@ -1149,7 +1156,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > g_free(extent_path); > if (local_err) { > error_propagate(errp, local_err); > - return -EINVAL; > + ret = -EINVAL; > + goto out; > } > > /* save to extents array */ > @@ -1160,7 +1168,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > 0, 0, 0, 0, 0, &extent, errp); > if (ret < 0) { > bdrv_unref_child(bs, extent_file); > - return ret; > + goto out; > } > extent->flat_start_offset = flat_offset << 9; > } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) { > @@ -1175,24 +1183,27 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > g_free(buf); > if (ret) { > bdrv_unref_child(bs, extent_file); > - return ret; > + goto out; > } > extent = &s->extents[s->num_extents - 1]; > } else if (!strcmp(type, "SESPARSE")) { > ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp); > if (ret) { > bdrv_unref_child(bs, extent_file); > - return ret; > + goto out; > } > extent = &s->extents[s->num_extents - 1]; > } else { > error_setg(errp, "Unsupported extent type '%s'", type); > bdrv_unref_child(bs, extent_file); > - return -ENOTSUP; > + ret = -ENOTSUP; > + goto out; > } > extent->type = g_strdup(type); > } > - return 0; > + > + ret = 0; > + goto out; > > invalid: > np = next_line(p); > @@ -1201,7 +1212,11 @@ invalid: > np--; > } > error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p); > - return -EINVAL; > + ret = -EINVAL; > + > +out: > + g_free(desc_file_dir); > + return ret; > } > > static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, > @@ -1228,8 +1243,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, > } > s->create_type = g_strdup(ct); > s->desc_offset = 0; > - ret = vmdk_parse_extents(buf, bs, bs->file->bs->exact_filename, options, > - errp); > + ret = vmdk_parse_extents(buf, bs, options, errp); > exit: > return ret; > } > diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 > index 279aee6815..fbed5f9483 100755 > --- a/tests/qemu-iotests/059 > +++ b/tests/qemu-iotests/059 > @@ -114,9 +114,12 @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2 > > echo > echo "=== Testing monolithicFlat with internally generated JSON file name ===" > +# Should work, because bdrv_dirname() works fine with blkdebug > IMGOPTS="subformat=monolithicFlat" _make_test_img 64M > -$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" 2>&1 \ > - | _filter_testdir | _filter_imgfmt > +$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \ > + -c info \ > + 2>&1 \ > + | _filter_testdir | _filter_imgfmt | _filter_img_info > _cleanup_test_img > > echo > diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out > index 77d8984428..120cddd207 100644 > --- a/tests/qemu-iotests/059.out > +++ b/tests/qemu-iotests/059.out > @@ -2050,7 +2050,9 @@ wrote 512/512 bytes at offset 10240 > > === Testing monolithicFlat with internally generated JSON file name === > Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 > -qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}' > +format name: IMGFMT > +cluster size: 0 bytes > +vm state offset: 0 bytes > > === Testing version 3 === > image: TEST_DIR/iotest-version3.IMGFMT >
diff --git a/block/vmdk.c b/block/vmdk.c index bd36ece125..db6acfc31e 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1076,8 +1076,7 @@ static const char *next_line(const char *s) } static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, - const char *desc_file_path, QDict *options, - Error **errp) + QDict *options, Error **errp) { int ret; int matches; @@ -1087,6 +1086,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, const char *p, *np; int64_t sectors = 0; int64_t flat_offset; + char *desc_file_dir = NULL; char *extent_path; BdrvChild *extent_file; BDRVVmdkState *s = bs->opaque; @@ -1130,16 +1130,23 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, continue; } - if (!path_is_absolute(fname) && !path_has_protocol(fname) && - !desc_file_path[0]) - { - bdrv_refresh_filename(bs->file->bs); - error_setg(errp, "Cannot use relative extent paths with VMDK " - "descriptor file '%s'", bs->file->bs->filename); - return -EINVAL; - } + if (path_is_absolute(fname) || path_has_protocol(fname)) { + extent_path = g_strdup(fname); + } else { + if (!desc_file_dir) { + desc_file_dir = bdrv_dirname(bs->file->bs, errp); + if (!desc_file_dir) { + bdrv_refresh_filename(bs->file->bs); + error_prepend(errp, "Cannot use relative paths with VMDK " + "descriptor file '%s': ", + bs->file->bs->filename); + ret = -EINVAL; + goto out; + } + } - extent_path = path_combine(desc_file_path, fname); + extent_path = g_strconcat(desc_file_dir, fname, NULL); + } ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents); assert(ret < 32); @@ -1149,7 +1156,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, g_free(extent_path); if (local_err) { error_propagate(errp, local_err); - return -EINVAL; + ret = -EINVAL; + goto out; } /* save to extents array */ @@ -1160,7 +1168,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, 0, 0, 0, 0, 0, &extent, errp); if (ret < 0) { bdrv_unref_child(bs, extent_file); - return ret; + goto out; } extent->flat_start_offset = flat_offset << 9; } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) { @@ -1175,24 +1183,27 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, g_free(buf); if (ret) { bdrv_unref_child(bs, extent_file); - return ret; + goto out; } extent = &s->extents[s->num_extents - 1]; } else if (!strcmp(type, "SESPARSE")) { ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp); if (ret) { bdrv_unref_child(bs, extent_file); - return ret; + goto out; } extent = &s->extents[s->num_extents - 1]; } else { error_setg(errp, "Unsupported extent type '%s'", type); bdrv_unref_child(bs, extent_file); - return -ENOTSUP; + ret = -ENOTSUP; + goto out; } extent->type = g_strdup(type); } - return 0; + + ret = 0; + goto out; invalid: np = next_line(p); @@ -1201,7 +1212,11 @@ invalid: np--; } error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p); - return -EINVAL; + ret = -EINVAL; + +out: + g_free(desc_file_dir); + return ret; } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, @@ -1228,8 +1243,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, } s->create_type = g_strdup(ct); s->desc_offset = 0; - ret = vmdk_parse_extents(buf, bs, bs->file->bs->exact_filename, options, - errp); + ret = vmdk_parse_extents(buf, bs, options, errp); exit: return ret; } diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 279aee6815..fbed5f9483 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -114,9 +114,12 @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2 echo echo "=== Testing monolithicFlat with internally generated JSON file name ===" +# Should work, because bdrv_dirname() works fine with blkdebug IMGOPTS="subformat=monolithicFlat" _make_test_img 64M -$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" 2>&1 \ - | _filter_testdir | _filter_imgfmt +$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \ + -c info \ + 2>&1 \ + | _filter_testdir | _filter_imgfmt | _filter_img_info _cleanup_test_img echo diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 77d8984428..120cddd207 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2050,7 +2050,9 @@ wrote 512/512 bytes at offset 10240 === Testing monolithicFlat with internally generated JSON file name === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 -qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}' +format name: IMGFMT +cluster size: 0 bytes +vm state offset: 0 bytes === Testing version 3 === image: TEST_DIR/iotest-version3.IMGFMT
This makes iotest 033 pass with e.g. subformat=monolithicFlat. It also turns a former error in 059 into success. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/vmdk.c | 54 ++++++++++++++++++++++++-------------- tests/qemu-iotests/059 | 7 +++-- tests/qemu-iotests/059.out | 4 ++- 3 files changed, 42 insertions(+), 23 deletions(-)