| Message ID | 1406035177-221890-2-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 22, 2014 at 05:19:34PM +0400, Denis V. Lunev wrote: > Parallels image format has several additional fields inside: > - nb_sectors is actually 64 bit wide. Upper 32bits are not used for > images with signature "WithoutFreeSpace" and must be explicitely s/explicitely/explicitly > zeroed according to Parallels. They will be used for images with > signature "WithouFreSpacExt" > - inuse is magic which means that the image is currently opened for > read/write or was not closed correctly, the magic is 0x746f6e59 > - data_off is the location of the first data block. It can be zero > and in this case I think you may have forgotten to finish this sentence :) > > This patch adds these values to struct parallels_header and adds > proper handling of nb_sectors for currently supported WithoutFreeSpace > images. > > WithouFreSpacExt will be covered in the next patch. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/parallels.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 1a5bd35..c44df87 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -41,8 +41,10 @@ struct parallels_header { > uint32_t cylinders; > uint32_t tracks; > uint32_t catalog_entries; > - uint32_t nb_sectors; > - char padding[24]; > + uint64_t nb_sectors; > + uint32_t inuse; > + uint32_t data_off; > + char padding[12]; > } QEMU_PACKED; > > typedef struct BDRVParallelsState { > @@ -90,7 +92,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > goto fail; > } > > - bs->total_sectors = le32_to_cpu(ph.nb_sectors); > + bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors); I think an explicit bit mask on the upper 32 bits would fit better here than a cast, especially since neither 'bs->total_sectors' nor 'ph.nb_sectors' is a uint32_t. E.g.: bs->total_sectors = 0xffffffff & le64_to_cpu(ph.nb_sectors); > > s->tracks = le32_to_cpu(ph.tracks); > if (s->tracks == 0) { > -- > 1.9.1 > >
On 24/07/14 22:34, Jeff Cody wrote: > On Tue, Jul 22, 2014 at 05:19:34PM +0400, Denis V. Lunev wrote: >> Parallels image format has several additional fields inside: >> - nb_sectors is actually 64 bit wide. Upper 32bits are not used for >> images with signature "WithoutFreeSpace" and must be explicitely > s/explicitely/explicitly > >> zeroed according to Parallels. They will be used for images with >> signature "WithouFreSpacExt" >> - inuse is magic which means that the image is currently opened for >> read/write or was not closed correctly, the magic is 0x746f6e59 >> - data_off is the location of the first data block. It can be zero >> and in this case > I think you may have forgotten to finish this sentence :) ok > >> This patch adds these values to struct parallels_header and adds >> proper handling of nb_sectors for currently supported WithoutFreeSpace >> images. >> >> WithouFreSpacExt will be covered in the next patch. >> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> block/parallels.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index 1a5bd35..c44df87 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -41,8 +41,10 @@ struct parallels_header { >> uint32_t cylinders; >> uint32_t tracks; >> uint32_t catalog_entries; >> - uint32_t nb_sectors; >> - char padding[24]; >> + uint64_t nb_sectors; >> + uint32_t inuse; >> + uint32_t data_off; >> + char padding[12]; >> } QEMU_PACKED; >> >> typedef struct BDRVParallelsState { >> @@ -90,7 +92,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> goto fail; >> } >> >> - bs->total_sectors = le32_to_cpu(ph.nb_sectors); >> + bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors); > I think an explicit bit mask on the upper 32 bits would fit better > here than a cast, especially since neither 'bs->total_sectors' nor > 'ph.nb_sectors' is a uint32_t. E.g.: > > bs->total_sectors = 0xffffffff & le64_to_cpu(ph.nb_sectors); > ok, will do
diff --git a/block/parallels.c b/block/parallels.c index 1a5bd35..c44df87 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -41,8 +41,10 @@ struct parallels_header { uint32_t cylinders; uint32_t tracks; uint32_t catalog_entries; - uint32_t nb_sectors; - char padding[24]; + uint64_t nb_sectors; + uint32_t inuse; + uint32_t data_off; + char padding[12]; } QEMU_PACKED; typedef struct BDRVParallelsState { @@ -90,7 +92,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } - bs->total_sectors = le32_to_cpu(ph.nb_sectors); + bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors); s->tracks = le32_to_cpu(ph.tracks); if (s->tracks == 0) {
Parallels image format has several additional fields inside: - nb_sectors is actually 64 bit wide. Upper 32bits are not used for images with signature "WithoutFreeSpace" and must be explicitely zeroed according to Parallels. They will be used for images with signature "WithouFreSpacExt" - inuse is magic which means that the image is currently opened for read/write or was not closed correctly, the magic is 0x746f6e59 - data_off is the location of the first data block. It can be zero and in this case This patch adds these values to struct parallels_header and adds proper handling of nb_sectors for currently supported WithoutFreeSpace images. WithouFreSpacExt will be covered in the next patch. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Stefan Hajnoczi <stefanha@redhat.com> --- block/parallels.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)