diff mbox

[v9] Support vhd type VHD_DIFFERENCING

Message ID 1424007349-3365-1-git-send-email-gongxiaodong1@huawei.com
State New
Headers show

Commit Message

=?ISO-8859-1?B?MjFH?= Feb. 15, 2015, 1:35 p.m. UTC
Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu
can't read snapshot volume of vhd, and can't support other storage
features of vhd file.

This patch add read parent information in function "vpc_open", read
bitmap in "vpc_read", and change bitmap in "vpc_write".

Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com>
Reviewed-by: Ding xiao <ssdxiao@163.com>
---
Changes since v8
- use backing_format to avoid being probed to format of raw

Changes since v7:
- use iconv to decode UTF-16LE(w2u) and UTF-8(macx) to ASCII
  (Stefan Hajnoczi)
- change the return value of vpc_write back to 0 (Stefan Hajnoczi)

Changes since v6:
- remove include of iconv.h (Stefan Hajnoczi)
- make sure data_length < length of backing_file (Stefan Hajnoczi)
- change big-ending of platform to cpu order (Stefan Hajnoczi)

Changes since v5:
- Change malloc to g_malloc. (Gonglei)(Stefan Hajnoczi)
- Fix the bug of free(null). (Gonglei)(Stefan Hajnoczi)

Changes since v4:
- Parse the batmap only when the version of VHD > 1.2. (Lucian Petrut)
- Add support to parent location of W2RU. (Lucian Petrut) (Philipp Hahn)

Changes since v3:
- Remove the PARENT_MAX_LOC.

Changes since v2:
- Change MACX to PLATFAORM_MACX. (Kevin Wolf)
- Return with EINVAL to parent location is W2RU and W2KU. (Kevin Wolf)
- Change -1 == ret to a natrual order of ret == -1. (Kevin Wolf)
- Get rid of the get_sector_offset_diff, get_sector_offset
  supports VHD_DIFF. (Kevin Wolf)
- Return code of get_sector_offset is set to, -1 for error,
  -2 for not allocate, -3 for in parent. (Kevin Wolf)
- Fix un init ret of vpc_write, when nb_sector == 0. (Kevin Wolf)
- Change if (diff == ture) to if (diff) and so on. (Kevin Wolf)
- Add PARENT_MAX_LOC to more understand. (Kevin Wolf)
- Restore the boundary check to write on dynamic type in
  get_sector_offset. (Kevin Wolf)

Changes since v1:
- Add Boundary check to any input. (Stefan Hajnoczi)
- Clean the code no used after in vpc_open. (Stefan Hajnoczi)
- Change bdrv_co_readv() to bdrv_preadv in vpc_read. (Stefan Hajnoczi)
- Added some code to make it easy to understand. (Stefan Hajnoczi)
---
 block/vpc.c | 560 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 494 insertions(+), 66 deletions(-)

Comments

Stefan Hajnoczi Feb. 25, 2015, 2:58 p.m. UTC | #1
On Sun, Feb 15, 2015 at 09:35:49PM +0800, Xiaodong Gong wrote:
> Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu
> can't read snapshot volume of vhd, and can't support other storage
> features of vhd file.
> 
> This patch add read parent information in function "vpc_open", read
> bitmap in "vpc_read", and change bitmap in "vpc_write".
> 
> Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com>
> Reviewed-by: Ding xiao <ssdxiao@163.com>
> ---
> Changes since v8
> - use backing_format to avoid being probed to format of raw
> 
> Changes since v7:
> - use iconv to decode UTF-16LE(w2u) and UTF-8(macx) to ASCII
>   (Stefan Hajnoczi)

I suggested glib's character set conversion functions, not iconv.

glib abstracts the dependency character set conversion so it will work
across platforms.  That way we don't have to add iconv library detection
to ./configure.  Please use glib since QEMU already depends on it.

https://developer.gnome.org/glib/stable/glib-Character-Set-Conversion.html

>  #define HEADER_SIZE 512
> +#define DYNAMIC_HEADER_SIZE 1024
> +#define PARENT_LOCATOR_NUM 8
> +#define TBBATMAP_HEAD_SIZE 28
> +
> +#define MACX_PREFIX_LEN 7 /* file:// */
> +
> +#define PLATFORM_MACX 0x5863614d /* big endian */

This comment doesn't make sense.  The constant is just a C integer
literal, it doesn't have endianness.

