* [Qemu-devel] [PATCH v2] qemu-img: change opening method for the output in dd
@ 2016-08-11 13:04 Reda Sallahi
2016-08-15 5:44 ` Fam Zheng
0 siblings, 1 reply; 2+ messages in thread
From: Reda Sallahi @ 2016-08-11 13:04 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Kevin Wolf, Max Reitz, Fam Zheng, Stefan Hajnoczi,
Reda Sallahi
dd was creating an output image regardless of whether there was one already
created. With this patch we try to open first the output image and resize it
if necessary.
We also make it mandatory to specify conv=notrunc when the file already
exists.
Signed-off-by: Reda Sallahi <fullmanet@gmail.com>
---
Depends on:
[PATCH v3] qemu-img: add conv=notrunc option to dd
Changes from v1:
* add --image-opts handling
qemu-img.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 98 insertions(+), 21 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index d08905b..3973990 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3919,7 +3919,8 @@ static int img_dd(int argc, char **argv)
char *tmp;
BlockDriver *drv = NULL, *proto_drv = NULL;
BlockBackend *blk1 = NULL, *blk2 = NULL;
- QemuOpts *opts = NULL;
+ QDict *qoptions = NULL;
+ QemuOpts *opts = NULL, *qopts = NULL;
QemuOptsList *create_opts = NULL;
Error *local_err = NULL;
bool image_opts = false;
@@ -4071,31 +4072,106 @@ static int img_dd(int argc, char **argv)
dd.count * in.bsz < size) {
size = dd.count * in.bsz;
}
-
- /* Overflow means the specified offset is beyond input image's size */
- if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
- size < in.bsz * in.offset)) {
- qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
+ if (image_opts) {
+ qopts = qemu_opts_parse_noisily(qemu_find_opts("source"),
+ out.filename, true);
+ if (!opts) {
+ ret = -1;
+ goto out;
+ }
+ qoptions = qemu_opts_to_qdict(qopts, NULL);
} else {
- qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
- size - in.bsz * in.offset, &error_abort);
+ qoptions = qdict_new();
+ qdict_put(qoptions, "driver", qstring_from_str(out_fmt));
}
- ret = bdrv_create(drv, out.filename, opts, &local_err);
- if (ret < 0) {
- error_reportf_err(local_err,
- "%s: error while creating output image: ",
- out.filename);
- ret = -1;
- goto out;
- }
-
- blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
- false, false);
+ blk2 = blk_new_open((image_opts ? NULL : out.filename),
+ NULL, qoptions, BDRV_O_RDWR, NULL);
if (!blk2) {
- ret = -1;
- goto out;
+ drv = bdrv_find_format(out_fmt);
+ if (!drv) {
+ error_report("Unknown file format");
+ ret = -1;
+ goto out;
+ }
+ proto_drv = bdrv_find_protocol(out.filename, true, &local_err);
+
+ if (!proto_drv) {
+ error_report_err(local_err);
+ ret = -1;
+ goto out;
+ }
+ if (!drv->create_opts) {
+ error_report("Format driver '%s' does not support image creation",
+ drv->format_name);
+ ret = -1;
+ goto out;
+ }
+ if (!proto_drv->create_opts) {
+ error_report("Protocol driver '%s' does not support image creation",
+ proto_drv->format_name);
+ ret = -1;
+ goto out;
+ }
+ create_opts = qemu_opts_append(create_opts, drv->create_opts);
+ create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
+
+ opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+
+ /* Overflow means the specified offset is beyond input image's size */
+ if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
+ size < in.offset * in.bsz)) {
+ qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+ 0, &error_abort);
+ } else {
+ qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+ size - in.offset * in.bsz, &error_abort);
+ }
+
+ ret = bdrv_create(drv, out.filename, opts, &local_err);
+ if (ret < 0) {
+ error_reportf_err(local_err,
+ "%s: error while creating output image: ",
+ out.filename);
+ ret = -1;
+ goto out;
+ }
+ blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
+ false, false);
+ if (!blk2) {
+ ret = -1;
+ goto out;
+ }
+ } else {
+ int64_t blk2sz = 0;
+
+ if (!(dd.conv & C_NOTRUNC)) {
+ /* We make conv=notrunc mandatory for the moment to avoid
+ accidental destruction of the output image. Needs to be
+ changed when a better solution is found */
+ error_report("conv=notrunc not specified");
+ ret = -1;
+ goto out;
+ }
+
+ blk2sz = blk_getlength(blk2);
+ if (blk2sz < 0) {
+ error_report("Failed to get size for '%s'", in.filename);
+ ret = -1;
+ goto out;
+ }
+
+ if (!(dd.conv & C_NOTRUNC)) {
+ blk_truncate(blk2, 0);
+ }
+ if (!(dd.flags & C_SKIP) || (in.offset <= INT64_MAX / in.bsz &&
+ size >= in.offset * in.bsz)) {
+ if (!(dd.conv & C_NOTRUNC) ||
+ blk2sz < size - in.offset * in.bsz) {
+ blk_truncate(blk2, size - in.offset * in.bsz);
+ }
+ }
}
if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
@@ -4141,6 +4217,7 @@ static int img_dd(int argc, char **argv)
out:
g_free(arg);
qemu_opts_del(opts);
+ qemu_opts_del(qopts);
qemu_opts_free(create_opts);
blk_unref(blk1);
blk_unref(blk2);
--
2.9.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qemu-img: change opening method for the output in dd
2016-08-11 13:04 [Qemu-devel] [PATCH v2] qemu-img: change opening method for the output in dd Reda Sallahi
@ 2016-08-15 5:44 ` Fam Zheng
0 siblings, 0 replies; 2+ messages in thread
From: Fam Zheng @ 2016-08-15 5:44 UTC (permalink / raw)
To: Reda Sallahi
Cc: qemu-devel, Kevin Wolf, qemu-block, Max Reitz, Stefan Hajnoczi
On Thu, 08/11 15:04, Reda Sallahi wrote:
> dd was creating an output image regardless of whether there was one already
> created. With this patch we try to open first the output image and resize it
> if necessary.
>
> We also make it mandatory to specify conv=notrunc when the file already
> exists.
>
> Signed-off-by: Reda Sallahi <fullmanet@gmail.com>
> ---
> Depends on:
> [PATCH v3] qemu-img: add conv=notrunc option to dd
>
> Changes from v1:
> * add --image-opts handling
>
> qemu-img.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 98 insertions(+), 21 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index d08905b..3973990 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3919,7 +3919,8 @@ static int img_dd(int argc, char **argv)
> char *tmp;
> BlockDriver *drv = NULL, *proto_drv = NULL;
> BlockBackend *blk1 = NULL, *blk2 = NULL;
> - QemuOpts *opts = NULL;
> + QDict *qoptions = NULL;
> + QemuOpts *opts = NULL, *qopts = NULL;
> QemuOptsList *create_opts = NULL;
> Error *local_err = NULL;
> bool image_opts = false;
> @@ -4071,31 +4072,106 @@ static int img_dd(int argc, char **argv)
> dd.count * in.bsz < size) {
> size = dd.count * in.bsz;
> }
> -
> - /* Overflow means the specified offset is beyond input image's size */
> - if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
> - size < in.bsz * in.offset)) {
> - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
> + if (image_opts) {
> + qopts = qemu_opts_parse_noisily(qemu_find_opts("source"),
> + out.filename, true);
> + if (!opts) {
> + ret = -1;
> + goto out;
> + }
> + qoptions = qemu_opts_to_qdict(qopts, NULL);
> } else {
> - qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> - size - in.bsz * in.offset, &error_abort);
> + qoptions = qdict_new();
> + qdict_put(qoptions, "driver", qstring_from_str(out_fmt));
> }
>
> - ret = bdrv_create(drv, out.filename, opts, &local_err);
> - if (ret < 0) {
> - error_reportf_err(local_err,
> - "%s: error while creating output image: ",
> - out.filename);
> - ret = -1;
> - goto out;
> - }
> -
> - blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
> - false, false);
> + blk2 = blk_new_open((image_opts ? NULL : out.filename),
Superfluous parenthesis around the conditional expression. And I think if you
put the filename into qoptions in the else branch above, the first parameter
can be NULL unconditionally.
> + NULL, qoptions, BDRV_O_RDWR, NULL);
>
> if (!blk2) {
> - ret = -1;
> - goto out;
> + drv = bdrv_find_format(out_fmt);
> + if (!drv) {
> + error_report("Unknown file format");
> + ret = -1;
> + goto out;
> + }
> + proto_drv = bdrv_find_protocol(out.filename, true, &local_err);
Does this work with image_opts = true? Or do you need to extract it from
qoptions in that case?
> +
> + if (!proto_drv) {
> + error_report_err(local_err);
> + ret = -1;
> + goto out;
> + }
> + if (!drv->create_opts) {
> + error_report("Format driver '%s' does not support image creation",
> + drv->format_name);
> + ret = -1;
> + goto out;
> + }
> + if (!proto_drv->create_opts) {
> + error_report("Protocol driver '%s' does not support image creation",
> + proto_drv->format_name);
> + ret = -1;
> + goto out;
> + }
> + create_opts = qemu_opts_append(create_opts, drv->create_opts);
> + create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
> +
> + opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> +
> + /* Overflow means the specified offset is beyond input image's size */
> + if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
> + size < in.offset * in.bsz)) {
> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> + 0, &error_abort);
> + } else {
> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
> + size - in.offset * in.bsz, &error_abort);
> + }
> +
> + ret = bdrv_create(drv, out.filename, opts, &local_err);
> + if (ret < 0) {
> + error_reportf_err(local_err,
> + "%s: error while creating output image: ",
> + out.filename);
> + ret = -1;
> + goto out;
> + }
> + blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
> + false, false);
> + if (!blk2) {
> + ret = -1;
> + goto out;
> + }
> + } else {
> + int64_t blk2sz = 0;
> +
> + if (!(dd.conv & C_NOTRUNC)) {
> + /* We make conv=notrunc mandatory for the moment to avoid
> + accidental destruction of the output image. Needs to be
> + changed when a better solution is found */
> + error_report("conv=notrunc not specified");
> + ret = -1;
> + goto out;
> + }
> +
> + blk2sz = blk_getlength(blk2);
> + if (blk2sz < 0) {
> + error_report("Failed to get size for '%s'", in.filename);
> + ret = -1;
> + goto out;
> + }
> +
> + if (!(dd.conv & C_NOTRUNC)) {
> + blk_truncate(blk2, 0);
> + }
> + if (!(dd.flags & C_SKIP) || (in.offset <= INT64_MAX / in.bsz &&
> + size >= in.offset * in.bsz)) {
> + if (!(dd.conv & C_NOTRUNC) ||
> + blk2sz < size - in.offset * in.bsz) {
> + blk_truncate(blk2, size - in.offset * in.bsz);
> + }
> + }
> }
>
> if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
> @@ -4141,6 +4217,7 @@ static int img_dd(int argc, char **argv)
> out:
> g_free(arg);
> qemu_opts_del(opts);
> + qemu_opts_del(qopts);
> qemu_opts_free(create_opts);
> blk_unref(blk1);
> blk_unref(blk2);
> --
> 2.9.2
>
>
Fam
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-08-15 5:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-11 13:04 [Qemu-devel] [PATCH v2] qemu-img: change opening method for the output in dd Reda Sallahi
2016-08-15 5:44 ` 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).