From 0d3cddbc24adf5dd4de811d5f3e69e129fd21b89 Mon Sep 17 00:00:00 2001
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Date: Mon, 31 Jan 2011 12:23:06 -0200
Subject: [PATCH 12/37] qemu-img: avoid calling exit(1) to release resources properly

RH-Author: Jes Sorensen <Jes.Sorensen@redhat.com>
Message-id: <1296476610-28514-3-git-send-email-Jes.Sorensen@redhat.com>
Patchwork-id: 17297
O-Subject: [PATCH 02/26] qemu-img: avoid calling exit(1) to release resources
	properly
Bugzilla: 637701
RH-Acked-by: Alex Williamson <alex.williamson@redhat.com>
RH-Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>

From: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

This patch removes exit(1) from error(), and properly releases
resources such as a block driver and an allocated memory.

For testing the Sheepdog block driver with qemu-iotests, it is
necessary to call bdrv_delete() before the program exits.  Because the
driver releases the lock of VM images in the close handler.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit c2abccecd93d5977460fdfdab19461ccfa09ae21)
---
 qemu-img.c |  235 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 183 insertions(+), 52 deletions(-)

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qemu-img.c |  235 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 183 insertions(+), 52 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0744945..2f0b6a1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -39,14 +39,13 @@ typedef struct img_cmd_t {
 /* Default to cache=writeback as data integrity is not important for qemu-tcg. */
 #define BDRV_O_FLAGS BDRV_O_CACHE_WB
 
-static void QEMU_NORETURN error(const char *fmt, ...)
+static void error(const char *fmt, ...)
 {
     va_list ap;
     va_start(ap, fmt);
     fprintf(stderr, "qemu-img: ");
     vfprintf(stderr, fmt, ap);
     fprintf(stderr, "\n");
-    exit(1);
     va_end(ap);
 }
 
@@ -196,57 +195,76 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     char password[256];
 
     bs = bdrv_new("");
-    if (!bs)
+    if (!bs) {
         error("Not enough memory");
+        goto fail;
+    }
     if (fmt) {
         drv = bdrv_find_format(fmt);
-        if (!drv)
+        if (!drv) {
             error("Unknown file format '%s'", fmt);
+            goto fail;
+        }
     } else {
         drv = NULL;
     }
     if (bdrv_open(bs, filename, flags, drv) < 0) {
         error("Could not open '%s'", filename);
+        goto fail;
     }
     if (bdrv_is_encrypted(bs)) {
         printf("Disk image '%s' is encrypted.\n", filename);
-        if (read_password(password, sizeof(password)) < 0)
+        if (read_password(password, sizeof(password)) < 0) {
             error("No password given");
-        if (bdrv_set_key(bs, password) < 0)
+            goto fail;
+        }
+        if (bdrv_set_key(bs, password) < 0) {
             error("invalid password");
+            goto fail;
+        }
     }
     return bs;
+fail:
+    if (bs) {
+        bdrv_delete(bs);
+    }
+    return NULL;
 }
 
-static void add_old_style_options(const char *fmt, QEMUOptionParameter *list,
+static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
     int flags, const char *base_filename, const char *base_fmt)
 {
     if (flags & BLOCK_FLAG_ENCRYPT) {
         if (set_option_parameter(list, BLOCK_OPT_ENCRYPT, "on")) {
             error("Encryption not supported for file format '%s'", fmt);
+            return -1;
         }
     }
     if (flags & BLOCK_FLAG_COMPAT6) {
         if (set_option_parameter(list, BLOCK_OPT_COMPAT6, "on")) {
             error("VMDK version 6 not supported for file format '%s'", fmt);
+            return -1;
         }
     }
 
     if (base_filename) {
         if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE, base_filename)) {
             error("Backing file not supported for file format '%s'", fmt);
+            return -1;
         }
     }
     if (base_fmt) {
         if (set_option_parameter(list, BLOCK_OPT_BACKING_FMT, base_fmt)) {
             error("Backing file format not supported for file format '%s'", fmt);
+            return -1;
         }
     }
