Patchwork [V12,5/6] add-cow file format

login
register
mail settings
Submitter Robert Wang
Date Aug. 10, 2012, 3:39 p.m.
Message ID <1344613185-12308-6-git-send-email-wdongxu@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/176515/
State New
Headers show

Comments

Robert Wang - Aug. 10, 2012, 3:39 p.m.
add-cow file format core code. It use block-cache.c as cache code.

Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
 block/Makefile.objs |    1 +
 block/add-cow.c     |  613 +++++++++++++++++++++++++++++++++++++++++++++++++++
 block/add-cow.h     |   85 +++++++
 block_int.h         |    2 +
 4 files changed, 701 insertions(+), 0 deletions(-)
 create mode 100644 block/add-cow.c
 create mode 100644 block/add-cow.h
Michael Roth - Sept. 6, 2012, 8:19 p.m.
On Fri, Aug 10, 2012 at 11:39:44PM +0800, Dong Xu Wang wrote:
> add-cow file format core code. It use block-cache.c as cache code.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> ---
>  block/Makefile.objs |    1 +
>  block/add-cow.c     |  613 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  block/add-cow.h     |   85 +++++++
>  block_int.h         |    2 +
>  4 files changed, 701 insertions(+), 0 deletions(-)
>  create mode 100644 block/add-cow.c
>  create mode 100644 block/add-cow.h
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 23bdfc8..7ed5051 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -2,6 +2,7 @@ block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat
>  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>  block-obj-y += qed-check.o
> +block-obj-y += add-cow.o
>  block-obj-y += block-cache.o
>  block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>  block-obj-y += stream.o
> diff --git a/block/add-cow.c b/block/add-cow.c
> new file mode 100644
> index 0000000..d4711d5
> --- /dev/null
> +++ b/block/add-cow.c
> @@ -0,0 +1,613 @@
> +/*
> + * QEMU ADD-COW Disk Format
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "block_int.h"
> +#include "module.h"
> +#include "add-cow.h"
> +
> +static void add_cow_header_le_to_cpu(const AddCowHeader *le, AddCowHeader *cpu)
> +{
> +    cpu->magic                      = le64_to_cpu(le->magic);
> +    cpu->version                    = le32_to_cpu(le->version);
> +
> +    cpu->backing_filename_offset    = le32_to_cpu(le->backing_filename_offset);
> +    cpu->backing_filename_size      = le32_to_cpu(le->backing_filename_size);
> +
> +    cpu->image_filename_offset      = le32_to_cpu(le->image_filename_offset);
> +    cpu->image_filename_size        = le32_to_cpu(le->image_filename_size);
> +
> +    cpu->features                   = le64_to_cpu(le->features);
> +    cpu->optional_features          = le64_to_cpu(le->optional_features);
> +    cpu->header_pages_size          = le32_to_cpu(le->header_pages_size);
> +}
> +
> +static void add_cow_header_cpu_to_le(const AddCowHeader *cpu, AddCowHeader *le)
> +{
> +    le->magic                       = cpu_to_le64(cpu->magic);
> +    le->version                     = cpu_to_le32(cpu->version);
> +
> +    le->backing_filename_offset     = cpu_to_le32(cpu->backing_filename_offset);
> +    le->backing_filename_size       = cpu_to_le32(cpu->backing_filename_size);
> +
> +    le->image_filename_offset       = cpu_to_le32(cpu->image_filename_offset);
> +    le->image_filename_size         = cpu_to_le32(cpu->image_filename_size);
> +
> +    le->features                    = cpu_to_le64(cpu->features);
> +    le->optional_features           = cpu_to_le64(cpu->optional_features);
> +    le->header_pages_size           = cpu_to_le32(cpu->header_pages_size);
> +}
> +
> +static int add_cow_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    const AddCowHeader *header = (const AddCowHeader *)buf;
> +
> +    if (le64_to_cpu(header->magic) == ADD_COW_MAGIC &&
> +        le32_to_cpu(header->version) == ADD_COW_VERSION) {
> +        return 100;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +static int add_cow_create(const char *filename, QEMUOptionParameter *options)
> +{
> +    AddCowHeader header = {
> +        .magic = ADD_COW_MAGIC,
> +        .version = ADD_COW_VERSION,
> +        .features = 0,
> +        .optional_features = 0,
> +        .header_pages_size = ADD_COW_DEFAULT_PAGE_SIZE,
> +    };
> +    AddCowHeader le_header;
> +    int64_t image_len = 0;
> +    const char *backing_filename = NULL;
> +    const char *backing_fmt = NULL;
> +    const char *image_filename = NULL;
> +    const char *image_format = NULL;
> +    BlockDriverState *bs, *image_bs = NULL, *backing_bs = NULL;
> +    BlockDriver *drv = bdrv_find_format("add-cow");
> +    BDRVAddCowState s;
> +    int ret;
> +
> +    while (options && options->name) {
> +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> +            image_len = options->value.n;
> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
> +            backing_filename = options->value.s;
> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
> +            backing_fmt = options->value.s;
> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FILE)) {
> +            image_filename = options->value.s;
> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FORMAT)) {
> +            image_format = options->value.s;
> +        }
> +        options++;
> +    }
> +
> +    if (backing_filename) {
> +        header.backing_filename_offset = sizeof(header)
> +            + sizeof(s.backing_file_format) + sizeof(s.image_file_format);
> +        header.backing_filename_size = strlen(backing_filename);
> +
> +        if (!backing_fmt) {
> +            backing_bs = bdrv_new("image");
> +            ret = bdrv_open(backing_bs, backing_filename, BDRV_O_RDWR
> +                    | BDRV_O_CACHE_WB, NULL);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +            backing_fmt = bdrv_get_format_name(backing_bs);
> +            bdrv_delete(backing_bs);
> +        }
> +    } else {
> +        header.features |= ADD_COW_F_All_ALLOCATED;
> +    }
> +
> +    if (image_filename) {
> +        header.image_filename_offset =
> +            sizeof(header) + sizeof(s.backing_file_format)
> +                + sizeof(s.image_file_format) + header.backing_filename_size;
> +        header.image_filename_size = strlen(image_filename);
> +    } else {
> +        error_report("Error: image_file should be given.");
> +        return -EINVAL;
> +    }
> +
> +    if (backing_filename && !strcmp(backing_filename, image_filename)) {
> +        error_report("Error: Trying to create an image with the "
> +                     "same backing file name as the image file name");
> +        return -EINVAL;
> +    }
> +
> +    if (!strcmp(filename, image_filename)) {
> +        error_report("Error: Trying to create an image with the "
> +                     "same filename as the image file name");
> +        return -EINVAL;
> +    }
> +
> +    if (header.image_filename_offset + header.image_filename_size
> +            > ADD_COW_PAGE_SIZE * ADD_COW_DEFAULT_PAGE_SIZE) {
> +        error_report("image_file name or backing_file name too long.");
> +        return -ENOSPC;
> +    }
> +
> +    ret = bdrv_file_open(&image_bs, image_filename, BDRV_O_RDWR);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    bdrv_delete(image_bs);
> +
> +    ret = bdrv_create_file(filename, NULL);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    add_cow_header_cpu_to_le(&header, &le_header);
> +    ret = bdrv_pwrite(bs, 0, &le_header, sizeof(le_header));
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_pwrite(bs, sizeof(le_header), backing_fmt ? backing_fmt : "",
> +        backing_fmt ? strlen(backing_fmt) : 0);
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_pwrite(bs, sizeof(le_header) + sizeof(s.backing_file_format),
> +        image_format ? image_format : "raw",
> +        image_format ? strlen(image_format) : sizeof("raw"));
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    if (backing_filename) {
> +        ret = bdrv_pwrite(bs, header.backing_filename_offset,
> +            backing_filename, header.backing_filename_size);
> +        if (ret < 0) {
> +            bdrv_delete(bs);
> +            return ret;
> +        }
> +    }
> +
> +    ret = bdrv_pwrite(bs, header.image_filename_offset,
> +        image_filename, header.image_filename_size);
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_open(bs, filename, BDRV_O_RDWR | BDRV_O_NO_FLUSH, drv);
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_truncate(bs, image_len);
> +    bdrv_delete(bs);
> +    return ret;
> +}
> +
> +static int add_cow_open(BlockDriverState *bs, int flags)
> +{
> +    char                image_filename[ADD_COW_FILE_LEN];
> +    char                tmp_name[ADD_COW_FILE_LEN];
> +    BlockDriver         *image_drv = NULL;
> +    int                 ret;
> +    int                 sector_per_byte;
> +    BDRVAddCowState     *s = bs->opaque;
> +    AddCowHeader        le_header;
> +
> +    ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
> +    if (ret != sizeof(s->header)) {
> +        goto fail;
> +    }
> +
> +    add_cow_header_le_to_cpu(&le_header, &s->header);
> +
> +    if (le64_to_cpu(s->header.magic) != ADD_COW_MAGIC) {
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    if (s->header.version != ADD_COW_VERSION) {
> +        char version[64];
> +        snprintf(version, sizeof(version), "ADD-COW version %d",
> +            s->header.version);
> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
> +            bs->device_name, "add-cow", version);
> +        ret = -ENOTSUP;
> +        goto fail;
> +    }
> +
> +    if (s->header.features & ~ADD_COW_FEATURE_MASK) {
> +        char buf[64];
> +        snprintf(buf, sizeof(buf), "%" PRIx64,
> +            s->header.features & ~ADD_COW_FEATURE_MASK);
> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
> +            bs->device_name, "add-cow", buf);
> +        return -ENOTSUP;
> +    }
> +
> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
> +        ret = bdrv_read_string(bs->file, sizeof(s->header),
> +            sizeof(s->backing_file_format) - 1, s->backing_file_format,
> +            sizeof(s->backing_file_format));
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +    }
> +
> +    ret = bdrv_read_string(bs->file,
> +            sizeof(s->header) + sizeof(s->image_file_format),
> +            sizeof(s->image_file_format) - 1, s->image_file_format,
> +            sizeof(s->image_file_format));
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +
> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
> +        ret = bdrv_read_string(bs->file, s->header.backing_filename_offset,
> +                          s->header.backing_filename_size, bs->backing_file,
> +                          sizeof(bs->backing_file));
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +    }
> +
> +    ret = bdrv_read_string(bs->file, s->header.image_filename_offset,
> +                      s->header.image_filename_size, tmp_name,
> +                      sizeof(tmp_name));
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +
> +    s->image_hd = bdrv_new("");
> +    if (path_has_protocol(image_filename)) {
> +        pstrcpy(image_filename, sizeof(image_filename), tmp_name);
> +    } else {
> +        path_combine(image_filename, sizeof(image_filename),
> +                     bs->filename, tmp_name);
> +    }
> +
> +    ret = bdrv_open(s->image_hd, image_filename, flags, image_drv);
> +    if (ret < 0) {
> +        bdrv_delete(s->image_hd);
> +        goto fail;
> +    }
> +
> +    bs->total_sectors = bdrv_getlength(s->image_hd) >> 9;
> +    s->cluster_size = ADD_COW_CLUSTER_SIZE;
> +    sector_per_byte = SECTORS_PER_CLUSTER * 8;
> +    s->bitmap_size =
> +        (bs->total_sectors + sector_per_byte - 1) / sector_per_byte;
> +    s->bitmap_cache =
> +        block_cache_create(bs, ADD_COW_CACHE_SIZE, ADD_COW_CACHE_ENTRY_SIZE);
> +
> +    qemu_co_mutex_init(&s->lock);
> +    return 0;
> +fail:
> +    if (s->bitmap_cache) {
> +        block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
> +    }
> +    return ret;
> +}
> +
> +static void add_cow_close(BlockDriverState *bs)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
> +    bdrv_delete(s->image_hd);
> +}
> +
> +static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
> +{
> +    BDRVAddCowState *s  = bs->opaque;
> +    BlockCache *c = s->bitmap_cache;
> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
> +    uint8_t *table      = NULL;
> +    uint64_t offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
> +        + (offset_in_bitmap(sector_num) & (~(c->entry_size - 1)));
> +    int ret = block_cache_get(bs, s->bitmap_cache, offset,
> +        (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
> +
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return table[cluster_num / 8 % ADD_COW_CACHE_ENTRY_SIZE]
> +        & (1 << (cluster_num % 8));
> +}
> +
> +static coroutine_fn int add_cow_is_allocated(BlockDriverState *bs,
> +        int64_t sector_num, int nb_sectors, int *num_same)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    int changed;
> +
> +    if (nb_sectors == 0) {
> +        *num_same = 0;
> +        return 0;
> +    }
> +
> +    if (s->header.features & ADD_COW_F_All_ALLOCATED) {
> +        *num_same = nb_sectors - 1;
> +        return 1;
> +    }
> +    changed = is_allocated(bs, sector_num);
> +
> +    for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
> +        if (is_allocated(bs, sector_num + *num_same) != changed) {
> +            break;
> +        }
> +    }
> +    return changed;
> +}
> +
> +static int add_cow_backing_read(BlockDriverState *bs, QEMUIOVector *qiov,
> +                  int64_t sector_num, int nb_sectors)
> +{
> +    int n1;
> +    if ((sector_num + nb_sectors) <= bs->total_sectors) {
> +        return nb_sectors;
> +    }
> +    if (sector_num >= bs->total_sectors) {
> +        n1 = 0;
> +    } else {
> +        n1 = bs->total_sectors - sector_num;
> +    }
> +
> +    qemu_iovec_memset(qiov, BDRV_SECTOR_SIZE * n1,
> +        0, BDRV_SECTOR_SIZE * (nb_sectors - n1));
> +
> +    return n1;
> +}
> +
> +static coroutine_fn int add_cow_co_readv(BlockDriverState *bs,
> +    int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
> +{
> +    BDRVAddCowState *s  = bs->opaque;
> +    int cur_nr_sectors;
> +    uint64_t bytes_done = 0;
> +    QEMUIOVector hd_qiov;
> +    int n, n1, ret = 0;
> +
> +    qemu_iovec_init(&hd_qiov, qiov->niov);
> +    qemu_co_mutex_lock(&s->lock);
> +    while (remaining_sectors != 0) {
> +        cur_nr_sectors = remaining_sectors;
> +        if (add_cow_is_allocated(bs, sector_num, cur_nr_sectors, &n)) {
> +            cur_nr_sectors = n;
> +            qemu_iovec_reset(&hd_qiov);
> +            qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
> +            qemu_co_mutex_unlock(&s->lock);
> +            ret = bdrv_co_readv(s->image_hd, sector_num, n, &hd_qiov);
> +            qemu_co_mutex_lock(&s->lock);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +        } else {
> +            cur_nr_sectors = n;
> +            if (bs->backing_hd) {
> +                qemu_iovec_reset(&hd_qiov);
> +                qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
> +                n1 = add_cow_backing_read(bs->backing_hd, &hd_qiov,
> +                    sector_num, cur_nr_sectors);
> +                if (n1 > 0) {
> +                    qemu_co_mutex_unlock(&s->lock);
> +                    ret = bdrv_co_readv(bs->backing_hd, sector_num,
> +                                        n, &hd_qiov);
> +                    qemu_co_mutex_lock(&s->lock);
> +                    if (ret < 0) {
> +                        goto fail;
> +                    }
> +                }
> +            } else {
> +                qemu_iovec_memset(&hd_qiov, 0, 0,
> +                    BDRV_SECTOR_SIZE * cur_nr_sectors);
> +            }
> +        }
> +        remaining_sectors -= cur_nr_sectors;
> +        sector_num += cur_nr_sectors;
> +        bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
> +    }
> +fail:
> +    qemu_co_mutex_unlock(&s->lock);
> +    qemu_iovec_destroy(&hd_qiov);
> +    return ret;
> +}
> +
> +static int coroutine_fn copy_sectors(BlockDriverState *bs,
> +                                     int n_start, int n_end)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    QEMUIOVector qiov;
> +    struct iovec iov;
> +    int n, ret;
> +
> +    n = n_end - n_start;
> +    if (n <= 0) {
> +        return 0;
> +    }
> +
> +    iov.iov_len = n * BDRV_SECTOR_SIZE;
> +    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
> +
> +    qemu_iovec_init_external(&qiov, &iov, 1);
> +
> +    ret = bdrv_co_readv(bs->backing_hd, n_start, n, &qiov);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +    ret = bdrv_co_writev(s->image_hd, n_start, n, &qiov);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    ret = 0;
> +out:
> +    qemu_vfree(iov.iov_base);
> +    return ret;
> +}
> +
> +static coroutine_fn int add_cow_co_writev(BlockDriverState *bs,
> +        int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    BlockCache *c = s->bitmap_cache;
> +    int ret = 0, i;
> +    QEMUIOVector hd_qiov;
> +    uint8_t *table;
> +    uint64_t offset;
> +
> +    qemu_co_mutex_lock(&s->lock);
> +    qemu_iovec_init(&hd_qiov, qiov->niov);
> +    ret = bdrv_co_writev(s->image_hd,
> +                     sector_num,
> +                     remaining_sectors, qiov);

alignment                   ^

or even at ^ if you prefer and have done in some places, just need to be
consistent about it for better readability.

> +
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
> +        /* Copy content of unmodified sectors */
> +        if (!is_cluster_head(sector_num) && !is_allocated(bs, sector_num)) {

Why do we avoid a COW when writing to the first sector of a cluster?

> +            ret = copy_sectors(bs, sector_num & ~(SECTORS_PER_CLUSTER - 1),
> +                sector_num);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +        }
> +
> +        if (!is_cluster_tail(sector_num + remaining_sectors - 1)
> +            && !is_allocated(bs, sector_num + remaining_sectors - 1)) {
> +            ret = copy_sectors(bs, sector_num + remaining_sectors,
> +                ((sector_num + remaining_sectors) | (SECTORS_PER_CLUSTER - 1)) + 1);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +        }
> +
> +        for (i = sector_num / SECTORS_PER_CLUSTER;
> +            i <= (sector_num + remaining_sectors - 1) / SECTORS_PER_CLUSTER;
> +            i++) {
> +            offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
> +                + (offset_in_bitmap(i * SECTORS_PER_CLUSTER) & (~(c->entry_size - 1)));
> +            ret = block_cache_get(bs, s->bitmap_cache, offset,
> +                (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +            if ((table[i / 8] & (1 << (i % 8))) == 0) {
> +                table[i / 8] |= (1 << (i % 8));
> +                block_cache_entry_mark_dirty(s->bitmap_cache, table);
> +            }
> +        }
> +    }
> +    ret = 0;
> +fail:
> +    qemu_co_mutex_unlock(&s->lock);
> +    qemu_iovec_destroy(&hd_qiov);
> +    return ret;
> +}
> +
> +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
> +    int ret;
> +    uint32_t bitmap_pos = s->header.header_pages_size * ADD_COW_PAGE_SIZE;
> +    int64_t bitmap_size =
> +        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
> +    bitmap_size = (bitmap_size + ADD_COW_CACHE_ENTRY_SIZE - 1)
> +        & (~(ADD_COW_CACHE_ENTRY_SIZE - 1));
> +
> +    ret = bdrv_truncate(bs->file, bitmap_pos + bitmap_size);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return 0;
> +}
> +
> +static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    int ret;
> +
> +    qemu_co_mutex_lock(&s->lock);
> +    ret = block_cache_flush(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP,
> +        ADD_COW_CACHE_ENTRY_SIZE);
> +    qemu_co_mutex_unlock(&s->lock);
> +    return ret;
> +}
> +
> +static QEMUOptionParameter add_cow_create_options[] = {
> +    {
> +        .name = BLOCK_OPT_SIZE,
> +        .type = OPT_SIZE,
> +        .help = "Virtual disk size"
> +    },
> +    {
> +        .name = BLOCK_OPT_BACKING_FILE,
> +        .type = OPT_STRING,
> +        .help = "File name of a base image"
> +    },
> +    {
> +        .name = BLOCK_OPT_BACKING_FMT,
> +        .type = OPT_STRING,
> +        .help = "Image format of the base image"
> +    },
> +    {
> +        .name = BLOCK_OPT_IMAGE_FILE,
> +        .type = OPT_STRING,
> +        .help = "File name of a image file"
> +    },
> +    {
> +        .name = BLOCK_OPT_IMAGE_FORMAT,
> +        .type = OPT_STRING,
> +        .help = "Image format of the image file"
> +    },
> +    { NULL }
> +};
> +
> +static BlockDriver bdrv_add_cow = {
> +    .format_name                = "add-cow",
> +    .instance_size              = sizeof(BDRVAddCowState),
> +    .bdrv_probe                 = add_cow_probe,
> +    .bdrv_open                  = add_cow_open,
> +    .bdrv_close                 = add_cow_close,
> +    .bdrv_create                = add_cow_create,
> +    .bdrv_co_readv              = add_cow_co_readv,
> +    .bdrv_co_writev             = add_cow_co_writev,
> +    .bdrv_truncate              = bdrv_add_cow_truncate,
> +    .bdrv_co_is_allocated       = add_cow_is_allocated,
> +
> +    .create_options             = add_cow_create_options,
> +    .bdrv_co_flush_to_os        = add_cow_co_flush,
> +};
> +
> +static void bdrv_add_cow_init(void)
> +{
> +    bdrv_register(&bdrv_add_cow);
> +}
> +
> +block_init(bdrv_add_cow_init);
> diff --git a/block/add-cow.h b/block/add-cow.h
> new file mode 100644
> index 0000000..f058376
> --- /dev/null
> +++ b/block/add-cow.h
> @@ -0,0 +1,85 @@
> +/*
> + * QEMU ADD-COW Disk Format
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef BLOCK_ADD_COW_H
> +#define BLOCK_ADD_COW_H
> +#include "block-cache.h"
> +
> +enum {
> +    ADD_COW_F_All_ALLOCATED     = 0X01,

Please use "ADD_COW_F_ALL_ALLOCATED" (all caps)

was searching your patch for how this was used and was scratching my
head when I wasn't seeing any matches :)

> +    ADD_COW_FEATURE_MASK        = ADD_COW_F_All_ALLOCATED,
> +
> +    ADD_COW_MAGIC = (((uint64_t)'A' << 56) | ((uint64_t)'D' << 48) | \
> +                    ((uint64_t)'D' << 40) | ((uint64_t)'_' << 32) | \
> +                    ((uint64_t)'C' << 24) | ((uint64_t)'O' << 16) | \
> +                    ((uint64_t)'W' << 8) | 0xFF),
> +    ADD_COW_VERSION             = 1,
> +    ADD_COW_FILE_LEN            = 1024,
> +    ADD_COW_CACHE_SIZE          = 16,
> +    ADD_COW_CACHE_ENTRY_SIZE    = 65536,
> +    ADD_COW_CLUSTER_SIZE        = 65536,
> +    SECTORS_PER_CLUSTER         = (ADD_COW_CLUSTER_SIZE / BDRV_SECTOR_SIZE),
> +    ADD_COW_PAGE_SIZE           = 4096,
> +    ADD_COW_DEFAULT_PAGE_SIZE   = 1,
> +};
> +
> +typedef struct AddCowHeader {
> +    uint64_t        magic;
> +    uint32_t        version;
> +
> +    uint32_t        backing_filename_offset;
> +    uint32_t        backing_filename_size;
> +
> +    uint32_t        image_filename_offset;
> +    uint32_t        image_filename_size;
> +
> +    uint64_t        features;
> +    uint64_t        optional_features;
> +    uint32_t        header_pages_size;
> +} QEMU_PACKED AddCowHeader;

You should avoid using packed structures for image format headers.
Instead, I would either:

a) re-order the fields so that 32/64-bit fields, respectively, fall on
32/64-bit boundaries (in your case, for instance, moving header_pages_size
above features) like qed/qcow2 do, or

b) read/write the fields individually rather than reading/writing directly
into/from the header struct.

The safest route is b). Adds a few lines of code, but you won't have to
re-work things (or worry about introducing bugs) later if you were to add,
say, a 32-bit value, and then a 64-bit value later.

> +
> +typedef struct BDRVAddCowState {
> +    BlockDriverState    *image_hd;
> +    CoMutex             lock;
> +    int                 cluster_size;
> +    BlockCache         *bitmap_cache;
> +    uint64_t            bitmap_size;
> +    AddCowHeader        header;
> +    char                backing_file_format[16];
> +    char                image_file_format[16];
> +} BDRVAddCowState;
> +
> +/* Convert sector_num to offset in bitmap */
> +static inline int64_t offset_in_bitmap(int64_t sector_num)
> +{
> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
> +    return cluster_num / 8;
> +}
> +
> +static inline bool is_cluster_head(int64_t sector_num)
> +{
> +    return sector_num % SECTORS_PER_CLUSTER == 0;
> +}
> +
> +static inline bool is_cluster_tail(int64_t sector_num)
> +{
> +    return (sector_num + 1) % SECTORS_PER_CLUSTER == 0;
> +}
> +
> +BlockCache *add_cow_cache_create(BlockDriverState *bs, int num_tables);
> +int add_cow_cache_destroy(BlockDriverState *bs, BlockCache *c);
> +void add_cow_cache_entry_mark_dirty(BlockCache *c, void *table);
> +int add_cow_cache_get(BlockDriverState *bs, BlockCache *c, uint64_t offset,
> +    void **table);
> +int add_cow_cache_flush(BlockDriverState *bs, BlockCache *c);
> +#endif
> diff --git a/block_int.h b/block_int.h
> index 6c1d9ca..67954ec 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -53,6 +53,8 @@
>  #define BLOCK_OPT_SUBFMT            "subformat"
>  #define BLOCK_OPT_COMPAT_LEVEL      "compat"
>  #define BLOCK_OPT_LAZY_REFCOUNTS    "lazy_refcounts"
> +#define BLOCK_OPT_IMAGE_FILE        "image_file"
> +#define BLOCK_OPT_IMAGE_FORMAT      "image_format"
> 
>  typedef struct BdrvTrackedRequest BdrvTrackedRequest;
> 
> -- 
> 1.7.1
> 
>
Robert Wang - Sept. 10, 2012, 2:25 a.m.
On Fri, Sep 7, 2012 at 4:19 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> On Fri, Aug 10, 2012 at 11:39:44PM +0800, Dong Xu Wang wrote:
>> add-cow file format core code. It use block-cache.c as cache code.
>>
>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> ---
>>  block/Makefile.objs |    1 +
>>  block/add-cow.c     |  613 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  block/add-cow.h     |   85 +++++++
>>  block_int.h         |    2 +
>>  4 files changed, 701 insertions(+), 0 deletions(-)
>>  create mode 100644 block/add-cow.c
>>  create mode 100644 block/add-cow.h
>>
>> diff --git a/block/Makefile.objs b/block/Makefile.objs
>> index 23bdfc8..7ed5051 100644
>> --- a/block/Makefile.objs
>> +++ b/block/Makefile.objs
>> @@ -2,6 +2,7 @@ block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat
>>  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>>  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>>  block-obj-y += qed-check.o
>> +block-obj-y += add-cow.o
>>  block-obj-y += block-cache.o
>>  block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>>  block-obj-y += stream.o
>> diff --git a/block/add-cow.c b/block/add-cow.c
>> new file mode 100644
>> index 0000000..d4711d5
>> --- /dev/null
>> +++ b/block/add-cow.c
>> @@ -0,0 +1,613 @@
>> +/*
>> + * QEMU ADD-COW Disk Format
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu-common.h"
>> +#include "block_int.h"
>> +#include "module.h"
>> +#include "add-cow.h"
>> +
>> +static void add_cow_header_le_to_cpu(const AddCowHeader *le, AddCowHeader *cpu)
>> +{
>> +    cpu->magic                      = le64_to_cpu(le->magic);
>> +    cpu->version                    = le32_to_cpu(le->version);
>> +
>> +    cpu->backing_filename_offset    = le32_to_cpu(le->backing_filename_offset);
>> +    cpu->backing_filename_size      = le32_to_cpu(le->backing_filename_size);
>> +
>> +    cpu->image_filename_offset      = le32_to_cpu(le->image_filename_offset);
>> +    cpu->image_filename_size        = le32_to_cpu(le->image_filename_size);
>> +
>> +    cpu->features                   = le64_to_cpu(le->features);
>> +    cpu->optional_features          = le64_to_cpu(le->optional_features);
>> +    cpu->header_pages_size          = le32_to_cpu(le->header_pages_size);
>> +}
>> +
>> +static void add_cow_header_cpu_to_le(const AddCowHeader *cpu, AddCowHeader *le)
>> +{
>> +    le->magic                       = cpu_to_le64(cpu->magic);
>> +    le->version                     = cpu_to_le32(cpu->version);
>> +
>> +    le->backing_filename_offset     = cpu_to_le32(cpu->backing_filename_offset);
>> +    le->backing_filename_size       = cpu_to_le32(cpu->backing_filename_size);
>> +
>> +    le->image_filename_offset       = cpu_to_le32(cpu->image_filename_offset);
>> +    le->image_filename_size         = cpu_to_le32(cpu->image_filename_size);
>> +
>> +    le->features                    = cpu_to_le64(cpu->features);
>> +    le->optional_features           = cpu_to_le64(cpu->optional_features);
>> +    le->header_pages_size           = cpu_to_le32(cpu->header_pages_size);
>> +}
>> +
>> +static int add_cow_probe(const uint8_t *buf, int buf_size, const char *filename)
>> +{
>> +    const AddCowHeader *header = (const AddCowHeader *)buf;
>> +
>> +    if (le64_to_cpu(header->magic) == ADD_COW_MAGIC &&
>> +        le32_to_cpu(header->version) == ADD_COW_VERSION) {
>> +        return 100;
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>> +static int add_cow_create(const char *filename, QEMUOptionParameter *options)
>> +{
>> +    AddCowHeader header = {
>> +        .magic = ADD_COW_MAGIC,
>> +        .version = ADD_COW_VERSION,
>> +        .features = 0,
>> +        .optional_features = 0,
>> +        .header_pages_size = ADD_COW_DEFAULT_PAGE_SIZE,
>> +    };
>> +    AddCowHeader le_header;
>> +    int64_t image_len = 0;
>> +    const char *backing_filename = NULL;
>> +    const char *backing_fmt = NULL;
>> +    const char *image_filename = NULL;
>> +    const char *image_format = NULL;
>> +    BlockDriverState *bs, *image_bs = NULL, *backing_bs = NULL;
>> +    BlockDriver *drv = bdrv_find_format("add-cow");
>> +    BDRVAddCowState s;
>> +    int ret;
>> +
>> +    while (options && options->name) {
>> +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>> +            image_len = options->value.n;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
>> +            backing_filename = options->value.s;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
>> +            backing_fmt = options->value.s;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FILE)) {
>> +            image_filename = options->value.s;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FORMAT)) {
>> +            image_format = options->value.s;
>> +        }
>> +        options++;
>> +    }
>> +
>> +    if (backing_filename) {
>> +        header.backing_filename_offset = sizeof(header)
>> +            + sizeof(s.backing_file_format) + sizeof(s.image_file_format);
>> +        header.backing_filename_size = strlen(backing_filename);
>> +
>> +        if (!backing_fmt) {
>> +            backing_bs = bdrv_new("image");
>> +            ret = bdrv_open(backing_bs, backing_filename, BDRV_O_RDWR
>> +                    | BDRV_O_CACHE_WB, NULL);
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +            backing_fmt = bdrv_get_format_name(backing_bs);
>> +            bdrv_delete(backing_bs);
>> +        }
>> +    } else {
>> +        header.features |= ADD_COW_F_All_ALLOCATED;
>> +    }
>> +
>> +    if (image_filename) {
>> +        header.image_filename_offset =
>> +            sizeof(header) + sizeof(s.backing_file_format)
>> +                + sizeof(s.image_file_format) + header.backing_filename_size;
>> +        header.image_filename_size = strlen(image_filename);
>> +    } else {
>> +        error_report("Error: image_file should be given.");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (backing_filename && !strcmp(backing_filename, image_filename)) {
>> +        error_report("Error: Trying to create an image with the "
>> +                     "same backing file name as the image file name");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (!strcmp(filename, image_filename)) {
>> +        error_report("Error: Trying to create an image with the "
>> +                     "same filename as the image file name");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (header.image_filename_offset + header.image_filename_size
>> +            > ADD_COW_PAGE_SIZE * ADD_COW_DEFAULT_PAGE_SIZE) {
>> +        error_report("image_file name or backing_file name too long.");
>> +        return -ENOSPC;
>> +    }
>> +
>> +    ret = bdrv_file_open(&image_bs, image_filename, BDRV_O_RDWR);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    bdrv_delete(image_bs);
>> +
>> +    ret = bdrv_create_file(filename, NULL);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    add_cow_header_cpu_to_le(&header, &le_header);
>> +    ret = bdrv_pwrite(bs, 0, &le_header, sizeof(le_header));
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_pwrite(bs, sizeof(le_header), backing_fmt ? backing_fmt : "",
>> +        backing_fmt ? strlen(backing_fmt) : 0);
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_pwrite(bs, sizeof(le_header) + sizeof(s.backing_file_format),
>> +        image_format ? image_format : "raw",
>> +        image_format ? strlen(image_format) : sizeof("raw"));
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    if (backing_filename) {
>> +        ret = bdrv_pwrite(bs, header.backing_filename_offset,
>> +            backing_filename, header.backing_filename_size);
>> +        if (ret < 0) {
>> +            bdrv_delete(bs);
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    ret = bdrv_pwrite(bs, header.image_filename_offset,
>> +        image_filename, header.image_filename_size);
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_open(bs, filename, BDRV_O_RDWR | BDRV_O_NO_FLUSH, drv);
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_truncate(bs, image_len);
>> +    bdrv_delete(bs);
>> +    return ret;
>> +}
>> +
>> +static int add_cow_open(BlockDriverState *bs, int flags)
>> +{
>> +    char                image_filename[ADD_COW_FILE_LEN];
>> +    char                tmp_name[ADD_COW_FILE_LEN];
>> +    BlockDriver         *image_drv = NULL;
>> +    int                 ret;
>> +    int                 sector_per_byte;
>> +    BDRVAddCowState     *s = bs->opaque;
>> +    AddCowHeader        le_header;
>> +
>> +    ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
>> +    if (ret != sizeof(s->header)) {
>> +        goto fail;
>> +    }
>> +
>> +    add_cow_header_le_to_cpu(&le_header, &s->header);
>> +
>> +    if (le64_to_cpu(s->header.magic) != ADD_COW_MAGIC) {
>> +        ret = -EINVAL;
>> +        goto fail;
>> +    }
>> +
>> +    if (s->header.version != ADD_COW_VERSION) {
>> +        char version[64];
>> +        snprintf(version, sizeof(version), "ADD-COW version %d",
>> +            s->header.version);
>> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
>> +            bs->device_name, "add-cow", version);
>> +        ret = -ENOTSUP;
>> +        goto fail;
>> +    }
>> +
>> +    if (s->header.features & ~ADD_COW_FEATURE_MASK) {
>> +        char buf[64];
>> +        snprintf(buf, sizeof(buf), "%" PRIx64,
>> +            s->header.features & ~ADD_COW_FEATURE_MASK);
>> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
>> +            bs->device_name, "add-cow", buf);
>> +        return -ENOTSUP;
>> +    }
>> +
>> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
>> +        ret = bdrv_read_string(bs->file, sizeof(s->header),
>> +            sizeof(s->backing_file_format) - 1, s->backing_file_format,
>> +            sizeof(s->backing_file_format));
>> +        if (ret < 0) {
>> +            goto fail;
>> +        }
>> +    }
>> +
>> +    ret = bdrv_read_string(bs->file,
>> +            sizeof(s->header) + sizeof(s->image_file_format),
>> +            sizeof(s->image_file_format) - 1, s->image_file_format,
>> +            sizeof(s->image_file_format));
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>> +
>> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
>> +        ret = bdrv_read_string(bs->file, s->header.backing_filename_offset,
>> +                          s->header.backing_filename_size, bs->backing_file,
>> +                          sizeof(bs->backing_file));
>> +        if (ret < 0) {
>> +            goto fail;
>> +        }
>> +    }
>> +
>> +    ret = bdrv_read_string(bs->file, s->header.image_filename_offset,
>> +                      s->header.image_filename_size, tmp_name,
>> +                      sizeof(tmp_name));
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>> +
>> +    s->image_hd = bdrv_new("");
>> +    if (path_has_protocol(image_filename)) {
>> +        pstrcpy(image_filename, sizeof(image_filename), tmp_name);
>> +    } else {
>> +        path_combine(image_filename, sizeof(image_filename),
>> +                     bs->filename, tmp_name);
>> +    }
>> +
>> +    ret = bdrv_open(s->image_hd, image_filename, flags, image_drv);
>> +    if (ret < 0) {
>> +        bdrv_delete(s->image_hd);
>> +        goto fail;
>> +    }
>> +
>> +    bs->total_sectors = bdrv_getlength(s->image_hd) >> 9;
>> +    s->cluster_size = ADD_COW_CLUSTER_SIZE;
>> +    sector_per_byte = SECTORS_PER_CLUSTER * 8;
>> +    s->bitmap_size =
>> +        (bs->total_sectors + sector_per_byte - 1) / sector_per_byte;
>> +    s->bitmap_cache =
>> +        block_cache_create(bs, ADD_COW_CACHE_SIZE, ADD_COW_CACHE_ENTRY_SIZE);
>> +
>> +    qemu_co_mutex_init(&s->lock);
>> +    return 0;
>> +fail:
>> +    if (s->bitmap_cache) {
>> +        block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
>> +    }
>> +    return ret;
>> +}
>> +
>> +static void add_cow_close(BlockDriverState *bs)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
>> +    bdrv_delete(s->image_hd);
>> +}
>> +
>> +static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
>> +{
>> +    BDRVAddCowState *s  = bs->opaque;
>> +    BlockCache *c = s->bitmap_cache;
>> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
>> +    uint8_t *table      = NULL;
>> +    uint64_t offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
>> +        + (offset_in_bitmap(sector_num) & (~(c->entry_size - 1)));
>> +    int ret = block_cache_get(bs, s->bitmap_cache, offset,
>> +        (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
>> +
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    return table[cluster_num / 8 % ADD_COW_CACHE_ENTRY_SIZE]
>> +        & (1 << (cluster_num % 8));
>> +}
>> +
>> +static coroutine_fn int add_cow_is_allocated(BlockDriverState *bs,
>> +        int64_t sector_num, int nb_sectors, int *num_same)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    int changed;
>> +
>> +    if (nb_sectors == 0) {
>> +        *num_same = 0;
>> +        return 0;
>> +    }
>> +
>> +    if (s->header.features & ADD_COW_F_All_ALLOCATED) {
>> +        *num_same = nb_sectors - 1;
>> +        return 1;
>> +    }
>> +    changed = is_allocated(bs, sector_num);
>> +
>> +    for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
>> +        if (is_allocated(bs, sector_num + *num_same) != changed) {
>> +            break;
>> +        }
>> +    }
>> +    return changed;
>> +}
>> +
>> +static int add_cow_backing_read(BlockDriverState *bs, QEMUIOVector *qiov,
>> +                  int64_t sector_num, int nb_sectors)
>> +{
>> +    int n1;
>> +    if ((sector_num + nb_sectors) <= bs->total_sectors) {
>> +        return nb_sectors;
>> +    }
>> +    if (sector_num >= bs->total_sectors) {
>> +        n1 = 0;
>> +    } else {
>> +        n1 = bs->total_sectors - sector_num;
>> +    }
>> +
>> +    qemu_iovec_memset(qiov, BDRV_SECTOR_SIZE * n1,
>> +        0, BDRV_SECTOR_SIZE * (nb_sectors - n1));
>> +
>> +    return n1;
>> +}
>> +
>> +static coroutine_fn int add_cow_co_readv(BlockDriverState *bs,
>> +    int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
>> +{
>> +    BDRVAddCowState *s  = bs->opaque;
>> +    int cur_nr_sectors;
>> +    uint64_t bytes_done = 0;
>> +    QEMUIOVector hd_qiov;
>> +    int n, n1, ret = 0;
>> +
>> +    qemu_iovec_init(&hd_qiov, qiov->niov);
>> +    qemu_co_mutex_lock(&s->lock);
>> +    while (remaining_sectors != 0) {
>> +        cur_nr_sectors = remaining_sectors;
>> +        if (add_cow_is_allocated(bs, sector_num, cur_nr_sectors, &n)) {
>> +            cur_nr_sectors = n;
>> +            qemu_iovec_reset(&hd_qiov);
>> +            qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
>> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
>> +            qemu_co_mutex_unlock(&s->lock);
>> +            ret = bdrv_co_readv(s->image_hd, sector_num, n, &hd_qiov);
>> +            qemu_co_mutex_lock(&s->lock);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +        } else {
>> +            cur_nr_sectors = n;
>> +            if (bs->backing_hd) {
>> +                qemu_iovec_reset(&hd_qiov);
>> +                qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
>> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
>> +                n1 = add_cow_backing_read(bs->backing_hd, &hd_qiov,
>> +                    sector_num, cur_nr_sectors);
>> +                if (n1 > 0) {
>> +                    qemu_co_mutex_unlock(&s->lock);
>> +                    ret = bdrv_co_readv(bs->backing_hd, sector_num,
>> +                                        n, &hd_qiov);
>> +                    qemu_co_mutex_lock(&s->lock);
>> +                    if (ret < 0) {
>> +                        goto fail;
>> +                    }
>> +                }
>> +            } else {
>> +                qemu_iovec_memset(&hd_qiov, 0, 0,
>> +                    BDRV_SECTOR_SIZE * cur_nr_sectors);
>> +            }
>> +        }
>> +        remaining_sectors -= cur_nr_sectors;
>> +        sector_num += cur_nr_sectors;
>> +        bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
>> +    }
>> +fail:
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    qemu_iovec_destroy(&hd_qiov);
>> +    return ret;
>> +}
>> +
>> +static int coroutine_fn copy_sectors(BlockDriverState *bs,
>> +                                     int n_start, int n_end)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    QEMUIOVector qiov;
>> +    struct iovec iov;
>> +    int n, ret;
>> +
>> +    n = n_end - n_start;
>> +    if (n <= 0) {
>> +        return 0;
>> +    }
>> +
>> +    iov.iov_len = n * BDRV_SECTOR_SIZE;
>> +    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
>> +
>> +    qemu_iovec_init_external(&qiov, &iov, 1);
>> +
>> +    ret = bdrv_co_readv(bs->backing_hd, n_start, n, &qiov);
>> +    if (ret < 0) {
>> +        goto out;
>> +    }
>> +    ret = bdrv_co_writev(s->image_hd, n_start, n, &qiov);
>> +    if (ret < 0) {
>> +        goto out;
>> +    }
>> +
>> +    ret = 0;
>> +out:
>> +    qemu_vfree(iov.iov_base);
>> +    return ret;
>> +}
>> +
>> +static coroutine_fn int add_cow_co_writev(BlockDriverState *bs,
>> +        int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    BlockCache *c = s->bitmap_cache;
>> +    int ret = 0, i;
>> +    QEMUIOVector hd_qiov;
>> +    uint8_t *table;
>> +    uint64_t offset;
>> +
>> +    qemu_co_mutex_lock(&s->lock);
>> +    qemu_iovec_init(&hd_qiov, qiov->niov);
>> +    ret = bdrv_co_writev(s->image_hd,
>> +                     sector_num,
>> +                     remaining_sectors, qiov);
>
> alignment                   ^
>
> or even at ^ if you prefer and have done in some places, just need to be
> consistent about it for better readability.
>
>> +
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
>> +        /* Copy content of unmodified sectors */
>> +        if (!is_cluster_head(sector_num) && !is_allocated(bs, sector_num)) {
>
> Why do we avoid a COW when writing to the first sector of a cluster?

Because if it is the first sector, we need not use copy_sector, we
write it directly would be enough, it starts at the begening of one
cluster.

>
>> +            ret = copy_sectors(bs, sector_num & ~(SECTORS_PER_CLUSTER - 1),
>> +                sector_num);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +        }
>> +
>> +        if (!is_cluster_tail(sector_num + remaining_sectors - 1)
>> +            && !is_allocated(bs, sector_num + remaining_sectors - 1)) {
>> +            ret = copy_sectors(bs, sector_num + remaining_sectors,
>> +                ((sector_num + remaining_sectors) | (SECTORS_PER_CLUSTER - 1)) + 1);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +        }
>> +
>> +        for (i = sector_num / SECTORS_PER_CLUSTER;
>> +            i <= (sector_num + remaining_sectors - 1) / SECTORS_PER_CLUSTER;
>> +            i++) {
>> +            offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
>> +                + (offset_in_bitmap(i * SECTORS_PER_CLUSTER) & (~(c->entry_size - 1)));
>> +            ret = block_cache_get(bs, s->bitmap_cache, offset,
>> +                (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +            if ((table[i / 8] & (1 << (i % 8))) == 0) {
>> +                table[i / 8] |= (1 << (i % 8));
>> +                block_cache_entry_mark_dirty(s->bitmap_cache, table);
>> +            }
>> +        }
>> +    }
>> +    ret = 0;
>> +fail:
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    qemu_iovec_destroy(&hd_qiov);
>> +    return ret;
>> +}
>> +
>> +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
>> +    int ret;
>> +    uint32_t bitmap_pos = s->header.header_pages_size * ADD_COW_PAGE_SIZE;
>> +    int64_t bitmap_size =
>> +        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
>> +    bitmap_size = (bitmap_size + ADD_COW_CACHE_ENTRY_SIZE - 1)
>> +        & (~(ADD_COW_CACHE_ENTRY_SIZE - 1));
>> +
>> +    ret = bdrv_truncate(bs->file, bitmap_pos + bitmap_size);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    int ret;
>> +
>> +    qemu_co_mutex_lock(&s->lock);
>> +    ret = block_cache_flush(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP,
>> +        ADD_COW_CACHE_ENTRY_SIZE);
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    return ret;
>> +}
>> +
>> +static QEMUOptionParameter add_cow_create_options[] = {
>> +    {
>> +        .name = BLOCK_OPT_SIZE,
>> +        .type = OPT_SIZE,
>> +        .help = "Virtual disk size"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_BACKING_FILE,
>> +        .type = OPT_STRING,
>> +        .help = "File name of a base image"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_BACKING_FMT,
>> +        .type = OPT_STRING,
>> +        .help = "Image format of the base image"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_IMAGE_FILE,
>> +        .type = OPT_STRING,
>> +        .help = "File name of a image file"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_IMAGE_FORMAT,
>> +        .type = OPT_STRING,
>> +        .help = "Image format of the image file"
>> +    },
>> +    { NULL }
>> +};
>> +
>> +static BlockDriver bdrv_add_cow = {
>> +    .format_name                = "add-cow",
>> +    .instance_size              = sizeof(BDRVAddCowState),
>> +    .bdrv_probe                 = add_cow_probe,
>> +    .bdrv_open                  = add_cow_open,
>> +    .bdrv_close                 = add_cow_close,
>> +    .bdrv_create                = add_cow_create,
>> +    .bdrv_co_readv              = add_cow_co_readv,
>> +    .bdrv_co_writev             = add_cow_co_writev,
>> +    .bdrv_truncate              = bdrv_add_cow_truncate,
>> +    .bdrv_co_is_allocated       = add_cow_is_allocated,
>> +
>> +    .create_options             = add_cow_create_options,
>> +    .bdrv_co_flush_to_os        = add_cow_co_flush,
>> +};
>> +
>> +static void bdrv_add_cow_init(void)
>> +{
>> +    bdrv_register(&bdrv_add_cow);
>> +}
>> +
>> +block_init(bdrv_add_cow_init);
>> diff --git a/block/add-cow.h b/block/add-cow.h
>> new file mode 100644
>> index 0000000..f058376
>> --- /dev/null
>> +++ b/block/add-cow.h
>> @@ -0,0 +1,85 @@
>> +/*
>> + * QEMU ADD-COW Disk Format
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef BLOCK_ADD_COW_H
>> +#define BLOCK_ADD_COW_H
>> +#include "block-cache.h"
>> +
>> +enum {
>> +    ADD_COW_F_All_ALLOCATED     = 0X01,
>
> Please use "ADD_COW_F_ALL_ALLOCATED" (all caps)

Okay.
>
> was searching your patch for how this was used and was scratching my
> head when I wasn't seeing any matches :)

It wil be used such as:
qemu-img create -f add-cow -o image_file=t.raw t.add-cow

while we need not read from backing_file any more.

>
>> +    ADD_COW_FEATURE_MASK        = ADD_COW_F_All_ALLOCATED,
>> +
>> +    ADD_COW_MAGIC = (((uint64_t)'A' << 56) | ((uint64_t)'D' << 48) | \
>> +                    ((uint64_t)'D' << 40) | ((uint64_t)'_' << 32) | \
>> +                    ((uint64_t)'C' << 24) | ((uint64_t)'O' << 16) | \
>> +                    ((uint64_t)'W' << 8) | 0xFF),
>> +    ADD_COW_VERSION             = 1,
>> +    ADD_COW_FILE_LEN            = 1024,
>> +    ADD_COW_CACHE_SIZE          = 16,
>> +    ADD_COW_CACHE_ENTRY_SIZE    = 65536,
>> +    ADD_COW_CLUSTER_SIZE        = 65536,
>> +    SECTORS_PER_CLUSTER         = (ADD_COW_CLUSTER_SIZE / BDRV_SECTOR_SIZE),
>> +    ADD_COW_PAGE_SIZE           = 4096,
>> +    ADD_COW_DEFAULT_PAGE_SIZE   = 1,
>> +};
>> +
>> +typedef struct AddCowHeader {
>> +    uint64_t        magic;
>> +    uint32_t        version;
>> +
>> +    uint32_t        backing_filename_offset;
>> +    uint32_t        backing_filename_size;
>> +
>> +    uint32_t        image_filename_offset;
>> +    uint32_t        image_filename_size;
>> +
>> +    uint64_t        features;
>> +    uint64_t        optional_features;
>> +    uint32_t        header_pages_size;
>> +} QEMU_PACKED AddCowHeader;
>
> You should avoid using packed structures for image format headers.
> Instead, I would either:
>
> a) re-order the fields so that 32/64-bit fields, respectively, fall on
> 32/64-bit boundaries (in your case, for instance, moving header_pages_size
> above features) like qed/qcow2 do, or
>
> b) read/write the fields individually rather than reading/writing directly
> into/from the header struct.
>
> The safest route is b). Adds a few lines of code, but you won't have to
> re-work things (or worry about introducing bugs) later if you were to add,
> say, a 32-bit value, and then a 64-bit value later.

While, Kevin's suggestion is using PACKED, so ..
>
>> +
>> +typedef struct BDRVAddCowState {
>> +    BlockDriverState    *image_hd;
>> +    CoMutex             lock;
>> +    int                 cluster_size;
>> +    BlockCache         *bitmap_cache;
>> +    uint64_t            bitmap_size;
>> +    AddCowHeader        header;
>> +    char                backing_file_format[16];
>> +    char                image_file_format[16];
>> +} BDRVAddCowState;
>> +
>> +/* Convert sector_num to offset in bitmap */
>> +static inline int64_t offset_in_bitmap(int64_t sector_num)
>> +{
>> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
>> +    return cluster_num / 8;
>> +}
>> +
>> +static inline bool is_cluster_head(int64_t sector_num)
>> +{
>> +    return sector_num % SECTORS_PER_CLUSTER == 0;
>> +}
>> +
>> +static inline bool is_cluster_tail(int64_t sector_num)
>> +{
>> +    return (sector_num + 1) % SECTORS_PER_CLUSTER == 0;
>> +}
>> +
>> +BlockCache *add_cow_cache_create(BlockDriverState *bs, int num_tables);
>> +int add_cow_cache_destroy(BlockDriverState *bs, BlockCache *c);
>> +void add_cow_cache_entry_mark_dirty(BlockCache *c, void *table);
>> +int add_cow_cache_get(BlockDriverState *bs, BlockCache *c, uint64_t offset,
>> +    void **table);
>> +int add_cow_cache_flush(BlockDriverState *bs, BlockCache *c);
>> +#endif
>> diff --git a/block_int.h b/block_int.h
>> index 6c1d9ca..67954ec 100644
>> --- a/block_int.h
>> +++ b/block_int.h
>> @@ -53,6 +53,8 @@
>>  #define BLOCK_OPT_SUBFMT            "subformat"
>>  #define BLOCK_OPT_COMPAT_LEVEL      "compat"
>>  #define BLOCK_OPT_LAZY_REFCOUNTS    "lazy_refcounts"
>> +#define BLOCK_OPT_IMAGE_FILE        "image_file"
>> +#define BLOCK_OPT_IMAGE_FORMAT      "image_format"
>>
>>  typedef struct BdrvTrackedRequest BdrvTrackedRequest;
>>
>> --
>> 1.7.1
>>
>>
>
Kevin Wolf - Sept. 11, 2012, 9:40 a.m.
Am 10.08.2012 17:39, schrieb Dong Xu Wang:
> add-cow file format core code. It use block-cache.c as cache code.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> ---
>  block/Makefile.objs |    1 +
>  block/add-cow.c     |  613 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  block/add-cow.h     |   85 +++++++
>  block_int.h         |    2 +
>  4 files changed, 701 insertions(+), 0 deletions(-)
>  create mode 100644 block/add-cow.c
>  create mode 100644 block/add-cow.h
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 23bdfc8..7ed5051 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -2,6 +2,7 @@ block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat
>  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>  block-obj-y += qed-check.o
> +block-obj-y += add-cow.o
>  block-obj-y += block-cache.o
>  block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>  block-obj-y += stream.o
> diff --git a/block/add-cow.c b/block/add-cow.c
> new file mode 100644
> index 0000000..d4711d5
> --- /dev/null
> +++ b/block/add-cow.c
> @@ -0,0 +1,613 @@
> +/*
> + * QEMU ADD-COW Disk Format
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "block_int.h"
> +#include "module.h"
> +#include "add-cow.h"
> +
> +static void add_cow_header_le_to_cpu(const AddCowHeader *le, AddCowHeader *cpu)
> +{
> +    cpu->magic                      = le64_to_cpu(le->magic);
> +    cpu->version                    = le32_to_cpu(le->version);
> +
> +    cpu->backing_filename_offset    = le32_to_cpu(le->backing_filename_offset);
> +    cpu->backing_filename_size      = le32_to_cpu(le->backing_filename_size);
> +
> +    cpu->image_filename_offset      = le32_to_cpu(le->image_filename_offset);
> +    cpu->image_filename_size        = le32_to_cpu(le->image_filename_size);
> +
> +    cpu->features                   = le64_to_cpu(le->features);
> +    cpu->optional_features          = le64_to_cpu(le->optional_features);
> +    cpu->header_pages_size          = le32_to_cpu(le->header_pages_size);
> +}
> +
> +static void add_cow_header_cpu_to_le(const AddCowHeader *cpu, AddCowHeader *le)
> +{
> +    le->magic                       = cpu_to_le64(cpu->magic);
> +    le->version                     = cpu_to_le32(cpu->version);
> +
> +    le->backing_filename_offset     = cpu_to_le32(cpu->backing_filename_offset);
> +    le->backing_filename_size       = cpu_to_le32(cpu->backing_filename_size);
> +
> +    le->image_filename_offset       = cpu_to_le32(cpu->image_filename_offset);
> +    le->image_filename_size         = cpu_to_le32(cpu->image_filename_size);
> +
> +    le->features                    = cpu_to_le64(cpu->features);
> +    le->optional_features           = cpu_to_le64(cpu->optional_features);
> +    le->header_pages_size           = cpu_to_le32(cpu->header_pages_size);
> +}
> +
> +static int add_cow_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    const AddCowHeader *header = (const AddCowHeader *)buf;
> +
> +    if (le64_to_cpu(header->magic) == ADD_COW_MAGIC &&
> +        le32_to_cpu(header->version) == ADD_COW_VERSION) {
> +        return 100;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +static int add_cow_create(const char *filename, QEMUOptionParameter *options)
> +{
> +    AddCowHeader header = {
> +        .magic = ADD_COW_MAGIC,
> +        .version = ADD_COW_VERSION,
> +        .features = 0,
> +        .optional_features = 0,
> +        .header_pages_size = ADD_COW_DEFAULT_PAGE_SIZE,
> +    };
> +    AddCowHeader le_header;
> +    int64_t image_len = 0;
> +    const char *backing_filename = NULL;
> +    const char *backing_fmt = NULL;
> +    const char *image_filename = NULL;
> +    const char *image_format = NULL;
> +    BlockDriverState *bs, *image_bs = NULL, *backing_bs = NULL;
> +    BlockDriver *drv = bdrv_find_format("add-cow");
> +    BDRVAddCowState s;
> +    int ret;
> +
> +    while (options && options->name) {
> +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> +            image_len = options->value.n;
> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
> +            backing_filename = options->value.s;
> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
> +            backing_fmt = options->value.s;
> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FILE)) {
> +            image_filename = options->value.s;
> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FORMAT)) {
> +            image_format = options->value.s;
> +        }
> +        options++;
> +    }
> +
> +    if (backing_filename) {
> +        header.backing_filename_offset = sizeof(header)
> +            + sizeof(s.backing_file_format) + sizeof(s.image_file_format);
> +        header.backing_filename_size = strlen(backing_filename);
> +
> +        if (!backing_fmt) {
> +            backing_bs = bdrv_new("image");
> +            ret = bdrv_open(backing_bs, backing_filename, BDRV_O_RDWR
> +                    | BDRV_O_CACHE_WB, NULL);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +            backing_fmt = bdrv_get_format_name(backing_bs);
> +            bdrv_delete(backing_bs);
> +        }
> +    } else {
> +        header.features |= ADD_COW_F_All_ALLOCATED;
> +    }
> +
> +    if (image_filename) {
> +        header.image_filename_offset =
> +            sizeof(header) + sizeof(s.backing_file_format)
> +                + sizeof(s.image_file_format) + header.backing_filename_size;
> +        header.image_filename_size = strlen(image_filename);
> +    } else {
> +        error_report("Error: image_file should be given.");
> +        return -EINVAL;
> +    }
> +
> +    if (backing_filename && !strcmp(backing_filename, image_filename)) {
> +        error_report("Error: Trying to create an image with the "
> +                     "same backing file name as the image file name");
> +        return -EINVAL;
> +    }
> +
> +    if (!strcmp(filename, image_filename)) {
> +        error_report("Error: Trying to create an image with the "
> +                     "same filename as the image file name");
> +        return -EINVAL;
> +    }
> +
> +    if (header.image_filename_offset + header.image_filename_size
> +            > ADD_COW_PAGE_SIZE * ADD_COW_DEFAULT_PAGE_SIZE) {
> +        error_report("image_file name or backing_file name too long.");
> +        return -ENOSPC;
> +    }
> +
> +    ret = bdrv_file_open(&image_bs, image_filename, BDRV_O_RDWR);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    bdrv_delete(image_bs);
> +
> +    ret = bdrv_create_file(filename, NULL);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    add_cow_header_cpu_to_le(&header, &le_header);
> +    ret = bdrv_pwrite(bs, 0, &le_header, sizeof(le_header));
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_pwrite(bs, sizeof(le_header), backing_fmt ? backing_fmt : "",
> +        backing_fmt ? strlen(backing_fmt) : 0);

The spec requires zero padding, which you don't do here.

> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_pwrite(bs, sizeof(le_header) + sizeof(s.backing_file_format),
> +        image_format ? image_format : "raw",
> +        image_format ? strlen(image_format) : sizeof("raw"));

And here.

> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    if (backing_filename) {
> +        ret = bdrv_pwrite(bs, header.backing_filename_offset,
> +            backing_filename, header.backing_filename_size);
> +        if (ret < 0) {
> +            bdrv_delete(bs);
> +            return ret;
> +        }
> +    }
> +
> +    ret = bdrv_pwrite(bs, header.image_filename_offset,
> +        image_filename, header.image_filename_size);
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_open(bs, filename, BDRV_O_RDWR | BDRV_O_NO_FLUSH, drv);
> +    if (ret < 0) {
> +        bdrv_delete(bs);
> +        return ret;
> +    }
> +
> +    ret = bdrv_truncate(bs, image_len);
> +    bdrv_delete(bs);
> +    return ret;
> +}
> +
> +static int add_cow_open(BlockDriverState *bs, int flags)
> +{
> +    char                image_filename[ADD_COW_FILE_LEN];
> +    char                tmp_name[ADD_COW_FILE_LEN];
> +    BlockDriver         *image_drv = NULL;
> +    int                 ret;
> +    int                 sector_per_byte;
> +    BDRVAddCowState     *s = bs->opaque;
> +    AddCowHeader        le_header;
> +
> +    ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
> +    if (ret != sizeof(s->header)) {

if (ret < 0) would be more consistent with the rest of the code.

> +        goto fail;
> +    }
> +
> +    add_cow_header_le_to_cpu(&le_header, &s->header);
> +
> +    if (le64_to_cpu(s->header.magic) != ADD_COW_MAGIC) {

Isn't this one endianess conversion too much? s->header is already LE.

Did you test add-cow on a big endian host?

> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    if (s->header.version != ADD_COW_VERSION) {
> +        char version[64];
> +        snprintf(version, sizeof(version), "ADD-COW version %d",
> +            s->header.version);
> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
> +            bs->device_name, "add-cow", version);
> +        ret = -ENOTSUP;
> +        goto fail;
> +    }
> +
> +    if (s->header.features & ~ADD_COW_FEATURE_MASK) {
> +        char buf[64];
> +        snprintf(buf, sizeof(buf), "%" PRIx64,
> +            s->header.features & ~ADD_COW_FEATURE_MASK);

This message is a bit terse, most users will be confused with an error
message that only consists of a hex number. Maybe better "Feature flags:
%" PRIx64.

> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
> +            bs->device_name, "add-cow", buf);
> +        return -ENOTSUP;
> +    }
> +
> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
> +        ret = bdrv_read_string(bs->file, sizeof(s->header),
> +            sizeof(s->backing_file_format) - 1, s->backing_file_format,
> +            sizeof(s->backing_file_format));
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +    }

Would be great if this was not only read into memory, but actually
used... It must end up in bs->backing_format in order take effect.

> +
> +    ret = bdrv_read_string(bs->file,
> +            sizeof(s->header) + sizeof(s->image_file_format),
> +            sizeof(s->image_file_format) - 1, s->image_file_format,
> +            sizeof(s->image_file_format));
> +    if (ret < 0) {
> +        goto fail;
> +    }

This one is unused, too.

> +
> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
> +        ret = bdrv_read_string(bs->file, s->header.backing_filename_offset,
> +                          s->header.backing_filename_size, bs->backing_file,
> +                          sizeof(bs->backing_file));
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +    }
> +
> +    ret = bdrv_read_string(bs->file, s->header.image_filename_offset,
> +                      s->header.image_filename_size, tmp_name,
> +                      sizeof(tmp_name));
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +
> +    s->image_hd = bdrv_new("");
> +    if (path_has_protocol(image_filename)) {
> +        pstrcpy(image_filename, sizeof(image_filename), tmp_name);
> +    } else {
> +        path_combine(image_filename, sizeof(image_filename),
> +                     bs->filename, tmp_name);
> +    }
> +
> +    ret = bdrv_open(s->image_hd, image_filename, flags, image_drv);

image_drv is always NULL.

> +    if (ret < 0) {
> +        bdrv_delete(s->image_hd);
> +        goto fail;
> +    }
> +
> +    bs->total_sectors = bdrv_getlength(s->image_hd) >> 9;
> +    s->cluster_size = ADD_COW_CLUSTER_SIZE;
> +    sector_per_byte = SECTORS_PER_CLUSTER * 8;
> +    s->bitmap_size =
> +        (bs->total_sectors + sector_per_byte - 1) / sector_per_byte;
> +    s->bitmap_cache =
> +        block_cache_create(bs, ADD_COW_CACHE_SIZE, ADD_COW_CACHE_ENTRY_SIZE);
> +
> +    qemu_co_mutex_init(&s->lock);
> +    return 0;
> +fail:
> +    if (s->bitmap_cache) {
> +        block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
> +    }
> +    return ret;
> +}
> +
> +static void add_cow_close(BlockDriverState *bs)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
> +    bdrv_delete(s->image_hd);
> +}
> +
> +static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
> +{
> +    BDRVAddCowState *s  = bs->opaque;
> +    BlockCache *c = s->bitmap_cache;
> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
> +    uint8_t *table      = NULL;
> +    uint64_t offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
> +        + (offset_in_bitmap(sector_num) & (~(c->entry_size - 1)));
> +    int ret = block_cache_get(bs, s->bitmap_cache, offset,
> +        (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);

No matching block_cache_put?

> +
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return table[cluster_num / 8 % ADD_COW_CACHE_ENTRY_SIZE]
> +        & (1 << (cluster_num % 8));
> +}
> +
> +static coroutine_fn int add_cow_is_allocated(BlockDriverState *bs,
> +        int64_t sector_num, int nb_sectors, int *num_same)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    int changed;
> +
> +    if (nb_sectors == 0) {
> +        *num_same = 0;
> +        return 0;
> +    }
> +
> +    if (s->header.features & ADD_COW_F_All_ALLOCATED) {
> +        *num_same = nb_sectors - 1;

Why - 1?

> +        return 1;
> +    }
> +    changed = is_allocated(bs, sector_num);
> +
> +    for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
> +        if (is_allocated(bs, sector_num + *num_same) != changed) {
> +            break;
> +        }
> +    }
> +    return changed;
> +}
> +
> +static int add_cow_backing_read(BlockDriverState *bs, QEMUIOVector *qiov,
> +                  int64_t sector_num, int nb_sectors)
> +{
> +    int n1;
> +    if ((sector_num + nb_sectors) <= bs->total_sectors) {
> +        return nb_sectors;
> +    }
> +    if (sector_num >= bs->total_sectors) {
> +        n1 = 0;
> +    } else {
> +        n1 = bs->total_sectors - sector_num;
> +    }
> +
> +    qemu_iovec_memset(qiov, BDRV_SECTOR_SIZE * n1,
> +        0, BDRV_SECTOR_SIZE * (nb_sectors - n1));
> +
> +    return n1;
> +}
> +
> +static coroutine_fn int add_cow_co_readv(BlockDriverState *bs,
> +    int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
> +{
> +    BDRVAddCowState *s  = bs->opaque;
> +    int cur_nr_sectors;
> +    uint64_t bytes_done = 0;
> +    QEMUIOVector hd_qiov;
> +    int n, n1, ret = 0;
> +
> +    qemu_iovec_init(&hd_qiov, qiov->niov);
> +    qemu_co_mutex_lock(&s->lock);
> +    while (remaining_sectors != 0) {
> +        cur_nr_sectors = remaining_sectors;
> +        if (add_cow_is_allocated(bs, sector_num, cur_nr_sectors, &n)) {
> +            cur_nr_sectors = n;

One of n and cur_nr_sectors is redundant.

> +            qemu_iovec_reset(&hd_qiov);
> +            qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
> +            qemu_co_mutex_unlock(&s->lock);
> +            ret = bdrv_co_readv(s->image_hd, sector_num, n, &hd_qiov);
> +            qemu_co_mutex_lock(&s->lock);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +        } else {
> +            cur_nr_sectors = n;
> +            if (bs->backing_hd) {
> +                qemu_iovec_reset(&hd_qiov);
> +                qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
> +                n1 = add_cow_backing_read(bs->backing_hd, &hd_qiov,
> +                    sector_num, cur_nr_sectors);
> +                if (n1 > 0) {
> +                    qemu_co_mutex_unlock(&s->lock);
> +                    ret = bdrv_co_readv(bs->backing_hd, sector_num,
> +                                        n, &hd_qiov);
> +                    qemu_co_mutex_lock(&s->lock);
> +                    if (ret < 0) {
> +                        goto fail;
> +                    }
> +                }
> +            } else {
> +                qemu_iovec_memset(&hd_qiov, 0, 0,
> +                    BDRV_SECTOR_SIZE * cur_nr_sectors);
> +            }
> +        }
> +        remaining_sectors -= cur_nr_sectors;
> +        sector_num += cur_nr_sectors;
> +        bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
> +    }
> +fail:
> +    qemu_co_mutex_unlock(&s->lock);
> +    qemu_iovec_destroy(&hd_qiov);
> +    return ret;
> +}
> +
> +static int coroutine_fn copy_sectors(BlockDriverState *bs,
> +                                     int n_start, int n_end)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    QEMUIOVector qiov;
> +    struct iovec iov;
> +    int n, ret;
> +
> +    n = n_end - n_start;
> +    if (n <= 0) {
> +        return 0;
> +    }
> +
> +    iov.iov_len = n * BDRV_SECTOR_SIZE;
> +    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
> +
> +    qemu_iovec_init_external(&qiov, &iov, 1);
> +
> +    ret = bdrv_co_readv(bs->backing_hd, n_start, n, &qiov);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +    ret = bdrv_co_writev(s->image_hd, n_start, n, &qiov);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    ret = 0;
> +out:
> +    qemu_vfree(iov.iov_base);
> +    return ret;
> +}
> +
> +static coroutine_fn int add_cow_co_writev(BlockDriverState *bs,
> +        int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    BlockCache *c = s->bitmap_cache;
> +    int ret = 0, i;
> +    QEMUIOVector hd_qiov;
> +    uint8_t *table;
> +    uint64_t offset;
> +
> +    qemu_co_mutex_lock(&s->lock);
> +    qemu_iovec_init(&hd_qiov, qiov->niov);
> +    ret = bdrv_co_writev(s->image_hd,
> +                     sector_num,
> +                     remaining_sectors, qiov);
> +
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
> +        /* Copy content of unmodified sectors */
> +        if (!is_cluster_head(sector_num) && !is_allocated(bs, sector_num)) {
> +            ret = copy_sectors(bs, sector_num & ~(SECTORS_PER_CLUSTER - 1),
> +                sector_num);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +        }
> +
> +        if (!is_cluster_tail(sector_num + remaining_sectors - 1)
> +            && !is_allocated(bs, sector_num + remaining_sectors - 1)) {
> +            ret = copy_sectors(bs, sector_num + remaining_sectors,
> +                ((sector_num + remaining_sectors) | (SECTORS_PER_CLUSTER - 1)) + 1);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +        }
> +
> +        for (i = sector_num / SECTORS_PER_CLUSTER;
> +            i <= (sector_num + remaining_sectors - 1) / SECTORS_PER_CLUSTER;
> +            i++) {
> +            offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
> +                + (offset_in_bitmap(i * SECTORS_PER_CLUSTER) & (~(c->entry_size - 1)));

The maths in this loop looks a bit confusing, but I think it's correct.

> +            ret = block_cache_get(bs, s->bitmap_cache, offset,
> +                (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +            if ((table[i / 8] & (1 << (i % 8))) == 0) {
> +                table[i / 8] |= (1 << (i % 8));
> +                block_cache_entry_mark_dirty(s->bitmap_cache, table);
> +            }

Missing block_cache_put again?

> +        }
> +    }
> +    ret = 0;
> +fail:
> +    qemu_co_mutex_unlock(&s->lock);
> +    qemu_iovec_destroy(&hd_qiov);
> +    return ret;
> +}
> +
> +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
> +    int ret;
> +    uint32_t bitmap_pos = s->header.header_pages_size * ADD_COW_PAGE_SIZE;
> +    int64_t bitmap_size =
> +        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
> +    bitmap_size = (bitmap_size + ADD_COW_CACHE_ENTRY_SIZE - 1)
> +        & (~(ADD_COW_CACHE_ENTRY_SIZE - 1));
> +
> +    ret = bdrv_truncate(bs->file, bitmap_pos + bitmap_size);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return 0;
> +}

