From 0d3cddbc24adf5dd4de811d5f3e69e129fd21b89 Mon Sep 17 00:00:00 2001 From: Jes Sorensen 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 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 RH-Acked-by: Marcelo Tosatti RH-Acked-by: Kevin Wolf From: MORITA Kazutaka 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 Signed-off-by: Kevin Wolf (cherry picked from commit c2abccecd93d5977460fdfdab19461ccfa09ae21) --- qemu-img.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 183 insertions(+), 52 deletions(-) Signed-off-by: Luiz Capitulino --- 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