+    return 0;
 }
 
 static int img_create(int argc, char **argv)
 {
-    int c, ret, flags;
+    int c, ret = 0, flags;
     const char *fmt = "raw";
     const char *base_fmt = NULL;
     const char *filename;
@@ -292,12 +310,16 @@ static int img_create(int argc, char **argv)
 
     /* Find driver and parse its options */
     drv = bdrv_find_format(fmt);
-    if (!drv)
+    if (!drv) {
         error("Unknown file format '%s'", fmt);
+        return 1;
+    }
 
     proto_drv = bdrv_find_protocol(filename);
-    if (!proto_drv)
+    if (!proto_drv) {
         error("Unknown protocol '%s'", filename);
+        return 1;
+    }
 
     create_options = append_option_parameters(create_options,
                                               drv->create_options);
@@ -306,7 +328,7 @@ static int img_create(int argc, char **argv)
 
     if (options && !strcmp(options, "?")) {
         print_option_help(create_options);
-        return 0;
+        goto out;
     }
 
     /* Create parameter list with default values */
@@ -318,6 +340,8 @@ static int img_create(int argc, char **argv)
         param = parse_option_parameters(options, create_options, param);
         if (param == NULL) {
             error("Invalid options for file format '%s'.", fmt);
+            ret = -1;
+            goto out;
         }
     }
 
@@ -327,7 +351,10 @@ static int img_create(int argc, char **argv)
     }
 
     /* Add old-style options to parameters */
-    add_old_style_options(fmt, param, flags, base_filename, base_fmt);
+    ret = add_old_style_options(fmt, param, flags, base_filename, base_fmt);
+    if (ret < 0) {
+        goto out;
+    }
 
     // The size for the image must always be specified, with one exception:
     // If we are using a backing file, we can obtain the size from there
@@ -350,10 +377,16 @@ static int img_create(int argc, char **argv)
                 } else {
                      error("Unknown backing file format '%s'",
                         backing_fmt->value.s);
+                     ret = -1;
+                     goto out;
                 }
             }
 
             bs = bdrv_new_open(backing_file->value.s, fmt, BDRV_O_FLAGS);
+            if (!bs) {
+                ret = -1;
+                goto out;
+            }
             bdrv_get_geometry(bs, &size);
             size *= 512;
             bdrv_delete(bs);
@@ -362,6 +395,8 @@ static int img_create(int argc, char **argv)
             set_option_parameter(param, BLOCK_OPT_SIZE, buf);
         } else {
             error("Image creation needs a size parameter");
+            ret = -1;
+            goto out;
         }
     }
 
@@ -382,6 +417,10 @@ static int img_create(int argc, char **argv)
             error("%s: error while creating %s: %s", filename, fmt, strerror(-ret));
         }
     }
+out:
+    if (ret) {
+        return 1;
+    }
     return 0;
 }
 
@@ -419,11 +458,15 @@ static int img_check(int argc, char **argv)
     filename = argv[optind++];
 
     bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS);
+    if (!bs) {
+        return 1;
+    }
     ret = bdrv_check(bs, &result);
 
     if (ret == -ENOTSUP) {
         bdrv_delete(bs);
         error("This image format does not support checks");
+        return 1;
     }
 
     if (!(result.corruptions || result.leaks || result.check_errors)) {
@@ -491,6 +534,9 @@ static int img_commit(int argc, char **argv)
     filename = argv[optind++];
 
     bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
+    if (!bs) {
+        return 1;
+    }
     ret = bdrv_commit(bs);
     switch(ret) {
     case 0:
@@ -511,6 +557,9 @@ static int img_commit(int argc, char **argv)
     }
 
     bdrv_delete(bs);
+    if (ret) {
+        return 1;
+    }
     return 0;
 }
 
@@ -585,13 +634,13 @@ static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
 
 static int img_convert(int argc, char **argv)
 {
-    int c, ret, n, n1, bs_n, bs_i, flags, cluster_size, cluster_sectors;
+    int c, ret = 0, n, n1, bs_n, bs_i, flags, cluster_size, cluster_sectors;
     const char *fmt, *out_fmt, *out_baseimg, *out_filename;
     BlockDriver *drv, *proto_drv;
-    BlockDriverState **bs, *out_bs;
+    BlockDriverState **bs = NULL, *out_bs = NULL;
     int64_t total_sectors, nb_sectors, sector_num, bs_offset;
     uint64_t bs_sectors;
-    uint8_t * buf;
+    uint8_t * buf = NULL;
     const uint8_t *buf1;
     BlockDriverInfo bdi;
     QEMUOptionParameter *param = NULL, *create_options = NULL;
@@ -638,30 +687,43 @@ static int img_convert(int argc, char **argv)
 
     out_filename = argv[argc - 1];
 
-    if (bs_n > 1 && out_baseimg)
+    if (bs_n > 1 && out_baseimg) {
         error("-B makes no sense when concatenating multiple input images");
+        return 1;
+    }
         
     bs = calloc(bs_n, sizeof(BlockDriverState *));
-    if (!bs)
+    if (!bs) {
         error("Out of memory");
+        return 1;
+    }
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
         bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS);
