| Submitter | Luiz Capitulino |
|---|---|
| Date | Oct. 17, 2012, 7:35 p.m. |
| Message ID | <1350502556-4885-5-git-send-email-lcapitulino@redhat.com> |
| Download | mbox | patch |
| Permalink | /patch/192127/ |
| State | New |
| Headers | show |
Comments
Am 17.10.2012 21:35, schrieb Luiz Capitulino: > This commit adds an Error ** argument to bdrv_img_create() and set it > appropriately on error. > > Callers of bdrv_img_create() pass NULL for the new argument and still > rely on bdrv_img_create()'s return value. Next commits will change > callers to use the Error object instead. > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > block.c | 23 +++++++++++++++++++++-- > block.h | 2 +- > blockdev.c | 2 +- > qemu-img.c | 2 +- > 4 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/block.c b/block.c > index bdb53af..83bcab5 100644 > --- a/block.c > +++ b/block.c > @@ -4295,7 +4295,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) > int bdrv_img_create(const char *filename, const char *fmt, > const char *base_filename, const char *base_fmt, > char *options, uint64_t img_size, int flags, > - QEMUOptionParameter **param_list) > + QEMUOptionParameter **param_list, Error **errp) > { > QEMUOptionParameter *param = NULL, *create_options = NULL; > QEMUOptionParameter *backing_fmt, *backing_file, *size; > @@ -4308,6 +4308,7 @@ int bdrv_img_create(const char *filename, const char *fmt, > drv = bdrv_find_format(fmt); > if (!drv) { > error_report("Unknown file format '%s'", fmt); > + error_setg(errp, "Unknown file format '%s'", fmt); > ret = -EINVAL; > goto out; > } > @@ -4315,6 +4316,7 @@ int bdrv_img_create(const char *filename, const char *fmt, > proto_drv = bdrv_find_protocol(filename); > if (!proto_drv) { > error_report("Unknown protocol '%s'", filename); > + error_setg(errp, "Unknown protocol '%s'", filename); > ret = -EINVAL; > goto out; > } > @@ -4334,6 +4336,7 @@ int bdrv_img_create(const char *filename, const char *fmt, > param = parse_option_parameters(options, create_options, param); > if (param == NULL) { > error_report("Invalid options for file format '%s'.", fmt); > + error_setg(errp, "Invalid options for file format '%s'.", fmt); > ret = -EINVAL; > goto out; > } > @@ -4344,6 +4347,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > base_filename)) { > error_report("Backing file not supported for file format '%s'", > fmt); > + error_setg(errp, "Backing file not supported for file format '%s'", > + fmt); > ret = -EINVAL; > goto out; > } > @@ -4353,6 +4358,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) { > error_report("Backing file format not supported for file " > "format '%s'", fmt); > + error_setg(errp, "Backing file format not supported for file " > + "format '%s'", fmt); > ret = -EINVAL; > goto out; > } > @@ -4363,6 +4370,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > if (!strcmp(filename, backing_file->value.s)) { > error_report("Error: Trying to create an image with the " > "same filename as the backing file"); > + error_setg(errp, "Error: Trying to create an image with the " > + "same filename as the backing file"); > ret = -EINVAL; > goto out; > } > @@ -4374,6 +4383,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > if (!backing_drv) { > error_report("Unknown backing file format '%s'", > backing_fmt->value.s); > + error_setg(errp, "Unknown backing file format '%s'", > + backing_fmt->value.s); > ret = -EINVAL; > goto out; > } > @@ -4396,7 +4407,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > > ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); > if (ret < 0) { > - error_report("Could not open '%s'", backing_file->value.s); This is the only error_report() that is dropped in this patch. Not sure what is the best patch to drop them, but consistency wouldn't hurt. If we want to keep the series bisectable, i.e. qemu-img output stays the same for each patch, you'd have to drop them in 5/8, I think. Kevin
On Thu, 18 Oct 2012 14:08:05 +0200 Kevin Wolf <kwolf@redhat.com> wrote: > Am 17.10.2012 21:35, schrieb Luiz Capitulino: > > This commit adds an Error ** argument to bdrv_img_create() and set it > > appropriately on error. > > > > Callers of bdrv_img_create() pass NULL for the new argument and still > > rely on bdrv_img_create()'s return value. Next commits will change > > callers to use the Error object instead. > > > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > --- > > block.c | 23 +++++++++++++++++++++-- > > block.h | 2 +- > > blockdev.c | 2 +- > > qemu-img.c | 2 +- > > 4 files changed, 24 insertions(+), 5 deletions(-) > > > > diff --git a/block.c b/block.c > > index bdb53af..83bcab5 100644 > > --- a/block.c > > +++ b/block.c > > @@ -4295,7 +4295,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) > > int bdrv_img_create(const char *filename, const char *fmt, > > const char *base_filename, const char *base_fmt, > > char *options, uint64_t img_size, int flags, > > - QEMUOptionParameter **param_list) > > + QEMUOptionParameter **param_list, Error **errp) > > { > > QEMUOptionParameter *param = NULL, *create_options = NULL; > > QEMUOptionParameter *backing_fmt, *backing_file, *size; > > @@ -4308,6 +4308,7 @@ int bdrv_img_create(const char *filename, const char *fmt, > > drv = bdrv_find_format(fmt); > > if (!drv) { > > error_report("Unknown file format '%s'", fmt); > > + error_setg(errp, "Unknown file format '%s'", fmt); > > ret = -EINVAL; > > goto out; > > } > > @@ -4315,6 +4316,7 @@ int bdrv_img_create(const char *filename, const char *fmt, > > proto_drv = bdrv_find_protocol(filename); > > if (!proto_drv) { > > error_report("Unknown protocol '%s'", filename); > > + error_setg(errp, "Unknown protocol '%s'", filename); > > ret = -EINVAL; > > goto out; > > } > > @@ -4334,6 +4336,7 @@ int bdrv_img_create(const char *filename, const char *fmt, > > param = parse_option_parameters(options, create_options, param); > > if (param == NULL) { > > error_report("Invalid options for file format '%s'.", fmt); > > + error_setg(errp, "Invalid options for file format '%s'.", fmt); > > ret = -EINVAL; > > goto out; > > } > > @@ -4344,6 +4347,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > > base_filename)) { > > error_report("Backing file not supported for file format '%s'", > > fmt); > > + error_setg(errp, "Backing file not supported for file format '%s'", > > + fmt); > > ret = -EINVAL; > > goto out; > > } > > @@ -4353,6 +4358,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > > if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) { > > error_report("Backing file format not supported for file " > > "format '%s'", fmt); > > + error_setg(errp, "Backing file format not supported for file " > > + "format '%s'", fmt); > > ret = -EINVAL; > > goto out; > > } > > @@ -4363,6 +4370,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > > if (!strcmp(filename, backing_file->value.s)) { > > error_report("Error: Trying to create an image with the " > > "same filename as the backing file"); > > + error_setg(errp, "Error: Trying to create an image with the " > > + "same filename as the backing file"); > > ret = -EINVAL; > > goto out; > > } > > @@ -4374,6 +4383,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > > if (!backing_drv) { > > error_report("Unknown backing file format '%s'", > > backing_fmt->value.s); > > + error_setg(errp, "Unknown backing file format '%s'", > > + backing_fmt->value.s); > > ret = -EINVAL; > > goto out; > > } > > @@ -4396,7 +4407,8 @@ int bdrv_img_create(const char *filename, const char *fmt, > > > > ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); > > if (ret < 0) { > > - error_report("Could not open '%s'", backing_file->value.s); > > This is the only error_report() that is dropped in this patch. Not sure > what is the best patch to drop them, but consistency wouldn't hurt. That's correct. I dropped it by accident because I was working on this series and the bdrv_open() conversion at the same time and probably mixed the two. Will fix it for v2. > > If we want to keep the series bisectable, i.e. qemu-img output stays the > same for each patch, you'd have to drop them in 5/8, I think. > > Kevin >
Patch
diff --git a/block.c b/block.c index bdb53af..83bcab5 100644 --- a/block.c +++ b/block.c @@ -4295,7 +4295,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie) int bdrv_img_create(const char *filename, const char *fmt, const char *base_filename, const char *base_fmt, char *options, uint64_t img_size, int flags, - QEMUOptionParameter **param_list) + QEMUOptionParameter **param_list, Error **errp) { QEMUOptionParameter *param = NULL, *create_options = NULL; QEMUOptionParameter *backing_fmt, *backing_file, *size; @@ -4308,6 +4308,7 @@ int bdrv_img_create(const char *filename, const char *fmt, drv = bdrv_find_format(fmt); if (!drv) { error_report("Unknown file format '%s'", fmt); + error_setg(errp, "Unknown file format '%s'", fmt); ret = -EINVAL; goto out; } @@ -4315,6 +4316,7 @@ int bdrv_img_create(const char *filename, const char *fmt, proto_drv = bdrv_find_protocol(filename); if (!proto_drv) { error_report("Unknown protocol '%s'", filename); + error_setg(errp, "Unknown protocol '%s'", filename); ret = -EINVAL; goto out; } @@ -4334,6 +4336,7 @@ int bdrv_img_create(const char *filename, const char *fmt, param = parse_option_parameters(options, create_options, param); if (param == NULL) { error_report("Invalid options for file format '%s'.", fmt); + error_setg(errp, "Invalid options for file format '%s'.", fmt); ret = -EINVAL; goto out; } @@ -4344,6 +4347,8 @@ int bdrv_img_create(const char *filename, const char *fmt, base_filename)) { error_report("Backing file not supported for file format '%s'", fmt); + error_setg(errp, "Backing file not supported for file format '%s'", + fmt); ret = -EINVAL; goto out; } @@ -4353,6 +4358,8 @@ int bdrv_img_create(const char *filename, const char *fmt, if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) { error_report("Backing file format not supported for file " "format '%s'", fmt); + error_setg(errp, "Backing file format not supported for file " + "format '%s'", fmt); ret = -EINVAL; goto out; } @@ -4363,6 +4370,8 @@ int bdrv_img_create(const char *filename, const char *fmt, if (!strcmp(filename, backing_file->value.s)) { error_report("Error: Trying to create an image with the " "same filename as the backing file"); + error_setg(errp, "Error: Trying to create an image with the " + "same filename as the backing file"); ret = -EINVAL; goto out; } @@ -4374,6 +4383,8 @@ int bdrv_img_create(const char *filename, const char *fmt, if (!backing_drv) { error_report("Unknown backing file format '%s'", backing_fmt->value.s); + error_setg(errp, "Unknown backing file format '%s'", + backing_fmt->value.s); ret = -EINVAL; goto out; } @@ -4396,7 +4407,8 @@ int bdrv_img_create(const char *filename, const char *fmt, ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv); if (ret < 0) { - error_report("Could not open '%s'", backing_file->value.s); + error_setg_errno(errp, -ret, "Could not open '%s'", + backing_file->value.s); goto out; } bdrv_get_geometry(bs, &size); @@ -4406,6 +4418,7 @@ int bdrv_img_create(const char *filename, const char *fmt, set_option_parameter(param, BLOCK_OPT_SIZE, buf); } else { error_report("Image creation needs a size parameter"); + error_setg(errp, "Image creation needs a size parameter"); ret = -EINVAL; goto out; } @@ -4417,12 +4430,18 @@ int bdrv_img_create(const char *filename, const char *fmt, if (ret == -ENOTSUP) { error_report("Formatting or formatting option not supported for " "file format '%s'", fmt); + error_setg(errp,"Formatting or formatting option not supported for " + "file format '%s'", fmt); } else if (ret == -EFBIG) { error_report("The image size is too large for file format '%s'", fmt); + error_setg(errp, "The image size is too large for file format '%s'", + fmt); } else { error_report("%s: error while creating %s: %s", filename, fmt, strerror(-ret)); + error_setg(errp, "%s: error while creating %s: %s", filename, fmt, + strerror(-ret)); } } diff --git a/block.h b/block.h index 55d13e6..49d89a1 100644 --- a/block.h +++ b/block.h @@ -342,7 +342,7 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf, int bdrv_img_create(const char *filename, const char *fmt, const char *base_filename, const char *base_fmt, char *options, uint64_t img_size, int flags, - QEMUOptionParameter **param_list); + QEMUOptionParameter **param_list, Error **errp); void bdrv_set_buffer_alignment(BlockDriverState *bs, int align); void *qemu_blockalign(BlockDriverState *bs, size_t size); diff --git a/blockdev.c b/blockdev.c index 9dddefd..01be90f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -783,7 +783,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) ret = bdrv_img_create(new_image_file, format, states->old_bs->filename, states->old_bs->drv->format_name, - NULL, -1, flags, NULL); + NULL, -1, flags, NULL, NULL); if (ret) { error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file); goto delete_and_fail; diff --git a/qemu-img.c b/qemu-img.c index e482443..12fb6c2 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -363,7 +363,7 @@ static int img_create(int argc, char **argv) } ret = bdrv_img_create(filename, fmt, base_filename, base_fmt, - options, img_size, BDRV_O_FLAGS, ¶ms); + options, img_size, BDRV_O_FLAGS, ¶ms, NULL); if (ret < 0) { goto out; }
This commit adds an Error ** argument to bdrv_img_create() and set it appropriately on error. Callers of bdrv_img_create() pass NULL for the new argument and still rely on bdrv_img_create()'s return value. Next commits will change callers to use the Error object instead. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- block.c | 23 +++++++++++++++++++++-- block.h | 2 +- blockdev.c | 2 +- qemu-img.c | 2 +- 4 files changed, 24 insertions(+), 5 deletions(-)