From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=39358 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PTC6O-0003gz-Fg for qemu-devel@nongnu.org; Thu, 16 Dec 2010 06:34:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PTC6J-0005Gw-AE for qemu-devel@nongnu.org; Thu, 16 Dec 2010 06:34:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:21543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PTC6I-0005Gr-Tq for qemu-devel@nongnu.org; Thu, 16 Dec 2010 06:34:23 -0500 Message-ID: <4D09F985.6050706@redhat.com> Date: Thu, 16 Dec 2010 12:35:33 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1292497454-32573-1-git-send-email-Jes.Sorensen@redhat.com> <1292497454-32573-2-git-send-email-Jes.Sorensen@redhat.com> In-Reply-To: <1292497454-32573-2-git-send-email-Jes.Sorensen@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 1/3] qemu-img.c: Re-factor img_create() List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes.Sorensen@redhat.com Cc: qemu-devel@nongnu.org, armbru@redhat.com, stefanha@linux.vnet.ibm.com Am 16.12.2010 12:04, schrieb Jes.Sorensen@redhat.com: > From: Jes Sorensen > > This patch re-factors img_create() moving the code doing the actual > work into block.c where it can be shared with QEMU. This is needed to > be able to create images from QEMU to be used for live snapshots. > > Signed-off-by: Jes Sorensen > --- > block.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > block.h | 4 ++ > qemu-img.c | 108 +-------------------------------------------- > 3 files changed, 150 insertions(+), 106 deletions(-) > > diff --git a/block.c b/block.c > index b4aaf41..765f9f3 100644 > --- a/block.c > +++ b/block.c > @@ -2758,3 +2758,147 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs) > { > return bs->dirty_count; > } > + > +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 = NULL, *create_options = NULL; > + QEMUOptionParameter *backing_fmt; > + BlockDriverState *bs = NULL; > + BlockDriver *drv, *proto_drv; > + int ret = 0; > + > + /* Find driver and parse its options */ > + drv = bdrv_find_format(fmt); > + if (!drv) { > + error_report("Unknown file format '%s'", fmt); > + ret = -1; > + goto out; > + } > + > + proto_drv = bdrv_find_protocol(filename); > + if (!proto_drv) { > + error_report("Unknown protocol '%s'", filename); > + ret = -1; > + goto out; > + } > + > + create_options = append_option_parameters(create_options, > + drv->create_options); > + create_options = append_option_parameters(create_options, > + proto_drv->create_options); > + > + /* Create parameter list with default values */ > + param = parse_option_parameters("", create_options, param); > + > + set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); > + > + /* Parse -o options */ > + if (options) { > + param = parse_option_parameters(options, create_options, param); > + if (param == NULL) { > + error_report("Invalid options for file format '%s'.", fmt); > + ret = -1; > + goto out; > + } > + } > + > + if (base_filename) { > + if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE, > + base_filename)) { > + error_report("Backing file not supported for file format '%s'", > + fmt); > + ret = -1; > + goto out; > + } > + } > + > + backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT); > + if (backing_fmt && backing_fmt->value.s) { > + if (!bdrv_find_format(backing_fmt->value.s)) { > + error_report("Unknown backing file format '%s'", > + backing_fmt->value.s); > + ret = -1; > + goto out; > + } > + } > + > + if (base_fmt) { > + if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) { > + error_report("Backing file format not supported for file " > + "format '%s'", fmt); > + ret = -1; > + goto out; > + } > + } The order is wrong here. If you use -F, the backing format won't be checked. > + > + // 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 > + if (get_option_parameter(param, BLOCK_OPT_SIZE)->value.n == -1) { > + QEMUOptionParameter *backing_file = > + get_option_parameter(param, BLOCK_OPT_BACKING_FILE); > + > + if (backing_file && backing_file->value.s) { > + uint64_t size; > + const char *fmt = NULL; > + char buf[32]; > + > + if (backing_fmt && backing_fmt->value.s) { > + fmt = backing_fmt->value.s; > + } > + > + bs = bdrv_new(""); > + if (!bs) { > + error_report("Not enough memory to allocate BlockDriverState"); > + ret = -1; > + goto out; > + } bdrv_new never returns NULL (it's an indirect qemu_malloc call). > + ret = bdrv_open(bs, backing_file->value.s, flags, drv); > + if (ret < 0) { > + error_report("Could not open '%s'", filename); > + ret = -1; > + goto out; > + } > + bdrv_get_geometry(bs, &size); > + size *= 512; > + > + snprintf(buf, sizeof(buf), "%" PRId64, size); > + set_option_parameter(param, BLOCK_OPT_SIZE, buf); > + } else { > + error_report("Image creation needs a size parameter"); > + ret = -1; > + goto out; > + } > + } > + > + printf("Formatting '%s', fmt=%s ", filename, fmt); > + print_option_parameters(param); > + puts(""); > + > + ret = bdrv_create(drv, filename, param); > + free_option_parameters(create_options); > + free_option_parameters(param); These need to be after out: to avoid leaking in error cases. You're basically reverting a87a6721d with this. > + > + if (ret < 0) { > + if (ret == -ENOTSUP) { > + error_report("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); > + } else { > + error_report("%s: error while creating %s: %s", filename, fmt, > + strerror(-ret)); > + } > + } > + > +out: > + if (bs) { > + bdrv_delete(bs); > + } > + if (ret) { > + return 1; > + } Maybe we should better use the usual 0/-errno style. In qemu-img it was the exit code of the program, but now it's a block layer function. Kevin