-        if (!bs[bs_i])
+        if (!bs[bs_i]) {
             error("Could not open '%s'", argv[optind + bs_i]);
+            ret = -1;
+            goto out;
+        }
         bdrv_get_geometry(bs[bs_i], &bs_sectors);
         total_sectors += bs_sectors;
     }
 
     /* Find driver and parse its options */
     drv = bdrv_find_format(out_fmt);
-    if (!drv)
+    if (!drv) {
         error("Unknown file format '%s'", out_fmt);
+        ret = -1;
+        goto out;
+    }
 
     proto_drv = bdrv_find_protocol(out_filename);
-    if (!proto_drv)
+    if (!proto_drv) {
         error("Unknown protocol '%s'", out_filename);
+        ret = -1;
+        goto out;
+    }
 
     create_options = append_option_parameters(create_options,
                                               drv->create_options);
@@ -669,21 +731,25 @@ static int img_convert(int argc, char **argv)
                                               proto_drv->create_options);
     if (options && !strcmp(options, "?")) {
         print_option_help(create_options);
-        free(bs);
-        return 0;
+        goto out;
     }
 
     if (options) {
         param = parse_option_parameters(options, create_options, param);
         if (param == NULL) {
             error("Invalid options for file format '%s'.", out_fmt);
+            ret = -1;
+            goto out;
         }
     } else {
         param = parse_option_parameters("", create_options, param);
     }
 
     set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
-    add_old_style_options(out_fmt, param, flags, out_baseimg, NULL);
+    ret = add_old_style_options(out_fmt, param, flags, out_baseimg, NULL);
+    if (ret < 0) {
+        goto out;
+    }
 
     /* Check if compression is supported */
     if (flags & BLOCK_FLAG_COMPRESS) {
@@ -692,18 +758,19 @@ static int img_convert(int argc, char **argv)
 
         if (!drv->bdrv_write_compressed) {
             error("Compression not supported for this file format");
+            ret = -1;
+            goto out;
         }
 
         if (encryption && encryption->value.n) {
             error("Compression and encryption not supported at the same time");
+            ret = -1;
+            goto out;
         }
     }
 
     /* Create the new image */
     ret = bdrv_create(drv, out_filename, param);
-    free_option_parameters(create_options);
-    free_option_parameters(param);
-
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting not supported for file format '%s'", out_fmt);
@@ -712,10 +779,15 @@ static int img_convert(int argc, char **argv)
         } else {
             error("%s: error while converting %s: %s", out_filename, out_fmt, strerror(-ret));
         }
+        goto out;
     }
 
     out_bs = bdrv_new_open(out_filename, out_fmt,
         BDRV_O_FLAGS | BDRV_O_RDWR | BDRV_O_NO_FLUSH);
+    if (!out_bs) {
+        ret = -1;
+        goto out;
+    }
 
     bs_i = 0;
     bs_offset = 0;
@@ -723,11 +795,17 @@ static int img_convert(int argc, char **argv)
     buf = qemu_malloc(IO_BUF_SIZE);
 
     if (flags & BLOCK_FLAG_COMPRESS) {
-        if (bdrv_get_info(out_bs, &bdi) < 0)
+        ret = bdrv_get_info(out_bs, &bdi);
+        if (ret < 0) {
             error("could not get block driver info");
+            goto out;
+        }
         cluster_size = bdi.cluster_size;
-        if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE)
+        if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
             error("invalid cluster size");
