* [Qemu-devel] [PATCH 0/2] vmdk: convert error reporting
@ 2013-10-10 7:20 Fam Zheng
2013-10-10 7:20 ` [Qemu-devel] [PATCH 1/2] " Fam Zheng
2013-10-10 7:20 ` [Qemu-devel] [PATCH 2/2] vmdk: refuse enabling zeroed grain with flat images Fam Zheng
0 siblings, 2 replies; 5+ messages in thread
From: Fam Zheng @ 2013-10-10 7:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
The first patch converts "fprintf(stderr,..." to error_setg with errp.
The second patch checks the compatibility of zeroed_grain flag and flat type
and reports error if both are true.
Fam Zheng (2):
vmdk: convert error reporting
vmdk: refuse enabling zeroed grain with flat images
block/vmdk.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 1/2] vmdk: convert error reporting
2013-10-10 7:20 [Qemu-devel] [PATCH 0/2] vmdk: convert error reporting Fam Zheng
@ 2013-10-10 7:20 ` Fam Zheng
2013-10-10 10:39 ` Kevin Wolf
2013-10-10 7:20 ` [Qemu-devel] [PATCH 2/2] vmdk: refuse enabling zeroed grain with flat images Fam Zheng
1 sibling, 1 reply; 5+ messages in thread
From: Fam Zheng @ 2013-10-10 7:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
Convert "fprintf(stderr,..." to error API by passing around errp to
functions those want to report error message.
There are 2 more "fprintf(stderr,..." remaining in read/write code path.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/vmdk.c | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 5d56e31..a98ad23 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -483,7 +483,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
BlockDriverState *file,
- int flags)
+ int flags, Error **errp)
{
int ret;
uint32_t magic;
@@ -514,11 +514,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
}
static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
- uint64_t desc_offset);
+ uint64_t desc_offset, Error **errp);
static int vmdk_open_vmdk4(BlockDriverState *bs,
BlockDriverState *file,
- int flags)
+ int flags, Error **errp)
{
int ret;
uint32_t magic;
@@ -534,7 +534,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (header.capacity == 0) {
uint64_t desc_offset = le64_to_cpu(header.desc_offset);
if (desc_offset) {
- return vmdk_open_desc_file(bs, flags, desc_offset << 9);
+ return vmdk_open_desc_file(bs, flags, desc_offset << 9, errp);
}
}
@@ -663,7 +663,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name,
/* Open an extent file and append to bs array */
static int vmdk_open_sparse(BlockDriverState *bs,
BlockDriverState *file,
- int flags)
+ int flags, Error **errp)
{
uint32_t magic;
@@ -674,10 +674,10 @@ static int vmdk_open_sparse(BlockDriverState *bs,
magic = be32_to_cpu(magic);
switch (magic) {
case VMDK3_MAGIC:
- return vmdk_open_vmfs_sparse(bs, file, flags);
+ return vmdk_open_vmfs_sparse(bs, file, flags, errp);
break;
case VMDK4_MAGIC:
- return vmdk_open_vmdk4(bs, file, flags);
+ return vmdk_open_vmdk4(bs, file, flags, errp);
break;
default:
return -EMEDIUMTYPE;
@@ -686,7 +686,7 @@ static int vmdk_open_sparse(BlockDriverState *bs,
}
static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
- const char *desc_file_path)
+ const char *desc_file_path, Error **errp)
{
int ret;
char access[11];
@@ -748,13 +748,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
extent->flat_start_offset = flat_offset << 9;
} else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
/* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/
- ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
+ ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, errp);
if (ret) {
bdrv_unref(extent_file);
return ret;
}
} else {
- fprintf(stderr,
+ error_setg(errp,
"VMDK: Not supported extent type \"%s\""".\n", type);
return -ENOTSUP;
}
@@ -769,7 +769,7 @@ next_line:
}
static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
- uint64_t desc_offset)
+ uint64_t desc_offset, Error **errp)
{
int ret;
char *buf = NULL;
@@ -798,13 +798,13 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
strcmp(ct, "vmfsSparse") &&
strcmp(ct, "twoGbMaxExtentSparse") &&
strcmp(ct, "twoGbMaxExtentFlat")) {
- fprintf(stderr,
+ error_setg(errp,
"VMDK: Not supported image type \"%s\""".\n", ct);
ret = -ENOTSUP;
goto exit;
}
s->desc_offset = 0;
- ret = vmdk_parse_extents(buf, bs, bs->file->filename);
+ ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp);
exit:
g_free(buf);
return ret;
@@ -816,10 +816,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
BDRVVmdkState *s = bs->opaque;
- if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
+ if (vmdk_open_sparse(bs, bs->file, flags, errp) == 0) {
s->desc_offset = 0x200;
} else {
- ret = vmdk_open_desc_file(bs, flags, 0);
+ ret = vmdk_open_desc_file(bs, flags, 0, errp);
if (ret) {
goto fail;
}
@@ -1517,12 +1517,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
}
static int filename_decompose(const char *filename, char *path, char *prefix,
- char *postfix, size_t buf_len)
+ char *postfix, size_t buf_len, Error **errp)
{
const char *p, *q;
if (filename == NULL || !strlen(filename)) {
- fprintf(stderr, "Vmdk: no filename provided.\n");
+ error_setg(errp, "Vmdk: no filename provided");
return VMDK_ERROR;
}
p = strrchr(filename, '/');
@@ -1597,7 +1597,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
"ddb.adapterType = \"%s\"\n";
Error *local_err = NULL;
- if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
+ if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) {
return -EINVAL;
}
/* Read out options */
@@ -1623,7 +1623,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
strcmp(adapter_type, "buslogic") &&
strcmp(adapter_type, "lsilogic") &&
strcmp(adapter_type, "legacyESX")) {
- fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type);
+ error_setg(errp, "VMDK: Unknown adapter type: '%s'", adapter_type);
return -EINVAL;
}
if (strcmp(adapter_type, "ide") != 0) {
@@ -1639,7 +1639,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
strcmp(fmt, "twoGbMaxExtentSparse") &&
strcmp(fmt, "twoGbMaxExtentFlat") &&
strcmp(fmt, "streamOptimized")) {
- fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt);
+ error_setg(errp, "VMDK: Unknown subformat: %s", fmt);
return -EINVAL;
}
split = !(strcmp(fmt, "twoGbMaxExtentFlat") &&
@@ -1653,7 +1653,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
desc_extent_line = "RW %lld SPARSE \"%s\"\n";
}
if (flat && backing_file) {
- /* not supporting backing file for flat image */
+ error_setg(errp, "flat image can't have backing file");
return -ENOTSUP;
}
if (backing_file) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] vmdk: refuse enabling zeroed grain with flat images
2013-10-10 7:20 [Qemu-devel] [PATCH 0/2] vmdk: convert error reporting Fam Zheng
2013-10-10 7:20 ` [Qemu-devel] [PATCH 1/2] " Fam Zheng
@ 2013-10-10 7:20 ` Fam Zheng
1 sibling, 0 replies; 5+ messages in thread
From: Fam Zheng @ 2013-10-10 7:20 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha
This is a header flag and we needs sparse for the header.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/vmdk.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index a98ad23..78c0129 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1656,6 +1656,10 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
error_setg(errp, "flat image can't have backing file");
return -ENOTSUP;
}
+ if (flat && zeroed_grain) {
+ error_setg(errp, "flat image can't enable zeroed grain");
+ return -ENOTSUP;
+ }
if (backing_file) {
BlockDriverState *bs = bdrv_new("");
ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vmdk: convert error reporting
2013-10-10 7:20 ` [Qemu-devel] [PATCH 1/2] " Fam Zheng
@ 2013-10-10 10:39 ` Kevin Wolf
2013-10-10 10:59 ` Fam Zheng
0 siblings, 1 reply; 5+ messages in thread
From: Kevin Wolf @ 2013-10-10 10:39 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, stefanha
Am 10.10.2013 um 09:20 hat Fam Zheng geschrieben:
> Convert "fprintf(stderr,..." to error API by passing around errp to
> functions those want to report error message.
>
> There are 2 more "fprintf(stderr,..." remaining in read/write code path.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/vmdk.c | 42 +++++++++++++++++++++---------------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 5d56e31..a98ad23 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -483,7 +483,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
>
> static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> BlockDriverState *file,
> - int flags)
> + int flags, Error **errp)
> {
> int ret;
> uint32_t magic;
The errp parameter is unused in this function. You should probably use
error_setg_errno() for the failure cases.
Callers generally don't distinguish different error return codes, so
converted functions could return void instead of int.
> @@ -514,11 +514,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> }
>
> static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> - uint64_t desc_offset);
> + uint64_t desc_offset, Error **errp);
>
> static int vmdk_open_vmdk4(BlockDriverState *bs,
> BlockDriverState *file,
> - int flags)
> + int flags, Error **errp)
> {
> int ret;
> uint32_t magic;
> @@ -534,7 +534,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
> if (header.capacity == 0) {
> uint64_t desc_offset = le64_to_cpu(header.desc_offset);
> if (desc_offset) {
> - return vmdk_open_desc_file(bs, flags, desc_offset << 9);
> + return vmdk_open_desc_file(bs, flags, desc_offset << 9, errp);
> }
> }
>
> @@ -663,7 +663,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name,
> /* Open an extent file and append to bs array */
> static int vmdk_open_sparse(BlockDriverState *bs,
> BlockDriverState *file,
> - int flags)
> + int flags, Error **errp)
> {
> uint32_t magic;
>
> @@ -674,10 +674,10 @@ static int vmdk_open_sparse(BlockDriverState *bs,
> magic = be32_to_cpu(magic);
> switch (magic) {
> case VMDK3_MAGIC:
> - return vmdk_open_vmfs_sparse(bs, file, flags);
> + return vmdk_open_vmfs_sparse(bs, file, flags, errp);
> break;
> case VMDK4_MAGIC:
> - return vmdk_open_vmdk4(bs, file, flags);
> + return vmdk_open_vmdk4(bs, file, flags, errp);
> break;
> default:
> return -EMEDIUMTYPE;
> @@ -686,7 +686,7 @@ static int vmdk_open_sparse(BlockDriverState *bs,
> }
>
> static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
> - const char *desc_file_path)
> + const char *desc_file_path, Error **errp)
> {
> int ret;
> char access[11];
> @@ -748,13 +748,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
> extent->flat_start_offset = flat_offset << 9;
> } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
> /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/
> - ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
> + ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, errp);
> if (ret) {
> bdrv_unref(extent_file);
> return ret;
> }
> } else {
> - fprintf(stderr,
> + error_setg(errp,
> "VMDK: Not supported extent type \"%s\""".\n", type);
> return -ENOTSUP;
> }
> @@ -769,7 +769,7 @@ next_line:
> }
>
> static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> - uint64_t desc_offset)
> + uint64_t desc_offset, Error **errp)
> {
> int ret;
> char *buf = NULL;
> @@ -798,13 +798,13 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> strcmp(ct, "vmfsSparse") &&
> strcmp(ct, "twoGbMaxExtentSparse") &&
> strcmp(ct, "twoGbMaxExtentFlat")) {
> - fprintf(stderr,
> + error_setg(errp,
> "VMDK: Not supported image type \"%s\""".\n", ct);
> ret = -ENOTSUP;
> goto exit;
> }
> s->desc_offset = 0;
> - ret = vmdk_parse_extents(buf, bs, bs->file->filename);
> + ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp);
> exit:
> g_free(buf);
> return ret;
> @@ -816,10 +816,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
> int ret;
> BDRVVmdkState *s = bs->opaque;
>
> - if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
> + if (vmdk_open_sparse(bs, bs->file, flags, errp) == 0) {
> s->desc_offset = 0x200;
> } else {
> - ret = vmdk_open_desc_file(bs, flags, 0);
> + ret = vmdk_open_desc_file(bs, flags, 0, errp);
> if (ret) {
> goto fail;
> }
> @@ -1517,12 +1517,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
> }
>
> static int filename_decompose(const char *filename, char *path, char *prefix,
> - char *postfix, size_t buf_len)
> + char *postfix, size_t buf_len, Error **errp)
> {
> const char *p, *q;
>
> if (filename == NULL || !strlen(filename)) {
> - fprintf(stderr, "Vmdk: no filename provided.\n");
> + error_setg(errp, "Vmdk: no filename provided");
Here we have a "Vmdk:" prefix and the error message starts in lower
case.
> return VMDK_ERROR;
> }
> p = strrchr(filename, '/');
> @@ -1597,7 +1597,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> "ddb.adapterType = \"%s\"\n";
> Error *local_err = NULL;
>
> - if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
> + if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) {
> return -EINVAL;
> }
> /* Read out options */
> @@ -1623,7 +1623,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> strcmp(adapter_type, "buslogic") &&
> strcmp(adapter_type, "lsilogic") &&
> strcmp(adapter_type, "legacyESX")) {
> - fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type);
> + error_setg(errp, "VMDK: Unknown adapter type: '%s'", adapter_type);
Here it's "VMDK:" and upper case.
> return -EINVAL;
> }
> if (strcmp(adapter_type, "ide") != 0) {
> @@ -1639,7 +1639,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> strcmp(fmt, "twoGbMaxExtentSparse") &&
> strcmp(fmt, "twoGbMaxExtentFlat") &&
> strcmp(fmt, "streamOptimized")) {
> - fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt);
> + error_setg(errp, "VMDK: Unknown subformat: %s", fmt);
> return -EINVAL;
> }
> split = !(strcmp(fmt, "twoGbMaxExtentFlat") &&
> @@ -1653,7 +1653,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> desc_extent_line = "RW %lld SPARSE \"%s\"\n";
> }
> if (flat && backing_file) {
> - /* not supporting backing file for flat image */
> + error_setg(errp, "flat image can't have backing file");
No prefix at all and lower case.
Can we standardise on using no prefix and starting the error message
with a capital letter?
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] vmdk: convert error reporting
2013-10-10 10:39 ` Kevin Wolf
@ 2013-10-10 10:59 ` Fam Zheng
0 siblings, 0 replies; 5+ messages in thread
From: Fam Zheng @ 2013-10-10 10:59 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, stefanha
On Thu, 10/10 12:39, Kevin Wolf wrote:
> Am 10.10.2013 um 09:20 hat Fam Zheng geschrieben:
> > Convert "fprintf(stderr,..." to error API by passing around errp to
> > functions those want to report error message.
> >
> > There are 2 more "fprintf(stderr,..." remaining in read/write code path.
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > block/vmdk.c | 42 +++++++++++++++++++++---------------------
> > 1 file changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 5d56e31..a98ad23 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -483,7 +483,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
> >
> > static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> > BlockDriverState *file,
> > - int flags)
> > + int flags, Error **errp)
> > {
> > int ret;
> > uint32_t magic;
>
> The errp parameter is unused in this function. You should probably use
> error_setg_errno() for the failure cases.
>
> Callers generally don't distinguish different error return codes, so
> converted functions could return void instead of int.
>
> > @@ -514,11 +514,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> > }
> >
> > static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> > - uint64_t desc_offset);
> > + uint64_t desc_offset, Error **errp);
> >
> > static int vmdk_open_vmdk4(BlockDriverState *bs,
> > BlockDriverState *file,
> > - int flags)
> > + int flags, Error **errp)
> > {
> > int ret;
> > uint32_t magic;
> > @@ -534,7 +534,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
> > if (header.capacity == 0) {
> > uint64_t desc_offset = le64_to_cpu(header.desc_offset);
> > if (desc_offset) {
> > - return vmdk_open_desc_file(bs, flags, desc_offset << 9);
> > + return vmdk_open_desc_file(bs, flags, desc_offset << 9, errp);
> > }
> > }
> >
> > @@ -663,7 +663,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name,
> > /* Open an extent file and append to bs array */
> > static int vmdk_open_sparse(BlockDriverState *bs,
> > BlockDriverState *file,
> > - int flags)
> > + int flags, Error **errp)
> > {
> > uint32_t magic;
> >
> > @@ -674,10 +674,10 @@ static int vmdk_open_sparse(BlockDriverState *bs,
> > magic = be32_to_cpu(magic);
> > switch (magic) {
> > case VMDK3_MAGIC:
> > - return vmdk_open_vmfs_sparse(bs, file, flags);
> > + return vmdk_open_vmfs_sparse(bs, file, flags, errp);
> > break;
> > case VMDK4_MAGIC:
> > - return vmdk_open_vmdk4(bs, file, flags);
> > + return vmdk_open_vmdk4(bs, file, flags, errp);
> > break;
> > default:
> > return -EMEDIUMTYPE;
> > @@ -686,7 +686,7 @@ static int vmdk_open_sparse(BlockDriverState *bs,
> > }
> >
> > static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
> > - const char *desc_file_path)
> > + const char *desc_file_path, Error **errp)
> > {
> > int ret;
> > char access[11];
> > @@ -748,13 +748,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
> > extent->flat_start_offset = flat_offset << 9;
> > } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
> > /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/
> > - ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
> > + ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, errp);
> > if (ret) {
> > bdrv_unref(extent_file);
> > return ret;
> > }
> > } else {
> > - fprintf(stderr,
> > + error_setg(errp,
> > "VMDK: Not supported extent type \"%s\""".\n", type);
> > return -ENOTSUP;
> > }
> > @@ -769,7 +769,7 @@ next_line:
> > }
> >
> > static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> > - uint64_t desc_offset)
> > + uint64_t desc_offset, Error **errp)
> > {
> > int ret;
> > char *buf = NULL;
> > @@ -798,13 +798,13 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> > strcmp(ct, "vmfsSparse") &&
> > strcmp(ct, "twoGbMaxExtentSparse") &&
> > strcmp(ct, "twoGbMaxExtentFlat")) {
> > - fprintf(stderr,
> > + error_setg(errp,
> > "VMDK: Not supported image type \"%s\""".\n", ct);
> > ret = -ENOTSUP;
> > goto exit;
> > }
> > s->desc_offset = 0;
> > - ret = vmdk_parse_extents(buf, bs, bs->file->filename);
> > + ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp);
> > exit:
> > g_free(buf);
> > return ret;
> > @@ -816,10 +816,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
> > int ret;
> > BDRVVmdkState *s = bs->opaque;
> >
> > - if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
> > + if (vmdk_open_sparse(bs, bs->file, flags, errp) == 0) {
> > s->desc_offset = 0x200;
> > } else {
> > - ret = vmdk_open_desc_file(bs, flags, 0);
> > + ret = vmdk_open_desc_file(bs, flags, 0, errp);
> > if (ret) {
> > goto fail;
> > }
> > @@ -1517,12 +1517,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
> > }
> >
> > static int filename_decompose(const char *filename, char *path, char *prefix,
> > - char *postfix, size_t buf_len)
> > + char *postfix, size_t buf_len, Error **errp)
> > {
> > const char *p, *q;
> >
> > if (filename == NULL || !strlen(filename)) {
> > - fprintf(stderr, "Vmdk: no filename provided.\n");
> > + error_setg(errp, "Vmdk: no filename provided");
>
> Here we have a "Vmdk:" prefix and the error message starts in lower
> case.
>
> > return VMDK_ERROR;
> > }
> > p = strrchr(filename, '/');
> > @@ -1597,7 +1597,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> > "ddb.adapterType = \"%s\"\n";
> > Error *local_err = NULL;
> >
> > - if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
> > + if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) {
> > return -EINVAL;
> > }
> > /* Read out options */
> > @@ -1623,7 +1623,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> > strcmp(adapter_type, "buslogic") &&
> > strcmp(adapter_type, "lsilogic") &&
> > strcmp(adapter_type, "legacyESX")) {
> > - fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type);
> > + error_setg(errp, "VMDK: Unknown adapter type: '%s'", adapter_type);
>
> Here it's "VMDK:" and upper case.
>
> > return -EINVAL;
> > }
> > if (strcmp(adapter_type, "ide") != 0) {
> > @@ -1639,7 +1639,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> > strcmp(fmt, "twoGbMaxExtentSparse") &&
> > strcmp(fmt, "twoGbMaxExtentFlat") &&
> > strcmp(fmt, "streamOptimized")) {
> > - fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt);
> > + error_setg(errp, "VMDK: Unknown subformat: %s", fmt);
> > return -EINVAL;
> > }
> > split = !(strcmp(fmt, "twoGbMaxExtentFlat") &&
> > @@ -1653,7 +1653,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> > desc_extent_line = "RW %lld SPARSE \"%s\"\n";
> > }
> > if (flat && backing_file) {
> > - /* not supporting backing file for flat image */
> > + error_setg(errp, "flat image can't have backing file");
>
> No prefix at all and lower case.
>
> Can we standardise on using no prefix and starting the error message
> with a capital letter?
>
OK, good points. I'll fix them. Thanks.
Fam
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-10 11:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-10 7:20 [Qemu-devel] [PATCH 0/2] vmdk: convert error reporting Fam Zheng
2013-10-10 7:20 ` [Qemu-devel] [PATCH 1/2] " Fam Zheng
2013-10-10 10:39 ` Kevin Wolf
2013-10-10 10:59 ` Fam Zheng
2013-10-10 7:20 ` [Qemu-devel] [PATCH 2/2] vmdk: refuse enabling zeroed grain with flat images Fam Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).