So you don't truncate s->image_file? Does this work?

> +
> +static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
> +{
> +    BDRVAddCowState *s = bs->opaque;
> +    int ret;
> +
> +    qemu_co_mutex_lock(&s->lock);
> +    ret = block_cache_flush(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP,
> +        ADD_COW_CACHE_ENTRY_SIZE);
> +    qemu_co_mutex_unlock(&s->lock);
> +    return ret;
> +}

What about flushing s->image_file?

> +
> +static QEMUOptionParameter add_cow_create_options[] = {
> +    {
> +        .name = BLOCK_OPT_SIZE,
> +        .type = OPT_SIZE,
> +        .help = "Virtual disk size"
> +    },
> +    {
> +        .name = BLOCK_OPT_BACKING_FILE,
> +        .type = OPT_STRING,
> +        .help = "File name of a base image"
> +    },
> +    {
> +        .name = BLOCK_OPT_BACKING_FMT,
> +        .type = OPT_STRING,
> +        .help = "Image format of the base image"
> +    },
> +    {
> +        .name = BLOCK_OPT_IMAGE_FILE,
> +        .type = OPT_STRING,
> +        .help = "File name of a image file"
> +    },
> +    {
> +        .name = BLOCK_OPT_IMAGE_FORMAT,
> +        .type = OPT_STRING,
> +        .help = "Image format of the image file"
> +    },
> +    { NULL }
> +};
> +
> +static BlockDriver bdrv_add_cow = {
> +    .format_name                = "add-cow",
> +    .instance_size              = sizeof(BDRVAddCowState),
> +    .bdrv_probe                 = add_cow_probe,
> +    .bdrv_open                  = add_cow_open,
> +    .bdrv_close                 = add_cow_close,
> +    .bdrv_create                = add_cow_create,
> +    .bdrv_co_readv              = add_cow_co_readv,
> +    .bdrv_co_writev             = add_cow_co_writev,
> +    .bdrv_truncate              = bdrv_add_cow_truncate,
> +    .bdrv_co_is_allocated       = add_cow_is_allocated,
> +
> +    .create_options             = add_cow_create_options,
> +    .bdrv_co_flush_to_os        = add_cow_co_flush,
> +};
> +
> +static void bdrv_add_cow_init(void)
> +{
> +    bdrv_register(&bdrv_add_cow);
> +}
> +
> +block_init(bdrv_add_cow_init);
> diff --git a/block/add-cow.h b/block/add-cow.h
> new file mode 100644
> index 0000000..f058376
> --- /dev/null
> +++ b/block/add-cow.h
> @@ -0,0 +1,85 @@
> +/*
> + * QEMU ADD-COW Disk Format
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef BLOCK_ADD_COW_H
> +#define BLOCK_ADD_COW_H
> +#include "block-cache.h"
> +
> +enum {
> +    ADD_COW_F_All_ALLOCATED     = 0X01,
> +    ADD_COW_FEATURE_MASK        = ADD_COW_F_All_ALLOCATED,
> +
> +    ADD_COW_MAGIC = (((uint64_t)'A' << 56) | ((uint64_t)'D' << 48) | \
> +                    ((uint64_t)'D' << 40) | ((uint64_t)'_' << 32) | \
> +                    ((uint64_t)'C' << 24) | ((uint64_t)'O' << 16) | \
> +                    ((uint64_t)'W' << 8) | 0xFF),
> +    ADD_COW_VERSION             = 1,
> +    ADD_COW_FILE_LEN            = 1024,
> +    ADD_COW_CACHE_SIZE          = 16,
> +    ADD_COW_CACHE_ENTRY_SIZE    = 65536,
> +    ADD_COW_CLUSTER_SIZE        = 65536,
> +    SECTORS_PER_CLUSTER         = (ADD_COW_CLUSTER_SIZE / BDRV_SECTOR_SIZE),
> +    ADD_COW_PAGE_SIZE           = 4096,
> +    ADD_COW_DEFAULT_PAGE_SIZE   = 1,
> +};
> +
> +typedef struct AddCowHeader {
> +    uint64_t        magic;
> +    uint32_t        version;
> +
> +    uint32_t        backing_filename_offset;
> +    uint32_t        backing_filename_size;
> +
> +    uint32_t        image_filename_offset;
> +    uint32_t        image_filename_size;
> +
> +    uint64_t        features;
> +    uint64_t        optional_features;
> +    uint32_t        header_pages_size;
> +} QEMU_PACKED AddCowHeader;

Why aren't backing/image_file_format part of the header here? They are
in the spec. It would also simplify some offset calculation code.

> +
> +typedef struct BDRVAddCowState {
> +    BlockDriverState    *image_hd;
> +    CoMutex             lock;
> +    int                 cluster_size;
> +    BlockCache         *bitmap_cache;
> +    uint64_t            bitmap_size;
> +    AddCowHeader        header;
> +    char                backing_file_format[16];
> +    char                image_file_format[16];
> +} BDRVAddCowState;
> +
> +/* Convert sector_num to offset in bitmap */
> +static inline int64_t offset_in_bitmap(int64_t sector_num)
> +{
> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
> +    return cluster_num / 8;
> +}
> +
> +static inline bool is_cluster_head(int64_t sector_num)
> +{
> +    return sector_num % SECTORS_PER_CLUSTER == 0;
> +}
> +
> +static inline bool is_cluster_tail(int64_t sector_num)
> +{
> +    return (sector_num + 1) % SECTORS_PER_CLUSTER == 0;
> +}
> +
> +BlockCache *add_cow_cache_create(BlockDriverState *bs, int num_tables);
> +int add_cow_cache_destroy(BlockDriverState *bs, BlockCache *c);
> +void add_cow_cache_entry_mark_dirty(BlockCache *c, void *table);
> +int add_cow_cache_get(BlockDriverState *bs, BlockCache *c, uint64_t offset,
> +    void **table);
> +int add_cow_cache_flush(BlockDriverState *bs, BlockCache *c);

