From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55407) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8rSD-00038l-GT for qemu-devel@nongnu.org; Wed, 07 Jan 2015 09:19:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8rSB-00042z-BO for qemu-devel@nongnu.org; Wed, 07 Jan 2015 09:19:21 -0500 Received: from mail.lekensteyn.nl ([2a02:2308::360:1:25]:37545) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8rSA-00042G-Vv for qemu-devel@nongnu.org; Wed, 07 Jan 2015 09:19:19 -0500 From: Peter Wu Date: Wed, 07 Jan 2015 15:19:13 +0100 Message-ID: <33825683.QeFjLdu2v7@al> In-Reply-To: <20150107131934.GB22440@stefanha-thinkpad.redhat.com> References: <1420566495-13284-1-git-send-email-peter@lekensteyn.nl> <1420566495-13284-2-git-send-email-peter@lekensteyn.nl> <20150107131934.GB22440@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [Qemu-devel] [PATCH v2 01/12] block/dmg: properly detect the UDIF trailer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , John Snow , qemu-devel@nongnu.org, Stefan Hajnoczi On Wednesday 07 January 2015 13:19:34 Stefan Hajnoczi wrote: > On Tue, Jan 06, 2015 at 06:48:04PM +0100, Peter Wu wrote: > > DMG files have a variable length with a UDIF trailer at the end of a > > file. This UDIF trailer is essential as it describes the contents of > > the image. At the moment however, the start of this trailer is almost > > always incorrect as bdrv_getlength() returns a multiple of the block > > size (rounded up). This results in a failure to recognize DMG files, > > resulting in Invalid argument (EINVAL) errors. > > > > As there is no API to retrieve the real file size, look for the magic > > header in the last two sectors to find the start of this 512-byte UDIF > > trailer (the "koly" block). > > > > The resource fork offset ("info_begin") has its offset adjusted as the > > initial value of offset does not mean "end of file" anymore, but "begin > > of UDIF trailer". > > > > Signed-off-by: Peter Wu > > Reviewed-by: John Snow > > --- > > v2: added R-b, set errp as suggested by Stefan Hajnoczi > > --- > > block/dmg.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 45 insertions(+), 4 deletions(-) > > > > diff --git a/block/dmg.c b/block/dmg.c > > index e455886..9183459 100644 > > --- a/block/dmg.c > > +++ b/block/dmg.c > > @@ -131,6 +131,48 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, > > } > > } > > > > +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs, Error **errp) > > +{ > > + int64_t length; > > + int64_t offset = 0; > > + uint8_t buffer[515]; > > + int i, ret; > > + > > + /* bdrv_getlength returns a multiple of block size (512), rounded up. Since > > + * dmg images can have odd sizes, try to look for the "koly" magic which > > + * marks the begin of the UDIF trailer (512 bytes). This magic can be found > > + * in the last 511 bytes of the second-last sector or the first 4 bytes of > > + * the last sector (search space: 515 bytes) */ > > + length = bdrv_getlength(file_bs); > > + if (length < 0) { > > + error_setg_errno(errp, -length, > > + "Failed to get file size while reading UDIF trailer"); > > + return length; > > + } else if (length < 512) { > > + error_set(errp, ERROR_CLASS_GENERIC_ERROR, > > + "dmg file must be at least 512 bytes long"); > > + return -EINVAL; > > Not worth respinning but error_setg() is a shortcut for error_set(errp, > ERROR_CLASS_GENERIC_ERROR, fmt, ...). > > Reviewed-by: Stefan Hajnoczi Good to know, I got a compile error when ERROR_CLASS_GENERIC_ERROR was omitted, didn't think of using error_setg instead. When merging, please replace the above ERROR_CLASS_GENERIC_ERROR by: error_setg(errp, "dmg file must be at least 512 bytes long"); (and likewise for the last error_set in the function) -- Kind regards, Peter https://lekensteyn.nl