[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] block/parallels: 2TB+ parallels images supp
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] block/parallels: 2TB+ parallels images support |
Date: |
Thu, 24 Jul 2014 15:25:38 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Jul 22, 2014 at 05:19:37PM +0400, Denis V. Lunev wrote:
> Parallels has released in the recent updates of Parallels Server 5/6
> new addition to his image format. Images with signature WithouFreSpacExt
> have offsets in the catalog coded not as offsets in sectors (multiple
> of 512 bytes) but offsets coded in blocks (i.e. header->tracks * 512)
>
> In this case all 64 bits of header->nb_sectors are used for image size.
>
> This patch implements support of this for qemu-img.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> ---
> block/parallels.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 02739cf..d9cb04f 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -30,6 +30,7 @@
> /**************************************************************/
>
> #define HEADER_MAGIC "WithoutFreeSpace"
> +#define HEADER_MAGIC2 "WithouFreSpacExt"
> #define HEADER_VERSION 2
> #define HEADER_SIZE 64
>
> @@ -54,6 +55,8 @@ typedef struct BDRVParallelsState {
> unsigned int catalog_size;
>
> unsigned int tracks;
> +
> + unsigned int off_multiplier;
> } BDRVParallelsState;
>
> static int parallels_probe(const uint8_t *buf, int buf_size, const char
> *filename)
> @@ -63,7 +66,8 @@ static int parallels_probe(const uint8_t *buf, int
> buf_size, const char *filenam
> if (buf_size < HEADER_SIZE)
> return 0;
>
> - if (!memcmp(ph->magic, HEADER_MAGIC, 16) &&
> + if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
> + !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
> (le32_to_cpu(ph->version) == HEADER_VERSION))
> return 100;
>
> @@ -85,13 +89,18 @@ static int parallels_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto fail;
> }
>
> + bs->total_sectors = le64_to_cpu(ph.nb_sectors);
> +
bs->total_sectors is a int64_t, and ph.nb_sectors is a uint64_t.
Should we do a sanity check here on the max number of sectors?
> if (le32_to_cpu(ph.version) != HEADER_VERSION)
> goto fail_format;
> - if (memcmp(ph.magic, HEADER_MAGIC, 16))
> + if (!memcmp(ph.magic, HEADER_MAGIC, 16)) {
> + s->off_multiplier = 1;
> + bs->total_sectors = (uint32_t)bs->total_sectors;
(same comment as in patch 1, w.r.t. cast vs. bitmask)
> + } else if (!memcmp(ph.magic, HEADER_MAGIC2, 16))
> + s->off_multiplier = le32_to_cpu(ph.tracks);
Is there a maximum size in the specification for ph.tracks?
> + else
> goto fail_format;
(same comment as the last patch, w.r.t. braces)
>
> - bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors);
> -
> s->tracks = le32_to_cpu(ph.tracks);
> if (s->tracks == 0) {
> error_setg(errp, "Invalid image: Zero sectors per track");
> @@ -137,7 +146,8 @@ static int64_t seek_to_sector(BlockDriverState *bs,
> int64_t sector_num)
> /* not allocated */
> if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
> return -1;
> - return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
> + return
> + ((uint64_t)s->catalog_bitmap[index] * s->off_multiplier + offset) *
> 512;
Do we need to worry about overflow here, depending on the value of
off_multiplier?
Also, (and this existed prior to this patch), - we are casting to
uint64_t, although the function returns int64_t.
> }
>
> static int parallels_read(BlockDriverState *bs, int64_t sector_num,
> --
> 1.9.1
>
>