From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48434) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5xIJ-0002CB-IZ for qemu-devel@nongnu.org; Tue, 20 Sep 2011 06:11:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R5xIF-0004cW-AG for qemu-devel@nongnu.org; Tue, 20 Sep 2011 06:11:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57819) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5xIF-0004cS-23 for qemu-devel@nongnu.org; Tue, 20 Sep 2011 06:11:11 -0400 Message-ID: <4E78676F.7040606@redhat.com> Date: Tue, 20 Sep 2011 12:14:07 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1315839554-14357-1-git-send-email-devin122@gmail.com> In-Reply-To: <1315839554-14357-1-git-send-email-devin122@gmail.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/6] qed: add bdrv_qed_copy_header() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Devin Nakamura Cc: qemu-devel@nongnu.org Am 12.09.2011 16:59, schrieb Devin Nakamura: > Signed-off-by: Devin Nakamura > --- > block/qed.c | 15 +++++++++++++++ > 1 files changed, 15 insertions(+), 0 deletions(-) > > diff --git a/block/qed.c b/block/qed.c > index 341cf9d..caecdff 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -1586,6 +1586,20 @@ static int bdrv_qed_map(BlockDriverState *bs, uint64_t guest_offset, > return 0; > } > > +static int bdrv_qed_copy_header(BlockDriverState *bs, char* filename) > +{ > + BlockDriverState *backup; > + uint8_t buffer[512]; > + bdrv_create_file(filename, NULL); > + bdrv_file_open(&backup, filename, BDRV_O_RDWR); > + bdrv_read(bs->file, 0, buffer, 1); /*TODO: check return code*/ > + bdrv_write(backup, 0, buffer, 1); /*TODO: check return code*/ Come on, checking return values isn't much more work than adding a TODO comment. > + bdrv_close(backup); > + > + qed_write_header_sync(bs->opaque); > + return 0; > +} This is probably right, but it makes me wonder if we are sure that QED never tries to write out the header before bdrv_qed_copy_header() is called? In that case we would have destroyed the original format before completing the conversion. I would feel more comfortable if BDRVQEDState had a field header_offset that would be set to a temporary location by bdrv_qed_open_conversion_target and reset to 0 by bdrv_qed_copy_header. Kevin