These functions don't really exist any more, do they?

Kevin
Kevin Wolf - Sept. 11, 2012, 9:44 a.m.
Am 10.09.2012 04:25, schrieb Dong Xu Wang:
> On Fri, Sep 7, 2012 at 4:19 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
>> On Fri, Aug 10, 2012 at 11:39:44PM +0800, Dong Xu Wang wrote:
>>> +typedef struct AddCowHeader {
>>> +    uint64_t        magic;
>>> +    uint32_t        version;
>>> +
>>> +    uint32_t        backing_filename_offset;
>>> +    uint32_t        backing_filename_size;
>>> +
>>> +    uint32_t        image_filename_offset;
>>> +    uint32_t        image_filename_size;
>>> +
>>> +    uint64_t        features;
>>> +    uint64_t        optional_features;
>>> +    uint32_t        header_pages_size;
>>> +} QEMU_PACKED AddCowHeader;
>>
>> You should avoid using packed structures for image format headers.
>> Instead, I would either:
>>
>> a) re-order the fields so that 32/64-bit fields, respectively, fall on
>> 32/64-bit boundaries (in your case, for instance, moving header_pages_size
>> above features) like qed/qcow2 do, or
>>
>> b) read/write the fields individually rather than reading/writing directly
>> into/from the header struct.
>>
>> The safest route is b). Adds a few lines of code, but you won't have to
>> re-work things (or worry about introducing bugs) later if you were to add,
>> say, a 32-bit value, and then a 64-bit value later.
> 
> While, Kevin's suggestion is using PACKED, so ..