+            ret = -1;
+            goto out;
+        }
         cluster_sectors = cluster_size >> 9;
         sector_num = 0;
         for(;;) {
@@ -763,8 +841,11 @@ static int img_convert(int argc, char **argv)
 
                 nlow = (remainder > bs_sectors - bs_num) ? bs_sectors - bs_num : remainder;
 
-                if (bdrv_read(bs[bs_i], bs_num, buf2, nlow) < 0) 
+                ret = bdrv_read(bs[bs_i], bs_num, buf2, nlow);
+                if (ret < 0) {
                     error("error while reading");
+                    goto out;
+                }
 
                 buf2 += nlow * 512;
                 bs_num += nlow;
@@ -776,10 +857,13 @@ static int img_convert(int argc, char **argv)
             if (n < cluster_sectors)
                 memset(buf + n * 512, 0, cluster_size - n * 512);
             if (is_not_zero(buf, cluster_size)) {
-                if (bdrv_write_compressed(out_bs, sector_num, buf,
-                                          cluster_sectors) != 0)
+                ret = bdrv_write_compressed(out_bs, sector_num, buf,
+                                            cluster_sectors);
+                if (ret != 0) {
                     error("error while compressing sector %" PRId64,
                           sector_num);
+                    goto out;
+                }
             }
             sector_num += n;
         }
@@ -830,8 +914,11 @@ static int img_convert(int argc, char **argv)
                 n1 = n;
             }
 
-            if (bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n) < 0) 
+            ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
+            if (ret < 0) {
                 error("error while reading");
+                goto out;
+            }
             /* NOTE: at the same time we convert, we do not write zero
                sectors to have a chance to compress the image. Ideally, we
                should add a specific call to have the info to go faster */
@@ -846,8 +933,11 @@ static int img_convert(int argc, char **argv)
                    already there is garbage, not 0s. */
                 if (!has_zero_init || out_baseimg ||
                     is_allocated_sectors(buf1, n, &n1)) {
-                    if (bdrv_write(out_bs, sector_num, buf1, n1) < 0)
+                    ret = bdrv_write(out_bs, sector_num, buf1, n1);
+                    if (ret < 0) {
                         error("error while writing");
+                        goto out;
+                    }
                 }
                 sector_num += n1;
                 n -= n1;
@@ -855,11 +945,22 @@ static int img_convert(int argc, char **argv)
             }
         }
     }
+out:
+    free_option_parameters(create_options);
+    free_option_parameters(param);
     qemu_free(buf);
-    bdrv_delete(out_bs);
-    for (bs_i = 0; bs_i < bs_n; bs_i++)
-        bdrv_delete(bs[bs_i]);
+    if (out_bs) {
+        bdrv_delete(out_bs);
+    }
+    for (bs_i = 0; bs_i < bs_n; bs_i++) {
+        if (bs[bs_i]) {
+            bdrv_delete(bs[bs_i]);
+        }
+    }
     free(bs);
+    if (ret) {
+        return 1;
+    }
     return 0;
 }
 
@@ -942,6 +1043,9 @@ static int img_info(int argc, char **argv)
     filename = argv[optind++];
 
     bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
+    if (!bs) {
+        return 1;
+    }
     bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
     bdrv_get_geometry(bs, &total_sectors);
     get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
@@ -987,7 +1091,7 @@ static int img_snapshot(int argc, char **argv)
     BlockDriverState *bs;
     QEMUSnapshotInfo sn;
     char *filename, *snapshot_name = NULL;
-    int c, ret, bdrv_oflags;
+    int c, ret = 0, bdrv_oflags;
     int action = 0;
     qemu_timeval tv;
 
@@ -1042,6 +1146,9 @@ static int img_snapshot(int argc, char **argv)
 
     /* Open the image */
     bs = bdrv_new_open(filename, NULL, bdrv_oflags);
