| Message ID | 1381389656-5026-2-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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) {
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(-)