[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] block/vpc.c: Detect too-large vpc file
From: |
Serge E. Hallyn |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] block/vpc.c: Detect too-large vpc file |
Date: |
Wed, 27 Jul 2011 15:16:58 +0000 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
Quoting Kevin Wolf (address@hidden):
> > The footer->size appears to be double the 'real' size. So I'm actually
> > doing
> > the blow. Does this seem sensible?
>
> Double size sounds really weird. 'qemu-img create' uses the size in
> bytes for it. Is that wrong?
>
> > Doing it this way, trying to convert the image gives me '-EPERM' rather than
> > -EFBIG. Not sure where we are best off catching that.
>
> Hm, any idea where the -EPERM (-1) comes from? Maybe wrong total_sectors
> number you're calculating?
No, vpc.c in some places just returns 0 or -1, while that return value is
taken to be a more meaningful errno by the caller. -1 is EPERM. That's
why my original patch had to return -EFBIG.
> > Subject: [PATCH 1/1] vpc: accurately detect file size
> >
> > VHD files technically can be up to 2Tb, but virtual pc uses CHS which
> > is limited to 127G. Currently qemu-img refused to create vpc files > 127G,
> > but it reports any file >= 127G as being 127G. If asked to convert such a
> > file, it creates a 127G (truncated) file without returning error.
> >
> > This patch uses the size reported in the footer to detect whether the
> > size is > 127G, and reports the size stored in footer if so.
> >
> > qemu-img info now reports the correct size.
> >
> > qemu-img convert refuses, but returns -EPERM rather than -EFBIG.
> >
> > See https://bugs.launchpad.net/qemu/+bug/814222 for details.
> >
> > Changelog: follow Kevin Wolf's suggestion for detecting true file size.
> >
> > Signed-off-by: Serge Hallyn <address@hidden>
> > ---
> > block/vpc.c | 10 ++++++++--
> > 1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/vpc.c b/block/vpc.c
> > index 56865da..37eebc2 100644
> > --- a/block/vpc.c
> > +++ b/block/vpc.c
> > @@ -173,8 +173,14 @@ static int vpc_open(BlockDriverState *bs, int flags)
> > // The visible size of a image in Virtual PC depends on the geometry
> > // rather than on the size stored in the footer (the size in the footer
> > // is too large usually)
> > - bs->total_sectors = (int64_t)
> > - be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
> > + // If the size stored in the footer is larger than CHS can represent,
> > + // then use the size stored in the footer.
> > + if (footer->size > (uint64_t) 65536 * 16 * 255 * 2) {
> > + bs->total_sectors = footer->size / 2;
>
> If footer->size is the double byte count, then it would be:;
>
> + if (footer->size / 2 > (uint64_t) 65536 * 16 * 255 * 512) {
> + bs->total_sectors = 512 * footer->size / 2;
I'm just reporting the facts :) The patch I sent ended up returning
the right size for 140G dynamic VHD image.
So given our fuzziness on the actual meanings, I think the right thing
to do is just:
if (footer->size > 65536 * 16 * 255 * 2)
return -EFBIG;
> Your calculation would assume that footer->size uses units of 256 bytes,
> which appears rather unlikely.