The following changes since commit 20d6c7312f1b812bb9c750f4087f69ac8485cc90:

Merge remote-tracking branch 'remotes/palmer/tags/riscv-for-master-3.2-part1' into staging (2019-01-03 13:26:30 +0000)

git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 39a0408e768cd00142f5b57d27ab234282bf4df5:

dmg: don't skip zero chunk (2019-01-04 11:15:09 +0000)

Bug fixes for the .dmg image file format.

Julio Faracco (1):

dmg: Fixing wrong dmg block type value for block terminator.

yuchenlin (3):

dmg: fix binary search

dmg: use enumeration type instead of hard coding number

dmg: don't skip zero chunk

block/dmg.c | 31 ++++++++++++++++++++-----------

1 file changed, 20 insertions(+), 11 deletions(-)

2.20.1

From: yuchenlin <npes87184@gmail.com>

The dmg file has many tables which describe: "start from sector XXX to

sector XXX, the compression method is XXX and where the compressed data

resides on".

Each sector in the expanded file should be covered by a table. The table

will describe the offset of compressed data (or raw depends on the type)

in the dmg.

block/dmg.c | 19 ++++++++++++-------

1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c

--- a/block/dmg.c

+++ b/block/dmg.c

@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,

case UDRW: /* copy */

uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);

break;

- case UDIG: /* zero */

+ case UDZE: /* zero */

+ case UDIG: /* ignore */

/* as the all-zeroes block may be large, it is treated specially: the

* sector is not copied from a large buffer, a simple memset is used

* instead. Therefore uncompressed_sectors does not need to be set. */

@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {

static bool dmg_is_known_block_type(uint32_t entry_type)

switch (entry_type) {

+ case UDZE: /* zeros */

case UDRW: /* uncompressed */

- case UDIG: /* zeroes */

+ case UDIG: /* ignore */

case UDZO: /* zlib */

return true;

case UDBZ: /* bzip2 */

@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,

/* sector count */

s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);

- /* all-zeroes sector (type 2) does not need to be "uncompressed" and can

- * therefore be unbounded. */

- if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {

+ /* all-zeroes sector (type UDZE and UDIG) does not need to be

+ * "uncompressed" and can therefore be unbounded. */

+ if (s->types[i] != UDZE && s->types[i] != UDIG

+ && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {

error_report("sector count %" PRIu64 " for chunk %" PRIu32

" is larger than max (%u)",

s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);

@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)

return -1;

}

break;

- case UDIG: /* zero */

+ case UDZE: /* zeros */

+ case UDIG: /* ignore */

/* see dmg_read, it is treated specially. No buffer needs to be

* pre-filled, the zeroes can be set directly. */

break;

@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,

/* Special case: current chunk is all zeroes. Do not perform a memcpy as

* s->uncompressed_chunk may be too small to cover the large all-zeroes

* section. dmg_read_chunk is called to find s->current_chunk */

- if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */

+ if (s->types[s->current_chunk] == UDZE

+ || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */

qemu_iovec_memset(qiov, i * 512, 0, 512);

continue;

}

2.20.1

From: Julio Faracco <jcfaracco@gmail.com>

This is a trivial patch to fix a wrong value for block terminator.

The old value was 0x7fffffff which is wrong. It was not affecting the

code because QEMU dmg block is not handling block terminator right now.

Neverthless, it should be fixed.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>

Reviewed-by: yuchenlin <yuchenlin@synology.com>

Message-id: 20181228145055.18039-1-jcfaracco@gmail.com

block/dmg.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/dmg.c b/block/dmg.c

--- a/block/dmg.c

+++ b/block/dmg.c

@@ -XXX,XX +XXX,XX @@ enum {

UDBZ,

ULFO,

UDCM = 0x7ffffffe, /* Comments */

- UDLE /* Last Entry */

+ UDLE = 0xffffffff /* Last Entry */

static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)

2.20.1

From: yuchenlin <npes87184@gmail.com>

There is a possible hang in original binary search implementation. That is

if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.

The chunk1 will be still 4, and so on.

Signed-off-by: yuchenlin <npes87184@gmail.com>

Message-id: 20190103114700.9686-2-npes87184@gmail.com

block/dmg.c | 10 +++++++---

1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c

--- a/block/dmg.c

+++ b/block/dmg.c

@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)

{

/* binary search */

uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;

- while (chunk1 != chunk2) {

+ while (chunk1 <= chunk2) {

chunk3 = (chunk1 + chunk2) / 2;

if (s->sectors[chunk3] > sector_num) {

- chunk2 = chunk3;

+ if (chunk3 == 0) {

+ goto err;

+ }

+ chunk2 = chunk3 - 1;

} else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {

return chunk3;

} else {

- chunk1 = chunk3;

+ chunk1 = chunk3 + 1;

}

}

+err:

return s->n_chunks; /* error */

2.20.1