Yes, I think QEMU_PACKED is fine, and it's the safest version.

It would be nice to additionally do Michael's option a) if you like, but
I don't think the header is accessed too often, so the optimisation
isn't that important.

Kevin
Robert Wang - Sept. 12, 2012, 7:28 a.m.
On Tue, Sep 11, 2012 at 5:40 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 10.08.2012 17:39, schrieb Dong Xu Wang:
>> add-cow file format core code. It use block-cache.c as cache code.
>>
>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> ---
>>  block/Makefile.objs |    1 +
>>  block/add-cow.c     |  613 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  block/add-cow.h     |   85 +++++++
>>  block_int.h         |    2 +
>>  4 files changed, 701 insertions(+), 0 deletions(-)
>>  create mode 100644 block/add-cow.c
>>  create mode 100644 block/add-cow.h
>>
>> diff --git a/block/Makefile.objs b/block/Makefile.objs
>> index 23bdfc8..7ed5051 100644
>> --- a/block/Makefile.objs
>> +++ b/block/Makefile.objs
>> @@ -2,6 +2,7 @@ block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat
>>  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
>>  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>>  block-obj-y += qed-check.o
>> +block-obj-y += add-cow.o
>>  block-obj-y += block-cache.o
>>  block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>>  block-obj-y += stream.o
>> diff --git a/block/add-cow.c b/block/add-cow.c
>> new file mode 100644
>> index 0000000..d4711d5
>> --- /dev/null
>> +++ b/block/add-cow.c
>> @@ -0,0 +1,613 @@
>> +/*
>> + * QEMU ADD-COW Disk Format
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu-common.h"
>> +#include "block_int.h"
>> +#include "module.h"
>> +#include "add-cow.h"
>> +
>> +static void add_cow_header_le_to_cpu(const AddCowHeader *le, AddCowHeader *cpu)
>> +{
>> +    cpu->magic                      = le64_to_cpu(le->magic);
>> +    cpu->version                    = le32_to_cpu(le->version);
>> +
>> +    cpu->backing_filename_offset    = le32_to_cpu(le->backing_filename_offset);
>> +    cpu->backing_filename_size      = le32_to_cpu(le->backing_filename_size);
>> +
>> +    cpu->image_filename_offset      = le32_to_cpu(le->image_filename_offset);
>> +    cpu->image_filename_size        = le32_to_cpu(le->image_filename_size);
>> +
>> +    cpu->features                   = le64_to_cpu(le->features);
>> +    cpu->optional_features          = le64_to_cpu(le->optional_features);
>> +    cpu->header_pages_size          = le32_to_cpu(le->header_pages_size);
>> +}
>> +
>> +static void add_cow_header_cpu_to_le(const AddCowHeader *cpu, AddCowHeader *le)
>> +{
>> +    le->magic                       = cpu_to_le64(cpu->magic);
>> +    le->version                     = cpu_to_le32(cpu->version);
>> +
>> +    le->backing_filename_offset     = cpu_to_le32(cpu->backing_filename_offset);
>> +    le->backing_filename_size       = cpu_to_le32(cpu->backing_filename_size);
>> +
>> +    le->image_filename_offset       = cpu_to_le32(cpu->image_filename_offset);
>> +    le->image_filename_size         = cpu_to_le32(cpu->image_filename_size);
>> +
>> +    le->features                    = cpu_to_le64(cpu->features);
>> +    le->optional_features           = cpu_to_le64(cpu->optional_features);
>> +    le->header_pages_size           = cpu_to_le32(cpu->header_pages_size);
>> +}
>> +
>> +static int add_cow_probe(const uint8_t *buf, int buf_size, const char *filename)
>> +{
>> +    const AddCowHeader *header = (const AddCowHeader *)buf;
>> +
>> +    if (le64_to_cpu(header->magic) == ADD_COW_MAGIC &&
>> +        le32_to_cpu(header->version) == ADD_COW_VERSION) {
>> +        return 100;
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>> +static int add_cow_create(const char *filename, QEMUOptionParameter *options)
>> +{
>> +    AddCowHeader header = {
>> +        .magic = ADD_COW_MAGIC,
>> +        .version = ADD_COW_VERSION,
>> +        .features = 0,
>> +        .optional_features = 0,
>> +        .header_pages_size = ADD_COW_DEFAULT_PAGE_SIZE,
>> +    };
>> +    AddCowHeader le_header;
>> +    int64_t image_len = 0;
>> +    const char *backing_filename = NULL;
>> +    const char *backing_fmt = NULL;
>> +    const char *image_filename = NULL;
>> +    const char *image_format = NULL;
>> +    BlockDriverState *bs, *image_bs = NULL, *backing_bs = NULL;
>> +    BlockDriver *drv = bdrv_find_format("add-cow");
>> +    BDRVAddCowState s;
>> +    int ret;
>> +
>> +    while (options && options->name) {
>> +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>> +            image_len = options->value.n;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
>> +            backing_filename = options->value.s;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
>> +            backing_fmt = options->value.s;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FILE)) {
>> +            image_filename = options->value.s;
>> +        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FORMAT)) {
>> +            image_format = options->value.s;
>> +        }
>> +        options++;
>> +    }
>> +
>> +    if (backing_filename) {
>> +        header.backing_filename_offset = sizeof(header)
>> +            + sizeof(s.backing_file_format) + sizeof(s.image_file_format);
>> +        header.backing_filename_size = strlen(backing_filename);
>> +
>> +        if (!backing_fmt) {
>> +            backing_bs = bdrv_new("image");
>> +            ret = bdrv_open(backing_bs, backing_filename, BDRV_O_RDWR
>> +                    | BDRV_O_CACHE_WB, NULL);
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +            backing_fmt = bdrv_get_format_name(backing_bs);
>> +            bdrv_delete(backing_bs);
>> +        }
>> +    } else {
>> +        header.features |= ADD_COW_F_All_ALLOCATED;
>> +    }
>> +
>> +    if (image_filename) {
>> +        header.image_filename_offset =
>> +            sizeof(header) + sizeof(s.backing_file_format)
>> +                + sizeof(s.image_file_format) + header.backing_filename_size;
>> +        header.image_filename_size = strlen(image_filename);
>> +    } else {
>> +        error_report("Error: image_file should be given.");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (backing_filename && !strcmp(backing_filename, image_filename)) {
>> +        error_report("Error: Trying to create an image with the "
>> +                     "same backing file name as the image file name");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (!strcmp(filename, image_filename)) {
>> +        error_report("Error: Trying to create an image with the "
>> +                     "same filename as the image file name");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (header.image_filename_offset + header.image_filename_size
>> +            > ADD_COW_PAGE_SIZE * ADD_COW_DEFAULT_PAGE_SIZE) {
>> +        error_report("image_file name or backing_file name too long.");
>> +        return -ENOSPC;
>> +    }
>> +
>> +    ret = bdrv_file_open(&image_bs, image_filename, BDRV_O_RDWR);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    bdrv_delete(image_bs);
>> +
>> +    ret = bdrv_create_file(filename, NULL);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    add_cow_header_cpu_to_le(&header, &le_header);
>> +    ret = bdrv_pwrite(bs, 0, &le_header, sizeof(le_header));
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_pwrite(bs, sizeof(le_header), backing_fmt ? backing_fmt : "",
>> +        backing_fmt ? strlen(backing_fmt) : 0);
>
> The spec requires zero padding, which you don't do here.
Okay.
>
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_pwrite(bs, sizeof(le_header) + sizeof(s.backing_file_format),
>> +        image_format ? image_format : "raw",
>> +        image_format ? strlen(image_format) : sizeof("raw"));
>
> And here.

Okay.

>
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    if (backing_filename) {
>> +        ret = bdrv_pwrite(bs, header.backing_filename_offset,
>> +            backing_filename, header.backing_filename_size);
>> +        if (ret < 0) {
>> +            bdrv_delete(bs);
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    ret = bdrv_pwrite(bs, header.image_filename_offset,
>> +        image_filename, header.image_filename_size);
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_open(bs, filename, BDRV_O_RDWR | BDRV_O_NO_FLUSH, drv);
>> +    if (ret < 0) {
>> +        bdrv_delete(bs);
>> +        return ret;
>> +    }
>> +
>> +    ret = bdrv_truncate(bs, image_len);
>> +    bdrv_delete(bs);
>> +    return ret;
>> +}
>> +
>> +static int add_cow_open(BlockDriverState *bs, int flags)
>> +{
>> +    char                image_filename[ADD_COW_FILE_LEN];
>> +    char                tmp_name[ADD_COW_FILE_LEN];
>> +    BlockDriver         *image_drv = NULL;
>> +    int                 ret;
>> +    int                 sector_per_byte;
>> +    BDRVAddCowState     *s = bs->opaque;
>> +    AddCowHeader        le_header;
>> +
>> +    ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
>> +    if (ret != sizeof(s->header)) {
>
> if (ret < 0) would be more consistent with the rest of the code.
>

Okay.

>> +        goto fail;
>> +    }
>> +
>> +    add_cow_header_le_to_cpu(&le_header, &s->header);
>> +
>> +    if (le64_to_cpu(s->header.magic) != ADD_COW_MAGIC) {
>
> Isn't this one endianess conversion too much? s->header is already LE.
>
> Did you test add-cow on a big endian host?

My fault, will correct it in next version.

>
>> +        ret = -EINVAL;
>> +        goto fail;
>> +    }
>> +
>> +    if (s->header.version != ADD_COW_VERSION) {
>> +        char version[64];
>> +        snprintf(version, sizeof(version), "ADD-COW version %d",
>> +            s->header.version);
>> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
>> +            bs->device_name, "add-cow", version);
>> +        ret = -ENOTSUP;
>> +        goto fail;
>> +    }
>> +
>> +    if (s->header.features & ~ADD_COW_FEATURE_MASK) {
>> +        char buf[64];
>> +        snprintf(buf, sizeof(buf), "%" PRIx64,
>> +            s->header.features & ~ADD_COW_FEATURE_MASK);
>
> This message is a bit terse, most users will be confused with an error
> message that only consists of a hex number. Maybe better "Feature flags:
> %" PRIx64.
>

Okay.

>> +        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
>> +            bs->device_name, "add-cow", buf);
>> +        return -ENOTSUP;
>> +    }
>> +
>> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
>> +        ret = bdrv_read_string(bs->file, sizeof(s->header),
>> +            sizeof(s->backing_file_format) - 1, s->backing_file_format,
>> +            sizeof(s->backing_file_format));
>> +        if (ret < 0) {
>> +            goto fail;
>> +        }
>> +    }
>
> Would be great if this was not only read into memory, but actually
> used... It must end up in bs->backing_format in order take effect.
>
>> +
>> +    ret = bdrv_read_string(bs->file,
>> +            sizeof(s->header) + sizeof(s->image_file_format),
>> +            sizeof(s->image_file_format) - 1, s->image_file_format,
>> +            sizeof(s->image_file_format));
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>
> This one is unused, too.
>
Okay.

>> +
>> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
>> +        ret = bdrv_read_string(bs->file, s->header.backing_filename_offset,
>> +                          s->header.backing_filename_size, bs->backing_file,
>> +                          sizeof(bs->backing_file));
>> +        if (ret < 0) {
>> +            goto fail;
>> +        }
>> +    }
>> +
>> +    ret = bdrv_read_string(bs->file, s->header.image_filename_offset,
>> +                      s->header.image_filename_size, tmp_name,
>> +                      sizeof(tmp_name));
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>> +
>> +    s->image_hd = bdrv_new("");
>> +    if (path_has_protocol(image_filename)) {
>> +        pstrcpy(image_filename, sizeof(image_filename), tmp_name);
>> +    } else {
>> +        path_combine(image_filename, sizeof(image_filename),
>> +                     bs->filename, tmp_name);
>> +    }
>> +
>> +    ret = bdrv_open(s->image_hd, image_filename, flags, image_drv);
>
> image_drv is always NULL.
>
>> +    if (ret < 0) {
>> +        bdrv_delete(s->image_hd);
>> +        goto fail;
>> +    }
>> +
>> +    bs->total_sectors = bdrv_getlength(s->image_hd) >> 9;
>> +    s->cluster_size = ADD_COW_CLUSTER_SIZE;
>> +    sector_per_byte = SECTORS_PER_CLUSTER * 8;
>> +    s->bitmap_size =
>> +        (bs->total_sectors + sector_per_byte - 1) / sector_per_byte;
>> +    s->bitmap_cache =
>> +        block_cache_create(bs, ADD_COW_CACHE_SIZE, ADD_COW_CACHE_ENTRY_SIZE);
>> +
>> +    qemu_co_mutex_init(&s->lock);
>> +    return 0;
>> +fail:
>> +    if (s->bitmap_cache) {
>> +        block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
>> +    }
>> +    return ret;
>> +}
>> +
>> +static void add_cow_close(BlockDriverState *bs)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
>> +    bdrv_delete(s->image_hd);
>> +}
>> +
>> +static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
>> +{
>> +    BDRVAddCowState *s  = bs->opaque;
>> +    BlockCache *c = s->bitmap_cache;
>> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
>> +    uint8_t *table      = NULL;
>> +    uint64_t offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
>> +        + (offset_in_bitmap(sector_num) & (~(c->entry_size - 1)));
>> +    int ret = block_cache_get(bs, s->bitmap_cache, offset,
>> +        (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
>
> No matching block_cache_put?
>
>> +
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    return table[cluster_num / 8 % ADD_COW_CACHE_ENTRY_SIZE]
>> +        & (1 << (cluster_num % 8));
>> +}
>> +
>> +static coroutine_fn int add_cow_is_allocated(BlockDriverState *bs,
>> +        int64_t sector_num, int nb_sectors, int *num_same)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    int changed;
>> +
>> +    if (nb_sectors == 0) {
>> +        *num_same = 0;
>> +        return 0;
>> +    }
>> +
>> +    if (s->header.features & ADD_COW_F_All_ALLOCATED) {
>> +        *num_same = nb_sectors - 1;
>
> Why - 1?
>
>> +        return 1;
>> +    }
>> +    changed = is_allocated(bs, sector_num);
>> +
>> +    for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
>> +        if (is_allocated(bs, sector_num + *num_same) != changed) {
>> +            break;
>> +        }
>> +    }
>> +    return changed;
>> +}
>> +
>> +static int add_cow_backing_read(BlockDriverState *bs, QEMUIOVector *qiov,
>> +                  int64_t sector_num, int nb_sectors)
>> +{
>> +    int n1;
>> +    if ((sector_num + nb_sectors) <= bs->total_sectors) {
>> +        return nb_sectors;
>> +    }
>> +    if (sector_num >= bs->total_sectors) {
>> +        n1 = 0;
>> +    } else {
>> +        n1 = bs->total_sectors - sector_num;
>> +    }
>> +
>> +    qemu_iovec_memset(qiov, BDRV_SECTOR_SIZE * n1,
>> +        0, BDRV_SECTOR_SIZE * (nb_sectors - n1));
>> +
>> +    return n1;
>> +}
>> +
>> +static coroutine_fn int add_cow_co_readv(BlockDriverState *bs,
>> +    int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
>> +{
>> +    BDRVAddCowState *s  = bs->opaque;
>> +    int cur_nr_sectors;
>> +    uint64_t bytes_done = 0;
>> +    QEMUIOVector hd_qiov;
>> +    int n, n1, ret = 0;
>> +
>> +    qemu_iovec_init(&hd_qiov, qiov->niov);
>> +    qemu_co_mutex_lock(&s->lock);
>> +    while (remaining_sectors != 0) {
>> +        cur_nr_sectors = remaining_sectors;
>> +        if (add_cow_is_allocated(bs, sector_num, cur_nr_sectors, &n)) {
>> +            cur_nr_sectors = n;
>
> One of n and cur_nr_sectors is redundant.
Okay.
>
>> +            qemu_iovec_reset(&hd_qiov);
>> +            qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
>> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
>> +            qemu_co_mutex_unlock(&s->lock);
>> +            ret = bdrv_co_readv(s->image_hd, sector_num, n, &hd_qiov);
>> +            qemu_co_mutex_lock(&s->lock);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +        } else {
>> +            cur_nr_sectors = n;
>> +            if (bs->backing_hd) {
>> +                qemu_iovec_reset(&hd_qiov);
>> +                qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
>> +                            cur_nr_sectors * BDRV_SECTOR_SIZE);
>> +                n1 = add_cow_backing_read(bs->backing_hd, &hd_qiov,
>> +                    sector_num, cur_nr_sectors);
>> +                if (n1 > 0) {
>> +                    qemu_co_mutex_unlock(&s->lock);
>> +                    ret = bdrv_co_readv(bs->backing_hd, sector_num,
>> +                                        n, &hd_qiov);
>> +                    qemu_co_mutex_lock(&s->lock);
>> +                    if (ret < 0) {
>> +                        goto fail;
>> +                    }
>> +                }
>> +            } else {
>> +                qemu_iovec_memset(&hd_qiov, 0, 0,
>> +                    BDRV_SECTOR_SIZE * cur_nr_sectors);
>> +            }
>> +        }
>> +        remaining_sectors -= cur_nr_sectors;
>> +        sector_num += cur_nr_sectors;
>> +        bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
>> +    }
>> +fail:
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    qemu_iovec_destroy(&hd_qiov);
>> +    return ret;
>> +}
>> +
>> +static int coroutine_fn copy_sectors(BlockDriverState *bs,
>> +                                     int n_start, int n_end)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    QEMUIOVector qiov;
>> +    struct iovec iov;
>> +    int n, ret;
>> +
>> +    n = n_end - n_start;
>> +    if (n <= 0) {
>> +        return 0;
>> +    }
>> +
>> +    iov.iov_len = n * BDRV_SECTOR_SIZE;
>> +    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
>> +
>> +    qemu_iovec_init_external(&qiov, &iov, 1);
>> +
>> +    ret = bdrv_co_readv(bs->backing_hd, n_start, n, &qiov);
>> +    if (ret < 0) {
>> +        goto out;
>> +    }
>> +    ret = bdrv_co_writev(s->image_hd, n_start, n, &qiov);
>> +    if (ret < 0) {
>> +        goto out;
>> +    }
>> +
>> +    ret = 0;
>> +out:
>> +    qemu_vfree(iov.iov_base);
>> +    return ret;
>> +}
>> +
>> +static coroutine_fn int add_cow_co_writev(BlockDriverState *bs,
>> +        int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    BlockCache *c = s->bitmap_cache;
>> +    int ret = 0, i;
>> +    QEMUIOVector hd_qiov;
>> +    uint8_t *table;
>> +    uint64_t offset;
>> +
>> +    qemu_co_mutex_lock(&s->lock);
>> +    qemu_iovec_init(&hd_qiov, qiov->niov);
>> +    ret = bdrv_co_writev(s->image_hd,
>> +                     sector_num,
>> +                     remaining_sectors, qiov);
>> +
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>> +    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
>> +        /* Copy content of unmodified sectors */
>> +        if (!is_cluster_head(sector_num) && !is_allocated(bs, sector_num)) {
>> +            ret = copy_sectors(bs, sector_num & ~(SECTORS_PER_CLUSTER - 1),
>> +                sector_num);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +        }
>> +
>> +        if (!is_cluster_tail(sector_num + remaining_sectors - 1)
>> +            && !is_allocated(bs, sector_num + remaining_sectors - 1)) {
>> +            ret = copy_sectors(bs, sector_num + remaining_sectors,
>> +                ((sector_num + remaining_sectors) | (SECTORS_PER_CLUSTER - 1)) + 1);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +        }
>> +
>> +        for (i = sector_num / SECTORS_PER_CLUSTER;
>> +            i <= (sector_num + remaining_sectors - 1) / SECTORS_PER_CLUSTER;
>> +            i++) {
>> +            offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
>> +                + (offset_in_bitmap(i * SECTORS_PER_CLUSTER) & (~(c->entry_size - 1)));
>
> The maths in this loop looks a bit confusing, but I think it's correct.
>
>> +            ret = block_cache_get(bs, s->bitmap_cache, offset,
>> +                (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
>> +            if (ret < 0) {
>> +                goto fail;
>> +            }
>> +            if ((table[i / 8] & (1 << (i % 8))) == 0) {
>> +                table[i / 8] |= (1 << (i % 8));
>> +                block_cache_entry_mark_dirty(s->bitmap_cache, table);
>> +            }
>
> Missing block_cache_put again?
>
>> +        }
>> +    }
>> +    ret = 0;
>> +fail:
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    qemu_iovec_destroy(&hd_qiov);
>> +    return ret;
>> +}
>> +
>> +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
>> +    int ret;
>> +    uint32_t bitmap_pos = s->header.header_pages_size * ADD_COW_PAGE_SIZE;
>> +    int64_t bitmap_size =
>> +        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
>> +    bitmap_size = (bitmap_size + ADD_COW_CACHE_ENTRY_SIZE - 1)
>> +        & (~(ADD_COW_CACHE_ENTRY_SIZE - 1));
>> +
>> +    ret = bdrv_truncate(bs->file, bitmap_pos + bitmap_size);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    return 0;
>> +}
>
> So you don't truncate s->image_file? Does this work?

