Re: [Qemu-devel] [PATCH v5] qemu-img: change opening method for the output in dd

2016-09-13 Thread Stefan Hajnoczi
On Fri, Aug 26, 2016 at 09:19:40AM +0200, Reda Sallahi wrote:
> -blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
> -false, false);
> +ret = access(out_filename, F_OK); /* Check if file exists */

The QEMU block layer does not use POSIX file I/O because it needs to
support protocols like iSCSI and ssh.

The only way to test for the existence of a disk image is by opening it
with block layer APIs.

Can you just move the img_open() call from the else statement up here
instead?


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH v5] qemu-img: change opening method for the output in dd

2016-08-26 Thread Reda Sallahi
The subcommand dd was creating an output image regardless of whether there
was one already created. With this patch we try to check first if the output
image exists and resize it if necessary.

We also make it mandatory to specify conv=notrunc when the file already
exists.

Signed-off-by: Reda Sallahi 
---
Depends on:
[PATCH v3] qemu-img: add conv=notrunc option to dd

Changes from v4:
* Use access() to check file existence first and remove blk_new_open()
Changes from v3:
* Remove unnecessary checks
Changes from v2:
* Remove redundant code
Changes from v1:
* add --image-opts handling

 qemu-img.c | 142 +++--
 1 file changed, 92 insertions(+), 50 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index f8ba5e5..816a406 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3919,14 +3919,15 @@ static int img_dd(int argc, char **argv)
 char *tmp;
 BlockDriver *drv = NULL, *proto_drv = NULL;
 BlockBackend *blk1 = NULL, *blk2 = NULL;
-QemuOpts *opts = NULL;
+QemuOpts *opts = NULL, *qopts = NULL;
 QemuOptsList *create_opts = NULL;
 Error *local_err = NULL;
 bool image_opts = false;
 int c, i;
 const char *out_fmt = "raw";
 const char *fmt = NULL;
-int64_t size = 0;
+const char *out_filename;
+int64_t size = 0, out_size;
 int64_t block_count = 0, out_pos, in_pos;
 struct DdInfo dd = {
 .flags = 0,
@@ -4030,36 +4031,6 @@ static int img_dd(int argc, char **argv)
 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, _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, _abort);
-
 size = blk_getlength(blk1);
 if (size < 0) {
 error_report("Failed to get size for '%s'", in.filename);
@@ -4071,31 +4042,101 @@ 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, _abort);
+if (in.offset > INT64_MAX / in.bsz || size < in.offset * in.bsz) {
+out_size = 0;
 } else {
-qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
-size - in.bsz * in.offset, _abort);
+out_size = size - in.offset * in.bsz;
 }
 
-ret = bdrv_create(drv, out.filename, opts, _err);
-if (ret < 0) {
-error_reportf_err(local_err,
-  "%s: error while creating output image: ",
-  out.filename);
-ret = -1;
-goto out;
+out_filename = out.filename;
+if (image_opts) {
+qopts = qemu_opts_parse_noisily(qemu_find_opts("source"),
+out.filename, true);
+out_filename = qemu_opt_get(qopts, "filename");
 }
 
-blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
-false, false);
+ret = access(out_filename, F_OK); /* Check if file exists */
 
-if (!blk2) {
-ret = -1;
-goto out;
+if (ret == -1) {
+ret = 0; /* Reset */
+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, _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 =