[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/10] block/dmg: properly detect the UDIF trail
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 01/10] block/dmg: properly detect the UDIF trailer |
Date: |
Tue, 6 Jan 2015 13:35:23 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Sat, Dec 27, 2014 at 04:01:35PM +0100, Peter Wu wrote:
> diff --git a/block/dmg.c b/block/dmg.c
> index e455886..df274f9 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -131,6 +131,39 @@ static void update_max_chunk_size(BDRVDMGState *s,
> uint32_t chunk,
> }
> }
>
> +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs)
> +{
> + 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 < 512) {
> + return length < 0 ? length : -EINVAL;
dmg_open() should pass in Error *errp so a detailed error reporting can
be used:
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, "dmg file must be at least 512 bytes long");
return -EINVAL;
}
This makes it much easier to pinpoint errors (instead of just -EINVAL)
and also gives the user a hint about the cause.
> + }
> + if (length > 511 + 512) {
> + offset = length - 511 - 512;
> + }
> + length = length < 515 ? length : 515;
> + ret = bdrv_pread(file_bs, offset, buffer, length);
> + if (ret < 4) {
> + return ret < 0 ? ret : -EINVAL;
bdrv_pread() does not return short reads. The return value will either
be length or an error. This could be just:
if (ret < 0) {
error_setg_errno(errp, -ret, "Failed to read last sectors in dmg file");
return ret;
}
(The unique error string makes it easy to track down the location where
the error occurs.)
> + }
> + for (i = 0; i < length - 3; i++) {
> + if (buffer[i] == 'k' && buffer[i+1] == 'o' &&
> + buffer[i+2] == 'l' && buffer[i+3] == 'y') {
> + return offset + i;
> + }
> + }
> + return -EINVAL;
error_set(errp, "Not a dmg file, unable to find UDIF footer");
return -EINVAL;
pgplHJG6gHeF_.pgp
Description: PGP signature