From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:41284) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFkkV-0001Bt-64 for qemu-devel@nongnu.org; Fri, 29 Apr 2011 06:16:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QFkkT-0006y6-Sr for qemu-devel@nongnu.org; Fri, 29 Apr 2011 06:16:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33264) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFkkT-0006xy-G2 for qemu-devel@nongnu.org; Fri, 29 Apr 2011 06:16:33 -0400 Message-ID: <4DBA9091.7090202@redhat.com> Date: Fri, 29 Apr 2011 12:18:57 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <4DBA1782.7050600@fnarfbargle.com> In-Reply-To: <4DBA1782.7050600@fnarfbargle.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Make qemu-img convert properly consider backing file contents when used with -o backing_file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Brad Campbell Cc: qemu-devel@nongnu.org Am 29.04.2011 03:42, schrieb Brad Campbell: > G'day all, > > This patch makes qemu-img properly consider the contents of the output > backing file when performing a convert operation. All things considered > it would also perform similar to rebase, where you could specify a > completely different backing file and it would just de-dup. > > I've poked this in as an attachment as apparently my last attempt at an > in-line patch munged the formatting. > > Comments, pokes or flames welcome. Please include a commit comment and a Signed-off-by line. Instead of sending a plain diff, you'll get a better result if you have a git commit locally and use git format-patch to create the patch file. Also please make sure to avoid trailing whitespace. > > @@ -719,6 +720,16 @@ static int img_convert(int argc, char **argv) > out_baseimg_param = get_option_parameter(param, BLOCK_OPT_BACKING_FILE); > if (out_baseimg_param) { > out_baseimg = out_baseimg_param->value.s; > + > + /* out_baseimg_parm != NULL even if there is no base img specified! */ > + if (out_baseimg) { > + out_bf = bdrv_new_open(out_baseimg, NULL, BDRV_O_FLAGS); > + if (!out_bf) { > + error_report("Could not open backing file '%s'", out_baseimg); > + ret = -1; > + goto out; > bdrv_new_open already prints an error message in case of failure, so we shouldn't duplicate it here. > @@ -889,8 +903,15 @@ static int img_convert(int argc, char **argv) > are present in both the output's and input's base images (no > need to copy them). */ > if (out_baseimg) { > - if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, > - n, &n1)) { > + if (bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n) < 0) { > + error_report("error while reading input file"); > + goto out; > + } > + if (bdrv_read(out_bf, sector_num - bs_offset, buf3, n) < 0) { > + error_report("error while reading backing file"); > + goto out; > + } Did you test what happens with a backing file that is smaller than the overlay? I suspect it will fail here after this change. Also I think you need to set ret = -1 before jumping to out: Kevin