+    if (!bs) {
+        return 1;
+    }
 
     /* Perform the requested action */
     switch(action) {
@@ -1080,13 +1187,15 @@ static int img_snapshot(int argc, char **argv)
 
     /* Cleanup */
     bdrv_delete(bs);
-
+    if (ret) {
+        return 1;
+    }
     return 0;
 }
 
 static int img_rebase(int argc, char **argv)
 {
-    BlockDriverState *bs, *bs_old_backing, *bs_new_backing;
+    BlockDriverState *bs, *bs_old_backing = NULL, *bs_new_backing = NULL;
     BlockDriver *old_backing_drv, *new_backing_drv;
     char *filename;
     const char *fmt, *out_basefmt, *out_baseimg;
@@ -1133,6 +1242,9 @@ static int img_rebase(int argc, char **argv)
      */
     flags = BDRV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
     bs = bdrv_new_open(filename, fmt, flags);
+    if (!bs) {
+        return 1;
+    }
 
     /* Find the right drivers for the backing files */
     old_backing_drv = NULL;
@@ -1142,6 +1254,8 @@ static int img_rebase(int argc, char **argv)
         old_backing_drv = bdrv_find_format(bs->backing_format);
         if (old_backing_drv == NULL) {
             error("Invalid format name: '%s'", bs->backing_format);
+            ret = -1;
+            goto out;
         }
     }
 
@@ -1149,6 +1263,8 @@ static int img_rebase(int argc, char **argv)
         new_backing_drv = bdrv_find_format(out_basefmt);
         if (new_backing_drv == NULL) {
             error("Invalid format name: '%s'", out_basefmt);
+            ret = -1;
+            goto out;
         }
     }
 
@@ -1162,19 +1278,19 @@ static int img_rebase(int argc, char **argv)
 
         bs_old_backing = bdrv_new("old_backing");
         bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
-        if (bdrv_open(bs_old_backing, backing_name, BDRV_O_FLAGS,
-            old_backing_drv))
-        {
+        ret = bdrv_open(bs_old_backing, backing_name, BDRV_O_FLAGS,
+                        old_backing_drv);
+        if (ret) {
             error("Could not open old backing file '%s'", backing_name);
-            return -1;
+            goto out;
         }
 
         bs_new_backing = bdrv_new("new_backing");
-        if (bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
-            new_backing_drv))
-        {
+        ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS | BDRV_O_RDWR,
+                        new_backing_drv);
+        if (ret) {
             error("Could not open new backing file '%s'", out_baseimg);
-            return -1;
+            goto out;
         }
     }
 
@@ -1215,11 +1331,15 @@ static int img_rebase(int argc, char **argv)
             }
 
             /* Read old and new backing file */
-            if (bdrv_read(bs_old_backing, sector, buf_old, n) < 0) {
+            ret = bdrv_read(bs_old_backing, sector, buf_old, n);
+            if (ret < 0) {
                 error("error while reading from old backing file");
+                goto out;
             }
-            if (bdrv_read(bs_new_backing, sector, buf_new, n) < 0) {
+            ret = bdrv_read(bs_new_backing, sector, buf_new, n);
+            if (ret < 0) {
                 error("error while reading from new backing file");
+                goto out;
             }
 
             /* If they differ, we need to write to the COW file */
@@ -1236,6 +1356,7 @@ static int img_rebase(int argc, char **argv)
                     if (ret < 0) {
                         error("Error while writing to COW image: %s",
                             strerror(-ret));
+                        goto out;
                     }
                 }
 
@@ -1267,7 +1388,7 @@ static int img_rebase(int argc, char **argv)
      * could be dropped from the COW file. Don't do this before switching the
      * backing file, in case of a crash this would lead to corruption.
      */
-
+out:
     /* Cleanup */
     if (!unsafe) {
         bdrv_delete(bs_old_backing);
@@ -1275,7 +1396,9 @@ static int img_rebase(int argc, char **argv)
     }
 
     bdrv_delete(bs);
-
+    if (ret) {
+        return 1;
+    }
     return 0;
 }
 
@@ -1341,6 +1464,9 @@ static int img_resize(int argc, char **argv)
     free_option_parameters(param);
 
     bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
+    if (!bs) {
+        return 1;
+    }
 
     if (relative) {
         total_size = bdrv_getlength(bs) + n * relative;
@@ -1349,6 +1475,8 @@ static int img_resize(int argc, char **argv)
     }
     if (total_size <= 0) {
         error("New image size must be positive");
+        ret = -1;
+        goto out;
     }
 
     ret = bdrv_truncate(bs, total_size);
@@ -1366,8 +1494,11 @@ static int img_resize(int argc, char **argv)
         error("Error resizing image (%d)", -ret);
         break;
     }
-
+out:
     bdrv_delete(bs);
+    if (ret) {
+        return 1;
+    }
     return 0;
 }
 
-- 
1.7.4.rc1.16.gd2f15e