> +static int vpc_decode_parent_loc(uint32_t platform,
> +                                 BlockDriverState *bs,
> +                                 int data_length)
> +{
> +    int ret;
> +
> +    switch (platform) {
> +    case PLATFORM_MACX:
> +        ret = vpc_decode_maxc_loc(bs, data_length);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +        break;
> +
> +    case PLATFORM_W2RU:
> +        /* fall through! */
> +    case PLATFORM_W2KU:
> +        ret = vpc_decode_w2u_loc(bs, data_length);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +        break;
> +
> +    default:
> +        return 0;

This should fail.  There are unimplemented platform codes.  We should
not attempt to open the file any further.

In the PLATFORM_NONE (0x0) case it may be cleanest to
vpc_read_backing_loc()'s for loop to skip empty platform locators
instead of trying to read 0 bytes and then calling
vpc_decode_parent_loc() with no data.

> +    }
> +
> +    return 0;
> +}
> +
> +static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header,
> +                                BlockDriverState *bs,
> +                                Error **errp)
> +{
> +    BDRVVPCState *s = bs->opaque;
> +    int64_t data_offset = 0;
> +    int data_length = 0;
> +    uint32_t platform;
> +    bool done = false;
> +    int parent_locator_offset = 0;
> +    int i;
> +    int ret = 0;
> +
> +    for (i = 0; i < PARENT_LOCATOR_NUM; i++) {
> +        /* The PLATFORM_* is big ending, and the dyndisk_header
> +         * is always big ending. So whatever this platform in cpu
> +         * is, it works. */

This comment is incorrect.  dyndisk_header is big endian but PLATFORM_*
is not big endian.  Please drop the comment.

> +        platform =
> +            dyndisk_header->parent_locator[i].platform;

Missing be32_to_cpu().

> +        data_offset =
> +            be64_to_cpu(dyndisk_header->parent_locator[i].data_offset);
> +        data_length =
> +            be32_to_cpu(dyndisk_header->parent_locator[i].data_length);
> +
> +        /* Extend the location offset */
> +        if (parent_locator_offset < data_offset) {
> +            parent_locator_offset = data_offset;

Why is parent_locator_offset and this function's return value int?

data_offset is uint64_t and it seems like values could get truncated if
int is used.

> @@ -278,7 +523,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
>          s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
>  
>          ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
> -                         s->max_table_entries * 4);
> +            s->max_table_entries * 4);
>          if (ret < 0) {
>              goto fail;
>          }

Unnecessary whitespace change?

> @@ -286,6 +531,48 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
>          s->free_data_block_offset =
>              (s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
>  
> +        /* Read tdbatmap header by offset */
> +        if (be32_to_cpu(footer->version) >= VHD_VERSION(1, 2)) {
> +            ret = bdrv_pread(bs->file, s->free_data_block_offset,
> +                tdbatmap_header_buf, TBBATMAP_HEAD_SIZE);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +
> +            tdbatmap_header = (VHDTdBatmapHeader *) tdbatmap_header_buf;
> +            if (!strncmp(tdbatmap_header->magic, "tdbatmap", 8)) {
> +                s->free_data_block_offset =
> +                    be32_to_cpu(tdbatmap_header->batmap_size) * 512
> +                    + be64_to_cpu(tdbatmap_header->batmap_offset);
> +            }
> +        }
> +
> +        if (dyndisk_header->parent_name[0] || dyndisk_header->parent_name[1]) {
> +            int len;
> +
> +            /* Read parent location from dyn header table */
> +            ret = parent_locator_offset = vpc_read_backing_loc(dyndisk_header,
> +                bs, errp);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +
> +            /* Fix me : Set parent format to avoid probing to raw in
> +             * format probe framework */
> +            len = strlen("vpc");
> +            if (sizeof(bs->backing_format) - 1 < len) {
> +                goto fail;
> +            }
> +            pstrcpy(bs->backing_format, sizeof(bs->backing_format), "vpc");
> +            bs->backing_format[len] = '\0';

pstrcpy() always NUL-terminates so this is not necessary.

> @@ -376,7 +704,7 @@ static inline int64_t get_sector_offset(BlockDriverState *bs,
>          bdrv_pwrite_sync(bs->file, bitmap_offset, bitmap, s->bitmap_size);
>      }
>  
> -//    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
> +//  printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
>  //	sector_num, pagetable_index, pageentry_index,
>  //	bitmap_offset, block_offset);
>  

Unnecessary whitespace change?
=?ISO-8859-1?B?MjFH?= Feb. 27, 2015, 5:49 a.m. UTC | #2
2015年2月25日 22:58于 "Stefan Hajnoczi" <stefanha@gmail.com>写道:
>
> On Sun, Feb 15, 2015 at 09:35:49PM +0800, Xiaodong Gong wrote:
> > Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu
> > can't read snapshot volume of vhd, and can't support other storage
> > features of vhd file.
> >
> > This patch add read parent information in function "vpc_open", read
> > bitmap in "vpc_read", and change bitmap in "vpc_write".
> >
> > Signed-off-by: Xiaodong Gong <gongxiaodong1@huawei.com>
> > Reviewed-by: Ding xiao <ssdxiao@163.com>
> > ---
> > Changes since v8
> > - use backing_format to avoid being probed to format of raw
> >
> > Changes since v7:
> > - use iconv to decode UTF-16LE(w2u) and UTF-8(macx) to ASCII
> >   (Stefan Hajnoczi)
>
> I suggested glib's character set conversion functions, not iconv.
>
> glib abstracts the dependency character set conversion so it will work
> across platforms.  That way we don't have to add iconv library detection
> to ./configure.  Please use glib since QEMU already depends on it.
>
> https://developer.gnome.org/glib/stable/glib-Character-Set-Conversion.html
>

iconv is a buildin fuction in libc.And I greped,but no use of g_conv*.
I will exchange icovn with g_conv

> >  #define HEADER_SIZE 512
> > +#define DYNAMIC_HEADER_SIZE 1024
> > +#define PARENT_LOCATOR_NUM 8
> > +#define TBBATMAP_HEAD_SIZE 28
> > +
> > +#define MACX_PREFIX_LEN 7 /* file:// */
> > +
> > +#define PLATFORM_MACX 0x5863614d /* big endian */
>
> This comment doesn't make sense.  The constant is just a C integer
> literal, it doesn't have endianness.
>

It is the source of mistake,I will change it to value to graft

> > +static int vpc_decode_parent_loc(uint32_t platform,
> > +                                 BlockDriverState *bs,
> > +                                 int data_length)
> > +{
> > +    int ret;
> > +
> > +    switch (platform) {
> > +    case PLATFORM_MACX:
> > +        ret = vpc_decode_maxc_loc(bs, data_length);
> > +        if (ret < 0) {
> > +            return ret;
> > +        }
> > +        break;
> > +
> > +    case PLATFORM_W2RU:
> > +        /* fall through! */
> > +    case PLATFORM_W2KU:
> > +        ret = vpc_decode_w2u_loc(bs, data_length);
> > +        if (ret < 0) {
> > +            return ret;
> > +        }
> > +        break;
> > +
> > +    default:
> > +        return 0;
>
> This should fail.  There are unimplemented platform codes.  We should
> not attempt to open the file any further.
>

yes

> In the PLATFORM_NONE (0x0) case it may be cleanest to
> vpc_read_backing_loc()'s for loop to skip empty platform locators
> instead of trying to read 0 bytes and then calling
> vpc_decode_parent_loc() with no data.
>
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header,
> > +                                BlockDriverState *bs,
> > +                                Error **errp)
> > +{
> > +    BDRVVPCState *s = bs->opaque;
> > +    int64_t data_offset = 0;
> > +    int data_length = 0;
> > +    uint32_t platform;
> > +    bool done = false;
> > +    int parent_locator_offset = 0;
> > +    int i;
> > +    int ret = 0;
> > +
> > +    for (i = 0; i < PARENT_LOCATOR_NUM; i++) {
> > +        /* The PLATFORM_* is big ending, and the dyndisk_header
> > +         * is always big ending. So whatever this platform in cpu
> > +         * is, it works. */
>
> This comment is incorrect.  dyndisk_header is big endian but PLATFORM_*
> is not big endian.  Please drop the comment.
>

the same as the upper commant of “big ending”

> > +        platform =
> > +            dyndisk_header->parent_locator[i].platform;
>
> Missing be32_to_cpu().
>
> > +        data_offset =
> > +            be64_to_cpu(dyndisk_header->parent_locator[i].data_offset);
> > +        data_length =
> > +            be32_to_cpu(dyndisk_header->parent_locator[i].data_length);
> > +
> > +        /* Extend the location offset */
> > +        if (parent_locator_offset < data_offset) {
> > +            parent_locator_offset = data_offset;
>
> Why is parent_locator_offset and this function's return value int?
>
> data_offset is uint64_t and it seems like values could get truncated if
> int is used.
>

This var is a water line of offset,it is directly a uint64. Is this the
worrest code I ever did?

> > @@ -278,7 +523,7 @@ static int vpc_open(BlockDriverState *bs, QDict
*options, int flags,
> >          s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
> >
> >          ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
> > -                         s->max_table_entries * 4);
> > +            s->max_table_entries * 4);
> >          if (ret < 0) {
> >              goto fail;
> >          }
>
> Unnecessary whitespace change?
>

yes,to pass checkpatch

> > @@ -286,6 +531,48 @@ static int vpc_open(BlockDriverState *bs, QDict
*options, int flags,
> >          s->free_data_block_offset =
> >              (s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
> >
> > +        /* Read tdbatmap header by offset */
> > +        if (be32_to_cpu(footer->version) >= VHD_VERSION(1, 2)) {
> > +            ret = bdrv_pread(bs->file, s->free_data_block_offset,
> > +                tdbatmap_header_buf, TBBATMAP_HEAD_SIZE);
> > +            if (ret < 0) {
> > +                goto fail;
> > +            }
> > +
> > +            tdbatmap_header = (VHDTdBatmapHeader *)
tdbatmap_header_buf;
> > +            if (!strncmp(tdbatmap_header->magic, "tdbatmap", 8)) {
> > +                s->free_data_block_offset =
> > +                    be32_to_cpu(tdbatmap_header->batmap_size) * 512
> > +                    + be64_to_cpu(tdbatmap_header->batmap_offset);
> > +            }
> > +        }
> > +
> > +        if (dyndisk_header->parent_name[0] ||
dyndisk_header->parent_name[1]) {
> > +            int len;
> > +
> > +            /* Read parent location from dyn header table */
> > +            ret = parent_locator_offset =
vpc_read_backing_loc(dyndisk_header,
> > +                bs, errp);
> > +            if (ret < 0) {
> > +                goto fail;
> > +            }
> > +
> > +            /* Fix me : Set parent format to avoid probing to raw in
> > +             * format probe framework */
> > +            len = strlen("vpc");
> > +            if (sizeof(bs->backing_format) - 1 < len) {
> > +                goto fail;
> > +            }
> > +            pstrcpy(bs->backing_format, sizeof(bs->backing_format),
"vpc");
> > +            bs->backing_format[len] = '\0';
>
> pstrcpy() always NUL-terminates so this is not necessary.
>

yes

> > @@ -376,7 +704,7 @@ static inline int64_t
get_sector_offset(BlockDriverState *bs,
> >          bdrv_pwrite_sync(bs->file, bitmap_offset, bitmap,
s->bitmap_size);
> >      }
> >
> > -//    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %"
PRIx64 ", bloff: %" PRIx64 "\n",
> > +//  printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %"
PRIx64 ", bloff: %" PRIx64 "\n",
> >  //   sector_num, pagetable_index, pageentry_index,
> >  //   bitmap_offset, block_offset);
> >
>
> Unnecessary whitespace change?

recover it
diff mbox

Patch

diff --git a/block/vpc.c b/block/vpc.c
index 46803b1..fdbfe36 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -29,17 +29,29 @@ 
 #if defined(CONFIG_UUID)
 #include <uuid/uuid.h>
 #endif
+#include <iconv.h>
 
 /**************************************************************/
 
 #define HEADER_SIZE 512
+#define DYNAMIC_HEADER_SIZE 1024
+#define PARENT_LOCATOR_NUM 8
+#define TBBATMAP_HEAD_SIZE 28
+
+#define MACX_PREFIX_LEN 7 /* file:// */
+
+#define PLATFORM_MACX 0x5863614d /* big endian */
+#define PLATFORM_W2RU 0x75723257
+#define PLATFORM_W2KU 0x756B3257
+
+#define VHD_VERSION(major, minor)  (((major) << 16) | ((minor) & 0x0000FFFF))
 
 //#define CACHE
 
 enum vhd_type {
     VHD_FIXED           = 2,
     VHD_DYNAMIC         = 3,
-    VHD_DIFFERENCING    = 4,
+    VHD_DIFF            = 4,
 };
 
 // Seconds since Jan 1, 2000 0:00:00 (UTC)
@@ -138,6 +150,15 @@  typedef struct BDRVVPCState {
     Error *migration_blocker;
 } BDRVVPCState;
 
+typedef struct vhd_tdbatmap_header {
+    char magic[8]; /* always "tdbatmap" */
+
+    uint64_t batmap_offset;
+    uint32_t batmap_size;
+    uint32_t batmap_version;
+    uint32_t checksum;
+} QEMU_PACKED VHDTdBatmapHeader;
+
 static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 {
     uint32_t res = 0;
@@ -157,6 +178,225 @@  static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 0;
 }
 
+static int vpc_decode_maxc_loc(BlockDriverState *bs, char data_length)
+{
+    iconv_t cd;
+    char *inbuf, *outbuf, *buf;
+    size_t inbyteleft, outbyteleft, outbyte;
+    int ret;
+
+    if (!bs || !bs->backing_file || !data_length) {
+        return -1;
+    }
+
+    cd = iconv_open("ASCII", "UTF8");
+    if (cd == (iconv_t) -1) {
+        return -1;
+    }
+
+    inbuf = bs->backing_file;
+    outbuf =  buf = (char *) g_malloc(data_length + 1);
+    if (!outbuf) {
+        ret = -1;
+        goto fail2;
+    }
+    inbyteleft = outbyteleft = data_length;
+
+    ret = iconv(cd, &inbuf, &inbyteleft, &outbuf, &outbyteleft);
+    if (ret == (size_t) -1 || inbyteleft) {
+        ret = -1;
+        goto fail1;
+    }
+    outbyte = data_length - outbyteleft;
+    if (outbyte > sizeof(bs->backing_file) - 1) {
+        ret = -1;
+        goto fail1;
+    }
+    if (outbyte < MACX_PREFIX_LEN) {
+        ret = -1;
+        goto fail1;
+    }
+    buf[outbyte] = '\0';
+
+    strncpy(bs->backing_file, buf + MACX_PREFIX_LEN,
+        outbyte - MACX_PREFIX_LEN);
+    bs->backing_file[outbyte - MACX_PREFIX_LEN] = '\0';
+
+fail1:
+    g_free(buf);
+    buf = NULL;
+
+fail2:
+    ret = iconv_close(cd);
+    if (ret == -1) {
+        return -1;
+    }
+
+    return 0;
+}
+
+static int vpc_decode_w2u_loc(BlockDriverState *bs, char data_length)
+{
+    iconv_t cd;
+    char *buf, *inbuf, *outbuf;
+    size_t inbyteleft, outbyteleft, outbyte;
+    char *ptr;
+    char len = 0;
+    int ret;
+
+    if (!bs || !bs->backing_file || !data_length) {
+        return -1;
+    }
+
+    cd = iconv_open("ASCII", "UTF-16LE");
+    if (cd == (iconv_t) -1) {
+        return -1;
+    }
+
+    inbuf = bs->backing_file;
+    outbuf = buf = ptr = (char *) g_malloc(data_length + 1);
+    if (!buf) {
+        ret = -1;
+        goto fail2;
+    }
+    inbyteleft = outbyteleft = data_length;
+
+    ret = iconv(cd, &inbuf, &inbyteleft, &outbuf, &outbyteleft);
+    if (ret == (size_t) -1 || inbyteleft) {
+        ret = -1;
+        goto fail1;
+    }
+    outbyte = data_length - outbyteleft;
+    if (outbyte > sizeof(bs->backing_file) - 1) {
+        ret = -1;
+        goto fail1;
+    }
+    buf[outbyte] = '\0';
+
+    while (ptr != outbuf) {
+        if (*ptr == '\\') {
+            *ptr = '/';
+        }
+        ptr++;
+    }
+    ptr = strstr(buf, ":");
+    if (ptr) {
+        *ptr = '.';
+        len = ptr - buf;
+    }
+    strncpy(bs->backing_file, buf + len, outbyte - len);
+    bs->backing_file[outbyte - len] = '\0';
+
+fail1:
+    g_free(buf);
+    buf = NULL;
+
+fail2:
+    ret = iconv_close(cd);
+    if (ret == -1) {
+        return -1;
+    }
+
+    return 0;
+}
+
+static int vpc_decode_parent_loc(uint32_t platform,
+                                 BlockDriverState *bs,
+                                 int data_length)
+{
+    int ret;
+
+    switch (platform) {
+    case PLATFORM_MACX:
+        ret = vpc_decode_maxc_loc(bs, data_length);
+        if (ret < 0) {
+            return ret;
+        }
+        break;
+
+    case PLATFORM_W2RU:
+        /* fall through! */
+    case PLATFORM_W2KU:
+        ret = vpc_decode_w2u_loc(bs, data_length);
+        if (ret < 0) {
+            return ret;
+        }
+        break;
+
+    default:
+        return 0;
+    }
+
+    return 0;
+}
+
+static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header,
+                                BlockDriverState *bs,
+                                Error **errp)
+{
+    BDRVVPCState *s = bs->opaque;
+    int64_t data_offset = 0;
+    int data_length = 0;
+    uint32_t platform;
+    bool done = false;
+    int parent_locator_offset = 0;
+    int i;
+    int ret = 0;
+
+    for (i = 0; i < PARENT_LOCATOR_NUM; i++) {
+        /* The PLATFORM_* is big ending, and the dyndisk_header
+         * is always big ending. So whatever this platform in cpu
+         * is, it works. */
+        platform =
+            dyndisk_header->parent_locator[i].platform;
+        data_offset =
+            be64_to_cpu(dyndisk_header->parent_locator[i].data_offset);
+        data_length =
+            be32_to_cpu(dyndisk_header->parent_locator[i].data_length);
+
+        /* Extend the location offset */
+        if (parent_locator_offset < data_offset) {
+            parent_locator_offset = data_offset;
+        }
+
+        if (done) {
+            continue;
+        }
+
+        /* Read location of backing file */
+        if (data_offset > s->max_table_entries * s->block_size) {
+            return -1;
+        }
+        if (data_length > sizeof(bs->backing_file) - 1) {
+            return -1;
+        }
+        ret = bdrv_pread(bs->file, data_offset, bs->backing_file,
+            data_length);
+        if (ret < 0) {
+            return ret;
+        }
+
+        bs->backing_file[data_length] = '\0';
+
+        /* Decode the parent location */
+        ret = vpc_decode_parent_loc(platform, bs, data_length);
+        if (ret < 0) {
+            return ret;
+        }
+
+        /* Right parent location is got */
+        if (bs->backing_file[0] != '\0') {
+            done = true;
+        }
+    }
+
+    if (!done) {
+        return -1;
+    }
+
+    return parent_locator_offset;
+}
+
 static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
@@ -164,11 +404,14 @@  static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     int i;
     VHDFooter *footer;
     VHDDynDiskHeader *dyndisk_header;
-    uint8_t buf[HEADER_SIZE];
+    uint8_t buf[DYNAMIC_HEADER_SIZE];
+    uint8_t tdbatmap_header_buf[TBBATMAP_HEAD_SIZE];
     uint32_t checksum;
     uint64_t computed_size;
-    int disk_type = VHD_DYNAMIC;
+    uint32_t disk_type;
     int ret;
+    VHDTdBatmapHeader *tdbatmap_header;
+    int parent_locator_offset = 0;
 
     ret = bdrv_pread(bs->file, 0, s->footer_buf, HEADER_SIZE);
     if (ret < 0) {
@@ -176,6 +419,8 @@  static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     footer = (VHDFooter *) s->footer_buf;
+    disk_type = be32_to_cpu(footer->type);
+
     if (strncmp(footer->creator, "conectix", 8)) {
         int64_t offset = bdrv_getlength(bs->file);
         if (offset < 0) {
@@ -230,9 +475,9 @@  static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    if (disk_type == VHD_DYNAMIC) {
+    if (disk_type == VHD_DYNAMIC || disk_type == VHD_DIFF) {
         ret = bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
-                         HEADER_SIZE);
+                         DYNAMIC_HEADER_SIZE);
         if (ret < 0) {
             goto fail;
         }
@@ -278,7 +523,7 @@  static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
         s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
 
         ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
-                         s->max_table_entries * 4);
+            s->max_table_entries * 4);
         if (ret < 0) {
             goto fail;
         }
@@ -286,6 +531,48 @@  static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
         s->free_data_block_offset =
             (s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
 
+        /* Read tdbatmap header by offset */
+        if (be32_to_cpu(footer->version) >= VHD_VERSION(1, 2)) {
+            ret = bdrv_pread(bs->file, s->free_data_block_offset,
+                tdbatmap_header_buf, TBBATMAP_HEAD_SIZE);
+            if (ret < 0) {
+                goto fail;
+            }
+
+            tdbatmap_header = (VHDTdBatmapHeader *) tdbatmap_header_buf;
+            if (!strncmp(tdbatmap_header->magic, "tdbatmap", 8)) {
+                s->free_data_block_offset =
+                    be32_to_cpu(tdbatmap_header->batmap_size) * 512
+                    + be64_to_cpu(tdbatmap_header->batmap_offset);
+            }
+        }
+
+        if (dyndisk_header->parent_name[0] || dyndisk_header->parent_name[1]) {
+            int len;
+
+            /* Read parent location from dyn header table */
+            ret = parent_locator_offset = vpc_read_backing_loc(dyndisk_header,
+                bs, errp);
+            if (ret < 0) {
+                goto fail;
+            }
+
+            /* Fix me : Set parent format to avoid probing to raw in
+             * format probe framework */
+            len = strlen("vpc");
+            if (sizeof(bs->backing_format) - 1 < len) {
+                goto fail;
+            }
+            pstrcpy(bs->backing_format, sizeof(bs->backing_format), "vpc");
+            bs->backing_format[len] = '\0';
+        }
+
+        if (s->free_data_block_offset < parent_locator_offset
+            + BDRV_SECTOR_SIZE) {
+            s->free_data_block_offset = parent_locator_offset
+                + BDRV_SECTOR_SIZE;
+        }
+
         for (i = 0; i < s->max_table_entries; i++) {
             be32_to_cpus(&s->pagetable[i]);
             if (s->pagetable[i] != 0xFFFFFFFF) {
@@ -340,35 +627,76 @@  static int vpc_reopen_prepare(BDRVReopenState *state,
 }
 
 /*
- * Returns the absolute byte offset of the given sector in the image file.
- * If the sector is not allocated, -1 is returned instead.
+ * Returns the absolute byte offset of the given sector in the differencing
+ * image file.
  *
- * The parameter write must be 1 if the offset will be used for a write
- * operation (the block bitmaps is updated then), 0 otherwise.
+ * If error happened, -1 is returned.
+ *
+ * When write all type or read dynamic, if the sector is not allocated, -2
+ * is returned instead. If the sector is allocated in current file, the block
+ * offset is returned.
+ *
+ * When read diff. If the sector is not allocated, -2 is returned instead.
+ * If the sector is allocated in the backing file, -3 is returned. If the
+ * sector is allocated in current file, the block offset is returned.
  */
 static inline int64_t get_sector_offset(BlockDriverState *bs,
-    int64_t sector_num, int write)
+    int64_t sector_num, bool write, bool diff)
 {
     BDRVVPCState *s = bs->opaque;
-    uint64_t offset = sector_num * 512;
-    uint64_t bitmap_offset, block_offset;
+    uint64_t offset = sector_num << BDRV_SECTOR_BITS;
+    uint64_t bitmap_offset;
     uint32_t pagetable_index, pageentry_index;
+    int64_t block_offset = LONG_MIN;
+    int ret;
 
     pagetable_index = offset / s->block_size;
-    pageentry_index = (offset % s->block_size) / 512;
+    pageentry_index = (offset % s->block_size) >> BDRV_SECTOR_BITS;
 
-    if (pagetable_index >= s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
-        return -1; // not allocated
+    if (pagetable_index >= s->max_table_entries) {
+        return -2;
+    }
+    if (s->pagetable[pagetable_index] == 0xffffffff) {
+        if (!write && diff) {
+            return -3; /* parent allocated */
+        } else {
+            return -2; /* not allocated */
+        }
+    }
 
-    bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
-    block_offset = bitmap_offset + s->bitmap_size + (512 * pageentry_index);
+    bitmap_offset = (uint64_t) s->pagetable[pagetable_index]
+        << BDRV_SECTOR_BITS;
+
+    if (!diff || write) {
+        block_offset = bitmap_offset + s->bitmap_size
+            + (pageentry_index << BDRV_SECTOR_BITS);
+    } else {
+        uint32_t bitmap_index, bitmapentry_index;
+        uint8_t bitmap[s->bitmap_size];
+
+        if (bitmap_offset > s->max_table_entries * s->block_size) {
+            return -1;
+        }
+        ret = bdrv_pread(bs->file, bitmap_offset, bitmap, s->bitmap_size);
+        if (ret < 0) {
+            return -1;
+        }
 
+        bitmap_index = pageentry_index / 8;
+        bitmapentry_index = 7 - pageentry_index % 8;
+        if (bitmap[bitmap_index] & 0x1 << bitmapentry_index) {
+            block_offset = bitmap_offset + s->bitmap_size
+                + (pageentry_index << BDRV_SECTOR_BITS);
+        } else {
+            return -3;
+        }
+    }
     // We must ensure that we don't write to any sectors which are marked as
     // unused in the bitmap. We get away with setting all bits in the block
     // bitmap each time we write to a new block. This might cause Virtual PC to
     // miss sparse read optimization, but it's not a problem in terms of
     // correctness.
-    if (write && (s->last_bitmap_offset != bitmap_offset)) {
+    if (!diff && write && (s->last_bitmap_offset != bitmap_offset)) {
         uint8_t bitmap[s->bitmap_size];
 
         s->last_bitmap_offset = bitmap_offset;
@@ -376,7 +704,7 @@  static inline int64_t get_sector_offset(BlockDriverState *bs,
         bdrv_pwrite_sync(bs->file, bitmap_offset, bitmap, s->bitmap_size);
     }
 
-//    printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
+//  printf("sector: %" PRIx64 ", index: %x, offset: %x, bioff: %" PRIx64 ", bloff: %" PRIx64 "\n",
 //	sector_num, pagetable_index, pageentry_index,
 //	bitmap_offset, block_offset);
 
@@ -437,7 +765,8 @@  static int rewrite_footer(BlockDriverState* bs)
  *
  * Returns the sectors' offset in the image file on success and < 0 on error
  */
-static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
+static int64_t alloc_block(BlockDriverState *bs, int64_t sector_num,
+    bool diff)
 {
     BDRVVPCState *s = bs->opaque;
     int64_t bat_offset;
@@ -457,7 +786,11 @@  static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
     s->pagetable[index] = s->free_data_block_offset / 512;
 
     // Initialize the block's bitmap
-    memset(bitmap, 0xff, s->bitmap_size);
+    if (diff) {
+        memset(bitmap, 0x0, s->bitmap_size);
+    } else {
+        memset(bitmap, 0xff, s->bitmap_size);
+    }
     ret = bdrv_pwrite_sync(bs->file, s->free_data_block_offset, bitmap,
         s->bitmap_size);
     if (ret < 0) {
@@ -477,7 +810,7 @@  static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
     if (ret < 0)
         goto fail;
 
-    return get_sector_offset(bs, sector_num, 0);
+    return get_sector_offset(bs, sector_num, false, diff);
 
 fail:
     s->free_data_block_offset -= (s->block_size + s->bitmap_size);
@@ -501,37 +834,71 @@  static int vpc_read(BlockDriverState *bs, int64_t sector_num,
                     uint8_t *buf, int nb_sectors)
 {
     BDRVVPCState *s = bs->opaque;
-    int ret;
-    int64_t offset;
-    int64_t sectors, sectors_per_block;
     VHDFooter *footer = (VHDFooter *) s->footer_buf;
+    int64_t sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
+    int64_t offset, sectors;
+    int ret;
 
-    if (be32_to_cpu(footer->type) == VHD_FIXED) {
+    switch (be32_to_cpu(footer->type)) {
+    case VHD_FIXED:
         return bdrv_read(bs->file, sector_num, buf, nb_sectors);
-    }
-    while (nb_sectors > 0) {
-        offset = get_sector_offset(bs, sector_num, 0);
 
-        sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
-        sectors = sectors_per_block - (sector_num % sectors_per_block);
-        if (sectors > nb_sectors) {
-            sectors = nb_sectors;
+    case VHD_DYNAMIC:
+        while (nb_sectors > 0) {
+            sectors = sectors_per_block - (sector_num % sectors_per_block);
+            if (sectors > nb_sectors) {
+                sectors = nb_sectors;
+            }
+
+            offset = get_sector_offset(bs, sector_num, false, false);
+            if (offset == -1) {
+                return -1;
+            } else if (offset == -2) {
+                memset(buf, 0, sectors * BDRV_SECTOR_SIZE);
+            } else {
+                ret = bdrv_pread(bs->file, offset, buf,
+                    sectors * BDRV_SECTOR_SIZE);
+                if (ret != sectors * BDRV_SECTOR_SIZE) {
+                    return -1;
+                }
+            }
+
+            nb_sectors -= sectors;
+            sector_num += sectors;
+            buf += sectors * BDRV_SECTOR_SIZE;
         }
+        break;
 
-        if (offset == -1) {
-            memset(buf, 0, sectors * BDRV_SECTOR_SIZE);
-        } else {
-            ret = bdrv_pread(bs->file, offset, buf,
-                sectors * BDRV_SECTOR_SIZE);
-            if (ret != sectors * BDRV_SECTOR_SIZE) {
+    case VHD_DIFF:
+        while (nb_sectors > 0) {
+            offset = get_sector_offset(bs, sector_num, false, true);
+            if (offset == -1) {
                 return -1;
+            } else if (offset == -2) {
+                memset(buf, 0, BDRV_SECTOR_SIZE);
+            } else if (offset == -3) {
+                ret = bdrv_pread(bs->backing_hd, sector_num << BDRV_SECTOR_BITS
+                    , buf, BDRV_SECTOR_SIZE);
+                if (ret < 0) {
+                    return -1;
+                }
+            } else {
+                ret = bdrv_pread(bs->file, offset, buf, BDRV_SECTOR_SIZE);
+                if (ret != BDRV_SECTOR_SIZE) {
+                    return -1;
+                }
             }
+
+            nb_sectors--;
+            sector_num++;
+            buf += BDRV_SECTOR_SIZE;
         }
+        break;
 
-        nb_sectors -= sectors;
-        sector_num += sectors;
-        buf += sectors * BDRV_SECTOR_SIZE;
+    default:
+        return -1;
     }
+
     return 0;
 }
 
@@ -546,41 +913,101 @@  static coroutine_fn int vpc_co_read(BlockDriverState *bs, int64_t sector_num,
     return ret;
 }
 
+static inline int64_t write_bitmap(BlockDriverState *bs, int64_t sector_num,
+    int64_t sectors)
+{
+    BDRVVPCState *s = bs->opaque;
+    uint64_t offset = sector_num << BDRV_SECTOR_BITS;
+    uint64_t bitmap_offset;
+    uint32_t pagetable_index, pageentry_index;
+    uint8_t bitmap[s->bitmap_size];
+    uint32_t bitmap_index, bitmapbit_index;
+    int i;
+    int ret;
+
+    pagetable_index = offset / s->block_size;
+    pageentry_index = (offset % s->block_size) / 512;
+    bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
+
+    if (bitmap_offset > s->max_table_entries * s->block_size) {
+        return -1;
+    }
+    ret = bdrv_pread(bs->file, bitmap_offset, bitmap, s->bitmap_size);
+    if (ret < 0) {
+        return -1;
+    }
+
+    for (i = 0; i < sectors; i++) {
+        bitmap_index = pageentry_index / 8;
+        bitmapbit_index = 7 - pageentry_index % 8;
+        bitmap[bitmap_index] |= (0x1 << bitmapbit_index);
+        pageentry_index++;
+    }
+    ret = bdrv_pwrite(bs->file, bitmap_offset, bitmap, s->bitmap_size);
+    if (ret < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
 static int vpc_write(BlockDriverState *bs, int64_t sector_num,
     const uint8_t *buf, int nb_sectors)
 {
     BDRVVPCState *s = bs->opaque;
-    int64_t offset;
-    int64_t sectors, sectors_per_block;
-    int ret;
-    VHDFooter *footer =  (VHDFooter *) s->footer_buf;
+    VHDFooter *footer = (VHDFooter *) s->footer_buf;
+    int64_t sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
+    int64_t offset, sectors;
+    bool diff = true;
+    int ret = 0;
 
-    if (be32_to_cpu(footer->type) == VHD_FIXED) {
+    switch (be32_to_cpu(footer->type)) {
+    case VHD_FIXED:
         return bdrv_write(bs->file, sector_num, buf, nb_sectors);
-    }
-    while (nb_sectors > 0) {
-        offset = get_sector_offset(bs, sector_num, 1);
 
-        sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
-        sectors = sectors_per_block - (sector_num % sectors_per_block);
-        if (sectors > nb_sectors) {
-            sectors = nb_sectors;
-        }
+    case VHD_DYNAMIC:
+        diff = false;
+        /* fall-through ! */
+
+    case VHD_DIFF:
+        while (nb_sectors > 0) {
+            sectors = sectors_per_block - (sector_num % sectors_per_block);
+            if (sectors > nb_sectors) {
+                sectors = nb_sectors;
+            }
 
-        if (offset == -1) {
-            offset = alloc_block(bs, sector_num);
-            if (offset < 0)
+            offset = get_sector_offset(bs, sector_num, true, diff);
+            if (offset == -1) {
                 return -1;
-        }
+            } else if (offset == -2) {
+                offset = alloc_block(bs, sector_num, diff);
+                if (offset < 0) {
+                    return -1;
+                }
+            }
 
-        ret = bdrv_pwrite(bs->file, offset, buf, sectors * BDRV_SECTOR_SIZE);
-        if (ret != sectors * BDRV_SECTOR_SIZE) {
-            return -1;
+            ret = bdrv_pwrite(bs->file, offset, buf,
+                sectors * BDRV_SECTOR_SIZE);
+            if (ret != sectors * BDRV_SECTOR_SIZE) {
+                return -1;
+            }
+
+            if (diff) {
+                ret = write_bitmap(bs, sector_num, sectors);
+                if (ret < 0) {
+                    return -1;
+                }
+            }
+
+            nb_sectors -= sectors;
+            sector_num += sectors;
+            buf += sectors * BDRV_SECTOR_SIZE;
         }
 
-        nb_sectors -= sectors;
-        sector_num += sectors;
-        buf += sectors * BDRV_SECTOR_SIZE;
+        break;
+
+    default:
+        return -1;
     }
 
     return 0;
@@ -906,6 +1333,7 @@  static BlockDriver bdrv_vpc = {
     .bdrv_close             = vpc_close,
     .bdrv_reopen_prepare    = vpc_reopen_prepare,
     .bdrv_create            = vpc_create,
+    .supports_backing       = true,
 
     .bdrv_read              = vpc_co_read,
     .bdrv_write             = vpc_co_write,