s->image_file should be truncated? Image file can have a larger virtual size
than backing_file, my understanding is we should not truncate image file.

>
>> +
>> +static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
>> +{
>> +    BDRVAddCowState *s = bs->opaque;
>> +    int ret;
>> +
>> +    qemu_co_mutex_lock(&s->lock);
>> +    ret = block_cache_flush(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP,
>> +        ADD_COW_CACHE_ENTRY_SIZE);
>> +    qemu_co_mutex_unlock(&s->lock);
>> +    return ret;
>> +}
>
> What about flushing s->image_file?
>
>> +
>> +static QEMUOptionParameter add_cow_create_options[] = {
>> +    {
>> +        .name = BLOCK_OPT_SIZE,
>> +        .type = OPT_SIZE,
>> +        .help = "Virtual disk size"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_BACKING_FILE,
>> +        .type = OPT_STRING,
>> +        .help = "File name of a base image"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_BACKING_FMT,
>> +        .type = OPT_STRING,
>> +        .help = "Image format of the base image"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_IMAGE_FILE,
>> +        .type = OPT_STRING,
>> +        .help = "File name of a image file"
>> +    },
>> +    {
>> +        .name = BLOCK_OPT_IMAGE_FORMAT,
>> +        .type = OPT_STRING,
>> +        .help = "Image format of the image file"
>> +    },
>> +    { NULL }
>> +};
>> +
>> +static BlockDriver bdrv_add_cow = {
>> +    .format_name                = "add-cow",
>> +    .instance_size              = sizeof(BDRVAddCowState),
>> +    .bdrv_probe                 = add_cow_probe,
>> +    .bdrv_open                  = add_cow_open,
>> +    .bdrv_close                 = add_cow_close,
>> +    .bdrv_create                = add_cow_create,
>> +    .bdrv_co_readv              = add_cow_co_readv,
>> +    .bdrv_co_writev             = add_cow_co_writev,
>> +    .bdrv_truncate              = bdrv_add_cow_truncate,
>> +    .bdrv_co_is_allocated       = add_cow_is_allocated,
>> +
>> +    .create_options             = add_cow_create_options,
>> +    .bdrv_co_flush_to_os        = add_cow_co_flush,
>> +};
>> +
>> +static void bdrv_add_cow_init(void)
>> +{
>> +    bdrv_register(&bdrv_add_cow);
>> +}
>> +
>> +block_init(bdrv_add_cow_init);
>> diff --git a/block/add-cow.h b/block/add-cow.h
>> new file mode 100644
>> index 0000000..f058376
>> --- /dev/null
>> +++ b/block/add-cow.h
>> @@ -0,0 +1,85 @@
>> +/*
>> + * QEMU ADD-COW Disk Format
>> + *
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Authors:
>> + *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef BLOCK_ADD_COW_H
>> +#define BLOCK_ADD_COW_H
>> +#include "block-cache.h"
>> +
>> +enum {
>> +    ADD_COW_F_All_ALLOCATED     = 0X01,
>> +    ADD_COW_FEATURE_MASK        = ADD_COW_F_All_ALLOCATED,
>> +
>> +    ADD_COW_MAGIC = (((uint64_t)'A' << 56) | ((uint64_t)'D' << 48) | \
>> +                    ((uint64_t)'D' << 40) | ((uint64_t)'_' << 32) | \
>> +                    ((uint64_t)'C' << 24) | ((uint64_t)'O' << 16) | \
>> +                    ((uint64_t)'W' << 8) | 0xFF),
>> +    ADD_COW_VERSION             = 1,
>> +    ADD_COW_FILE_LEN            = 1024,
>> +    ADD_COW_CACHE_SIZE          = 16,
>> +    ADD_COW_CACHE_ENTRY_SIZE    = 65536,
>> +    ADD_COW_CLUSTER_SIZE        = 65536,
>> +    SECTORS_PER_CLUSTER         = (ADD_COW_CLUSTER_SIZE / BDRV_SECTOR_SIZE),
>> +    ADD_COW_PAGE_SIZE           = 4096,
>> +    ADD_COW_DEFAULT_PAGE_SIZE   = 1,
>> +};
>> +
>> +typedef struct AddCowHeader {
>> +    uint64_t        magic;
>> +    uint32_t        version;
>> +
>> +    uint32_t        backing_filename_offset;
>> +    uint32_t        backing_filename_size;
>> +
>> +    uint32_t        image_filename_offset;
>> +    uint32_t        image_filename_size;
>> +
>> +    uint64_t        features;
>> +    uint64_t        optional_features;
>> +    uint32_t        header_pages_size;
>> +} QEMU_PACKED AddCowHeader;
>
> Why aren't backing/image_file_format part of the header here? They are
> in the spec. It would also simplify some offset calculation code.
>

Anthony said "It's far better to shrink the size of the header and use
an offset/len
pointer to the backing file string.  Limiting backing files to 1023 is
unacceptable"

http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg04110.html

So I use offset  and length instead of using string directly.

>> +
>> +typedef struct BDRVAddCowState {
>> +    BlockDriverState    *image_hd;
>> +    CoMutex             lock;
>> +    int                 cluster_size;
>> +    BlockCache         *bitmap_cache;
>> +    uint64_t            bitmap_size;
>> +    AddCowHeader        header;
>> +    char                backing_file_format[16];
>> +    char                image_file_format[16];
>> +} BDRVAddCowState;
>> +
>> +/* Convert sector_num to offset in bitmap */
>> +static inline int64_t offset_in_bitmap(int64_t sector_num)
>> +{
>> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
>> +    return cluster_num / 8;
>> +}
>> +
>> +static inline bool is_cluster_head(int64_t sector_num)
>> +{
>> +    return sector_num % SECTORS_PER_CLUSTER == 0;
>> +}
>> +
>> +static inline bool is_cluster_tail(int64_t sector_num)
>> +{
>> +    return (sector_num + 1) % SECTORS_PER_CLUSTER == 0;
>> +}
>> +
>> +BlockCache *add_cow_cache_create(BlockDriverState *bs, int num_tables);
>> +int add_cow_cache_destroy(BlockDriverState *bs, BlockCache *c);
>> +void add_cow_cache_entry_mark_dirty(BlockCache *c, void *table);
>> +int add_cow_cache_get(BlockDriverState *bs, BlockCache *c, uint64_t offset,
>> +    void **table);
>> +int add_cow_cache_flush(BlockDriverState *bs, BlockCache *c);
>
> These functions don't really exist any more, do they?

Right, sorry.

>
> Kevin
>

Thank you, Kevin.
Kevin Wolf - Sept. 12, 2012, 7:50 a.m.
Am 12.09.2012 09:28, schrieb Dong Xu Wang:
>>> +static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
>>> +{
>>> +    BDRVAddCowState *s  = bs->opaque;
>>> +    BlockCache *c = s->bitmap_cache;
>>> +    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
>>> +    uint8_t *table      = NULL;
>>> +    uint64_t offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
>>> +        + (offset_in_bitmap(sector_num) & (~(c->entry_size - 1)));
>>> +    int ret = block_cache_get(bs, s->bitmap_cache, offset,
>>> +        (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
>>
>> No matching block_cache_put?
>>
>>> +
>>> +    if (ret < 0) {
>>> +        return ret;
>>> +    }
>>> +    return table[cluster_num / 8 % ADD_COW_CACHE_ENTRY_SIZE]
>>> +        & (1 << (cluster_num % 8));
>>> +}
>>> +
>>> +static coroutine_fn int add_cow_is_allocated(BlockDriverState *bs,
>>> +        int64_t sector_num, int nb_sectors, int *num_same)
>>> +{
>>> +    BDRVAddCowState *s = bs->opaque;
>>> +    int changed;
>>> +
>>> +    if (nb_sectors == 0) {
>>> +        *num_same = 0;
>>> +        return 0;
>>> +    }
>>> +
>>> +    if (s->header.features & ADD_COW_F_All_ALLOCATED) {
>>> +        *num_same = nb_sectors - 1;
>>
>> Why - 1?
>>
>>> +        return 1;
>>> +    }
>>> +    changed = is_allocated(bs, sector_num);
>>> +
>>> +    for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
>>> +        if (is_allocated(bs, sector_num + *num_same) != changed) {
>>> +            break;
>>> +        }
>>> +    }
>>> +    return changed;
>>> +}

>>> +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
>>> +{
>>> +    BDRVAddCowState *s = bs->opaque;
>>> +    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
>>> +    int ret;
>>> +    uint32_t bitmap_pos = s->header.header_pages_size * ADD_COW_PAGE_SIZE;
>>> +    int64_t bitmap_size =
>>> +        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
>>> +    bitmap_size = (bitmap_size + ADD_COW_CACHE_ENTRY_SIZE - 1)
>>> +        & (~(ADD_COW_CACHE_ENTRY_SIZE - 1));
>>> +
>>> +    ret = bdrv_truncate(bs->file, bitmap_pos + bitmap_size);
>>> +    if (ret < 0) {
>>> +        return ret;
>>> +    }
>>> +    return 0;
>>> +}
>>
>> So you don't truncate s->image_file? Does this work?
> 
> s->image_file should be truncated? Image file can have a larger virtual size
> than backing_file, my understanding is we should not truncate image file.

I'm talking about s->image_hd, not bs->backing_hd. You are right that
the backing file should not be changed. But the associated raw image
should be resized, shouldn't it?

>>> +static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
>>> +{
>>> +    BDRVAddCowState *s = bs->opaque;
>>> +    int ret;
>>> +
>>> +    qemu_co_mutex_lock(&s->lock);
>>> +    ret = block_cache_flush(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP,
>>> +        ADD_COW_CACHE_ENTRY_SIZE);
>>> +    qemu_co_mutex_unlock(&s->lock);
>>> +    return ret;
>>> +}
>>
>> What about flushing s->image_file?

>>> +typedef struct AddCowHeader {
>>> +    uint64_t        magic;
>>> +    uint32_t        version;
>>> +
>>> +    uint32_t        backing_filename_offset;
>>> +    uint32_t        backing_filename_size;
>>> +
>>> +    uint32_t        image_filename_offset;
>>> +    uint32_t        image_filename_size;
>>> +
>>> +    uint64_t        features;
>>> +    uint64_t        optional_features;
>>> +    uint32_t        header_pages_size;
>>> +} QEMU_PACKED AddCowHeader;
>>
>> Why aren't backing/image_file_format part of the header here? They are
>> in the spec. It would also simplify some offset calculation code.
>>
> 
> Anthony said "It's far better to shrink the size of the header and use
> an offset/len
> pointer to the backing file string.  Limiting backing files to 1023 is
> unacceptable"
> 
> http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg04110.html
> 
> So I use offset  and length instead of using string directly.

I'm talking about the format, not the path.

Kevin

Patch

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 23bdfc8..7ed5051 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -2,6 +2,7 @@  block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
+block-obj-y += add-cow.o
 block-obj-y += block-cache.o
 block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
 block-obj-y += stream.o
diff --git a/block/add-cow.c b/block/add-cow.c
new file mode 100644
index 0000000..d4711d5
--- /dev/null
+++ b/block/add-cow.c
@@ -0,0 +1,613 @@ 
+/*
+ * QEMU ADD-COW Disk Format
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "block_int.h"
+#include "module.h"
+#include "add-cow.h"
+
+static void add_cow_header_le_to_cpu(const AddCowHeader *le, AddCowHeader *cpu)
+{
+    cpu->magic                      = le64_to_cpu(le->magic);
+    cpu->version                    = le32_to_cpu(le->version);
+
+    cpu->backing_filename_offset    = le32_to_cpu(le->backing_filename_offset);
+    cpu->backing_filename_size      = le32_to_cpu(le->backing_filename_size);
+
+    cpu->image_filename_offset      = le32_to_cpu(le->image_filename_offset);
+    cpu->image_filename_size        = le32_to_cpu(le->image_filename_size);
+
+    cpu->features                   = le64_to_cpu(le->features);
+    cpu->optional_features          = le64_to_cpu(le->optional_features);
+    cpu->header_pages_size          = le32_to_cpu(le->header_pages_size);
+}
+
+static void add_cow_header_cpu_to_le(const AddCowHeader *cpu, AddCowHeader *le)
+{
+    le->magic                       = cpu_to_le64(cpu->magic);
+    le->version                     = cpu_to_le32(cpu->version);
+
+    le->backing_filename_offset     = cpu_to_le32(cpu->backing_filename_offset);
+    le->backing_filename_size       = cpu_to_le32(cpu->backing_filename_size);
+
+    le->image_filename_offset       = cpu_to_le32(cpu->image_filename_offset);
+    le->image_filename_size         = cpu_to_le32(cpu->image_filename_size);
+
+    le->features                    = cpu_to_le64(cpu->features);
+    le->optional_features           = cpu_to_le64(cpu->optional_features);
+    le->header_pages_size           = cpu_to_le32(cpu->header_pages_size);
+}
+
+static int add_cow_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    const AddCowHeader *header = (const AddCowHeader *)buf;
+
+    if (le64_to_cpu(header->magic) == ADD_COW_MAGIC &&
+        le32_to_cpu(header->version) == ADD_COW_VERSION) {
+        return 100;
+    } else {
+        return 0;
+    }
+}
+
+static int add_cow_create(const char *filename, QEMUOptionParameter *options)
+{
+    AddCowHeader header = {
+        .magic = ADD_COW_MAGIC,
+        .version = ADD_COW_VERSION,
+        .features = 0,
+        .optional_features = 0,
+        .header_pages_size = ADD_COW_DEFAULT_PAGE_SIZE,
+    };
+    AddCowHeader le_header;
+    int64_t image_len = 0;
+    const char *backing_filename = NULL;
+    const char *backing_fmt = NULL;
+    const char *image_filename = NULL;
+    const char *image_format = NULL;
+    BlockDriverState *bs, *image_bs = NULL, *backing_bs = NULL;
+    BlockDriver *drv = bdrv_find_format("add-cow");
+    BDRVAddCowState s;
+    int ret;
+
+    while (options && options->name) {
+        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+            image_len = options->value.n;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
+            backing_filename = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
+            backing_fmt = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FILE)) {
+            image_filename = options->value.s;
+        } else if (!strcmp(options->name, BLOCK_OPT_IMAGE_FORMAT)) {
+            image_format = options->value.s;
+        }
+        options++;
+    }
+
+    if (backing_filename) {
+        header.backing_filename_offset = sizeof(header)
+            + sizeof(s.backing_file_format) + sizeof(s.image_file_format);
+        header.backing_filename_size = strlen(backing_filename);
+
+        if (!backing_fmt) {
+            backing_bs = bdrv_new("image");
+            ret = bdrv_open(backing_bs, backing_filename, BDRV_O_RDWR
+                    | BDRV_O_CACHE_WB, NULL);
+            if (ret < 0) {
+                return ret;
+            }
+            backing_fmt = bdrv_get_format_name(backing_bs);
+            bdrv_delete(backing_bs);
+        }
+    } else {
+        header.features |= ADD_COW_F_All_ALLOCATED;
+    }
+
+    if (image_filename) {
+        header.image_filename_offset =
+            sizeof(header) + sizeof(s.backing_file_format)
+                + sizeof(s.image_file_format) + header.backing_filename_size;
+        header.image_filename_size = strlen(image_filename);
+    } else {
+        error_report("Error: image_file should be given.");
+        return -EINVAL;
+    }
+
+    if (backing_filename && !strcmp(backing_filename, image_filename)) {
+        error_report("Error: Trying to create an image with the "
+                     "same backing file name as the image file name");
+        return -EINVAL;
+    }
+
+    if (!strcmp(filename, image_filename)) {
+        error_report("Error: Trying to create an image with the "
+                     "same filename as the image file name");
+        return -EINVAL;
+    }
+
+    if (header.image_filename_offset + header.image_filename_size
+            > ADD_COW_PAGE_SIZE * ADD_COW_DEFAULT_PAGE_SIZE) {
+        error_report("image_file name or backing_file name too long.");
+        return -ENOSPC;
+    }
+
+    ret = bdrv_file_open(&image_bs, image_filename, BDRV_O_RDWR);
+    if (ret < 0) {
+        return ret;
+    }
+    bdrv_delete(image_bs);
+
+    ret = bdrv_create_file(filename, NULL);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = bdrv_file_open(&bs, filename, BDRV_O_RDWR);
+    if (ret < 0) {
+        return ret;
+    }
+    add_cow_header_cpu_to_le(&header, &le_header);
+    ret = bdrv_pwrite(bs, 0, &le_header, sizeof(le_header));
+    if (ret < 0) {
+        bdrv_delete(bs);
+        return ret;
+    }
+
+    ret = bdrv_pwrite(bs, sizeof(le_header), backing_fmt ? backing_fmt : "",
+        backing_fmt ? strlen(backing_fmt) : 0);
+    if (ret < 0) {
+        bdrv_delete(bs);
+        return ret;
+    }
+
+    ret = bdrv_pwrite(bs, sizeof(le_header) + sizeof(s.backing_file_format),
+        image_format ? image_format : "raw",
+        image_format ? strlen(image_format) : sizeof("raw"));
+    if (ret < 0) {
+        bdrv_delete(bs);
+        return ret;
+    }
+
+    if (backing_filename) {
+        ret = bdrv_pwrite(bs, header.backing_filename_offset,
+            backing_filename, header.backing_filename_size);
+        if (ret < 0) {
+            bdrv_delete(bs);
+            return ret;
+        }
+    }
+
+    ret = bdrv_pwrite(bs, header.image_filename_offset,
+        image_filename, header.image_filename_size);
+    if (ret < 0) {
+        bdrv_delete(bs);
+        return ret;
+    }
+
+    ret = bdrv_open(bs, filename, BDRV_O_RDWR | BDRV_O_NO_FLUSH, drv);
+    if (ret < 0) {
+        bdrv_delete(bs);
+        return ret;
+    }
+
+    ret = bdrv_truncate(bs, image_len);
+    bdrv_delete(bs);
+    return ret;
+}
+
+static int add_cow_open(BlockDriverState *bs, int flags)
+{
+    char                image_filename[ADD_COW_FILE_LEN];
+    char                tmp_name[ADD_COW_FILE_LEN];
+    BlockDriver         *image_drv = NULL;
+    int                 ret;
+    int                 sector_per_byte;
+    BDRVAddCowState     *s = bs->opaque;
+    AddCowHeader        le_header;
+
+    ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
+    if (ret != sizeof(s->header)) {
+        goto fail;
+    }
+
+    add_cow_header_le_to_cpu(&le_header, &s->header);
+
+    if (le64_to_cpu(s->header.magic) != ADD_COW_MAGIC) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    if (s->header.version != ADD_COW_VERSION) {
+        char version[64];
+        snprintf(version, sizeof(version), "ADD-COW version %d",
+            s->header.version);
+        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+            bs->device_name, "add-cow", version);
+        ret = -ENOTSUP;
+        goto fail;
+    }
+
+    if (s->header.features & ~ADD_COW_FEATURE_MASK) {
+        char buf[64];
+        snprintf(buf, sizeof(buf), "%" PRIx64,
+            s->header.features & ~ADD_COW_FEATURE_MASK);
+        qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+            bs->device_name, "add-cow", buf);
+        return -ENOTSUP;
+    }
+
+    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
+        ret = bdrv_read_string(bs->file, sizeof(s->header),
+            sizeof(s->backing_file_format) - 1, s->backing_file_format,
+            sizeof(s->backing_file_format));
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
+    ret = bdrv_read_string(bs->file,
+            sizeof(s->header) + sizeof(s->image_file_format),
+            sizeof(s->image_file_format) - 1, s->image_file_format,
+            sizeof(s->image_file_format));
+    if (ret < 0) {
+        goto fail;
+    }
+
+    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
+        ret = bdrv_read_string(bs->file, s->header.backing_filename_offset,
+                          s->header.backing_filename_size, bs->backing_file,
+                          sizeof(bs->backing_file));
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
+    ret = bdrv_read_string(bs->file, s->header.image_filename_offset,
+                      s->header.image_filename_size, tmp_name,
+                      sizeof(tmp_name));
+    if (ret < 0) {
+        goto fail;
+    }
+
+    s->image_hd = bdrv_new("");
+    if (path_has_protocol(image_filename)) {
+        pstrcpy(image_filename, sizeof(image_filename), tmp_name);
+    } else {
+        path_combine(image_filename, sizeof(image_filename),
+                     bs->filename, tmp_name);
+    }
+
+    ret = bdrv_open(s->image_hd, image_filename, flags, image_drv);
+    if (ret < 0) {
+        bdrv_delete(s->image_hd);
+        goto fail;
+    }
+
+    bs->total_sectors = bdrv_getlength(s->image_hd) >> 9;
+    s->cluster_size = ADD_COW_CLUSTER_SIZE;
+    sector_per_byte = SECTORS_PER_CLUSTER * 8;
+    s->bitmap_size =
+        (bs->total_sectors + sector_per_byte - 1) / sector_per_byte;
+    s->bitmap_cache =
+        block_cache_create(bs, ADD_COW_CACHE_SIZE, ADD_COW_CACHE_ENTRY_SIZE);
+
+    qemu_co_mutex_init(&s->lock);
+    return 0;
+fail:
+    if (s->bitmap_cache) {
+        block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
+    }
+    return ret;
+}
+
+static void add_cow_close(BlockDriverState *bs)
+{
+    BDRVAddCowState *s = bs->opaque;
+    block_cache_destroy(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP);
+    bdrv_delete(s->image_hd);
+}
+
+static bool is_allocated(BlockDriverState *bs, int64_t sector_num)
+{
+    BDRVAddCowState *s  = bs->opaque;
+    BlockCache *c = s->bitmap_cache;
+    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
+    uint8_t *table      = NULL;
+    uint64_t offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
+        + (offset_in_bitmap(sector_num) & (~(c->entry_size - 1)));
+    int ret = block_cache_get(bs, s->bitmap_cache, offset,
+        (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
+
+    if (ret < 0) {
+        return ret;
+    }
+    return table[cluster_num / 8 % ADD_COW_CACHE_ENTRY_SIZE]
+        & (1 << (cluster_num % 8));
+}
+
+static coroutine_fn int add_cow_is_allocated(BlockDriverState *bs,
+        int64_t sector_num, int nb_sectors, int *num_same)
+{
+    BDRVAddCowState *s = bs->opaque;
+    int changed;
+
+    if (nb_sectors == 0) {
+        *num_same = 0;
+        return 0;
+    }
+
+    if (s->header.features & ADD_COW_F_All_ALLOCATED) {
+        *num_same = nb_sectors - 1;
+        return 1;
+    }
+    changed = is_allocated(bs, sector_num);
+
+    for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
+        if (is_allocated(bs, sector_num + *num_same) != changed) {
+            break;
+        }
+    }
+    return changed;
+}
+
+static int add_cow_backing_read(BlockDriverState *bs, QEMUIOVector *qiov,
+                  int64_t sector_num, int nb_sectors)
+{
+    int n1;
+    if ((sector_num + nb_sectors) <= bs->total_sectors) {
+        return nb_sectors;
+    }
+    if (sector_num >= bs->total_sectors) {
+        n1 = 0;
+    } else {
+        n1 = bs->total_sectors - sector_num;
+    }
+
+    qemu_iovec_memset(qiov, BDRV_SECTOR_SIZE * n1,
+        0, BDRV_SECTOR_SIZE * (nb_sectors - n1));
+
+    return n1;
+}
+
+static coroutine_fn int add_cow_co_readv(BlockDriverState *bs,
+    int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
+{
+    BDRVAddCowState *s  = bs->opaque;
+    int cur_nr_sectors;
+    uint64_t bytes_done = 0;
+    QEMUIOVector hd_qiov;
+    int n, n1, ret = 0;
+
+    qemu_iovec_init(&hd_qiov, qiov->niov);
+    qemu_co_mutex_lock(&s->lock);
+    while (remaining_sectors != 0) {
+        cur_nr_sectors = remaining_sectors;
+        if (add_cow_is_allocated(bs, sector_num, cur_nr_sectors, &n)) {
+            cur_nr_sectors = n;
+            qemu_iovec_reset(&hd_qiov);
+            qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
+                            cur_nr_sectors * BDRV_SECTOR_SIZE);
+            qemu_co_mutex_unlock(&s->lock);
+            ret = bdrv_co_readv(s->image_hd, sector_num, n, &hd_qiov);
+            qemu_co_mutex_lock(&s->lock);
+            if (ret < 0) {
+                goto fail;
+            }
+        } else {
+            cur_nr_sectors = n;
+            if (bs->backing_hd) {
+                qemu_iovec_reset(&hd_qiov);
+                qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
+                            cur_nr_sectors * BDRV_SECTOR_SIZE);
+                n1 = add_cow_backing_read(bs->backing_hd, &hd_qiov,
+                    sector_num, cur_nr_sectors);
+                if (n1 > 0) {
+                    qemu_co_mutex_unlock(&s->lock);
+                    ret = bdrv_co_readv(bs->backing_hd, sector_num,
+                                        n, &hd_qiov);
+                    qemu_co_mutex_lock(&s->lock);
+                    if (ret < 0) {
+                        goto fail;
+                    }
+                }
+            } else {
+                qemu_iovec_memset(&hd_qiov, 0, 0,
+                    BDRV_SECTOR_SIZE * cur_nr_sectors);
+            }
+        }
+        remaining_sectors -= cur_nr_sectors;
+        sector_num += cur_nr_sectors;
+        bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
+    }
+fail:
+    qemu_co_mutex_unlock(&s->lock);
+    qemu_iovec_destroy(&hd_qiov);
+    return ret;
+}
+
+static int coroutine_fn copy_sectors(BlockDriverState *bs,
+                                     int n_start, int n_end)
+{
+    BDRVAddCowState *s = bs->opaque;
+    QEMUIOVector qiov;
+    struct iovec iov;
+    int n, ret;
+
+    n = n_end - n_start;
+    if (n <= 0) {
+        return 0;
+    }
+
+    iov.iov_len = n * BDRV_SECTOR_SIZE;
+    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
+
+    qemu_iovec_init_external(&qiov, &iov, 1);
+
+    ret = bdrv_co_readv(bs->backing_hd, n_start, n, &qiov);
+    if (ret < 0) {
+        goto out;
+    }
+    ret = bdrv_co_writev(s->image_hd, n_start, n, &qiov);
+    if (ret < 0) {
+        goto out;
+    }
+
+    ret = 0;
+out:
+    qemu_vfree(iov.iov_base);
+    return ret;
+}
+
+static coroutine_fn int add_cow_co_writev(BlockDriverState *bs,
+        int64_t sector_num, int remaining_sectors, QEMUIOVector *qiov)
+{
+    BDRVAddCowState *s = bs->opaque;
+    BlockCache *c = s->bitmap_cache;
+    int ret = 0, i;
+    QEMUIOVector hd_qiov;
+    uint8_t *table;
+    uint64_t offset;
+
+    qemu_co_mutex_lock(&s->lock);
+    qemu_iovec_init(&hd_qiov, qiov->niov);
+    ret = bdrv_co_writev(s->image_hd,
+                     sector_num,
+                     remaining_sectors, qiov);
+
+    if (ret < 0) {
+        goto fail;
+    }
+    if ((s->header.features & ADD_COW_F_All_ALLOCATED) == 0) {
+        /* Copy content of unmodified sectors */
+        if (!is_cluster_head(sector_num) && !is_allocated(bs, sector_num)) {
+            ret = copy_sectors(bs, sector_num & ~(SECTORS_PER_CLUSTER - 1),
+                sector_num);
+            if (ret < 0) {
+                goto fail;
+            }
+        }
+
+        if (!is_cluster_tail(sector_num + remaining_sectors - 1)
+            && !is_allocated(bs, sector_num + remaining_sectors - 1)) {
+            ret = copy_sectors(bs, sector_num + remaining_sectors,
+                ((sector_num + remaining_sectors) | (SECTORS_PER_CLUSTER - 1)) + 1);
+            if (ret < 0) {
+                goto fail;
+            }
+        }
+
+        for (i = sector_num / SECTORS_PER_CLUSTER;
+            i <= (sector_num + remaining_sectors - 1) / SECTORS_PER_CLUSTER;
+            i++) {
+            offset = ADD_COW_PAGE_SIZE * s->header.header_pages_size
+                + (offset_in_bitmap(i * SECTORS_PER_CLUSTER) & (~(c->entry_size - 1)));
+            ret = block_cache_get(bs, s->bitmap_cache, offset,
+                (void **)&table, BLOCK_TABLE_BITMAP, ADD_COW_CACHE_ENTRY_SIZE);
+            if (ret < 0) {
+                goto fail;
+            }
+            if ((table[i / 8] & (1 << (i % 8))) == 0) {
+                table[i / 8] |= (1 << (i % 8));
+                block_cache_entry_mark_dirty(s->bitmap_cache, table);
+            }
+        }
+    }
+    ret = 0;
+fail:
+    qemu_co_mutex_unlock(&s->lock);
+    qemu_iovec_destroy(&hd_qiov);
+    return ret;
+}
+
+static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
+{
+    BDRVAddCowState *s = bs->opaque;
+    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
+    int ret;
+    uint32_t bitmap_pos = s->header.header_pages_size * ADD_COW_PAGE_SIZE;
+    int64_t bitmap_size =
+        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
+    bitmap_size = (bitmap_size + ADD_COW_CACHE_ENTRY_SIZE - 1)
+        & (~(ADD_COW_CACHE_ENTRY_SIZE - 1));
+
+    ret = bdrv_truncate(bs->file, bitmap_pos + bitmap_size);
+    if (ret < 0) {
+        return ret;
+    }
+    return 0;
+}
+
+static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
+{
+    BDRVAddCowState *s = bs->opaque;
+    int ret;
+
+    qemu_co_mutex_lock(&s->lock);
+    ret = block_cache_flush(bs, s->bitmap_cache, BLOCK_TABLE_BITMAP,
+        ADD_COW_CACHE_ENTRY_SIZE);
+    qemu_co_mutex_unlock(&s->lock);
+    return ret;
+}
+
+static QEMUOptionParameter add_cow_create_options[] = {
+    {
+        .name = BLOCK_OPT_SIZE,
+        .type = OPT_SIZE,
+        .help = "Virtual disk size"
+    },
+    {
+        .name = BLOCK_OPT_BACKING_FILE,
+        .type = OPT_STRING,
+        .help = "File name of a base image"
+    },
+    {
+        .name = BLOCK_OPT_BACKING_FMT,
+        .type = OPT_STRING,
+        .help = "Image format of the base image"
+    },
+    {
+        .name = BLOCK_OPT_IMAGE_FILE,
+        .type = OPT_STRING,
+        .help = "File name of a image file"
+    },
+    {
+        .name = BLOCK_OPT_IMAGE_FORMAT,
+        .type = OPT_STRING,
+        .help = "Image format of the image file"
+    },
+    { NULL }
+};
+
+static BlockDriver bdrv_add_cow = {
+    .format_name                = "add-cow",
+    .instance_size              = sizeof(BDRVAddCowState),
+    .bdrv_probe                 = add_cow_probe,
+    .bdrv_open                  = add_cow_open,
+    .bdrv_close                 = add_cow_close,
+    .bdrv_create                = add_cow_create,
+    .bdrv_co_readv              = add_cow_co_readv,
+    .bdrv_co_writev             = add_cow_co_writev,
+    .bdrv_truncate              = bdrv_add_cow_truncate,
+    .bdrv_co_is_allocated       = add_cow_is_allocated,
+
+    .create_options             = add_cow_create_options,
+    .bdrv_co_flush_to_os        = add_cow_co_flush,
+};
+
+static void bdrv_add_cow_init(void)
+{
+    bdrv_register(&bdrv_add_cow);
+}
+
+block_init(bdrv_add_cow_init);
diff --git a/block/add-cow.h b/block/add-cow.h
new file mode 100644
index 0000000..f058376
--- /dev/null
+++ b/block/add-cow.h
@@ -0,0 +1,85 @@ 
+/*
+ * QEMU ADD-COW Disk Format
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef BLOCK_ADD_COW_H
+#define BLOCK_ADD_COW_H
+#include "block-cache.h"
+
+enum {
+    ADD_COW_F_All_ALLOCATED     = 0X01,
+    ADD_COW_FEATURE_MASK        = ADD_COW_F_All_ALLOCATED,
+
+    ADD_COW_MAGIC = (((uint64_t)'A' << 56) | ((uint64_t)'D' << 48) | \
+                    ((uint64_t)'D' << 40) | ((uint64_t)'_' << 32) | \
+                    ((uint64_t)'C' << 24) | ((uint64_t)'O' << 16) | \
+                    ((uint64_t)'W' << 8) | 0xFF),
+    ADD_COW_VERSION             = 1,
+    ADD_COW_FILE_LEN            = 1024,
+    ADD_COW_CACHE_SIZE          = 16,
+    ADD_COW_CACHE_ENTRY_SIZE    = 65536,
+    ADD_COW_CLUSTER_SIZE        = 65536,
+    SECTORS_PER_CLUSTER         = (ADD_COW_CLUSTER_SIZE / BDRV_SECTOR_SIZE),
+    ADD_COW_PAGE_SIZE           = 4096,
+    ADD_COW_DEFAULT_PAGE_SIZE   = 1,
+};
+
+typedef struct AddCowHeader {
+    uint64_t        magic;
+    uint32_t        version;
+
+    uint32_t        backing_filename_offset;
+    uint32_t        backing_filename_size;
+
+    uint32_t        image_filename_offset;
+    uint32_t        image_filename_size;
+
+    uint64_t        features;
+    uint64_t        optional_features;
+    uint32_t        header_pages_size;
+} QEMU_PACKED AddCowHeader;
+
+typedef struct BDRVAddCowState {
+    BlockDriverState    *image_hd;
+    CoMutex             lock;
+    int                 cluster_size;
+    BlockCache         *bitmap_cache;
+    uint64_t            bitmap_size;
+    AddCowHeader        header;
+    char                backing_file_format[16];
+    char                image_file_format[16];
+} BDRVAddCowState;
+
+/* Convert sector_num to offset in bitmap */
+static inline int64_t offset_in_bitmap(int64_t sector_num)
+{
+    int64_t cluster_num = sector_num / SECTORS_PER_CLUSTER;
+    return cluster_num / 8;
+}
+
+static inline bool is_cluster_head(int64_t sector_num)
+{
+    return sector_num % SECTORS_PER_CLUSTER == 0;
+}
+
+static inline bool is_cluster_tail(int64_t sector_num)
+{
+    return (sector_num + 1) % SECTORS_PER_CLUSTER == 0;
+}
+
+BlockCache *add_cow_cache_create(BlockDriverState *bs, int num_tables);
+int add_cow_cache_destroy(BlockDriverState *bs, BlockCache *c);
+void add_cow_cache_entry_mark_dirty(BlockCache *c, void *table);
+int add_cow_cache_get(BlockDriverState *bs, BlockCache *c, uint64_t offset,
+    void **table);
+int add_cow_cache_flush(BlockDriverState *bs, BlockCache *c);
+#endif
diff --git a/block_int.h b/block_int.h
index 6c1d9ca..67954ec 100644
--- a/block_int.h
+++ b/block_int.h
@@ -53,6 +53,8 @@ 
 #define BLOCK_OPT_SUBFMT            "subformat"
 #define BLOCK_OPT_COMPAT_LEVEL      "compat"
 #define BLOCK_OPT_LAZY_REFCOUNTS    "lazy_refcounts"
+#define BLOCK_OPT_IMAGE_FILE        "image_file"
+#define BLOCK_OPT_IMAGE_FORMAT      "image_format"
 
 typedef struct BdrvTrackedRequest BdrvTrackedRequest;