Am 10.09.2011 02:54, schrieb Dong Xu Wang:
> 于Fri 09 Sep 2011 10:27:26 PM CST,Kevin Wolf写到:
>> Am 09.09.2011 07:48, schrieb Dong Xu Wang:
>>> As raw file format does not support backing_file and copy on write feature,
>>> so
>>> I add COW to it to support backing_file option. I store dirty bitmap in an
>>> add-cow file. When executed, it looks like this:
>>> qemu-img create -f add-cow -o backing_file=ubuntu.img,image_file=test.img
>>> test.add-cow
>>> qemu -drive if=virtio,file=test.add-cow -m 1024
>>>
>>> (test.img is a raw format file; test.add-cow stores bitmap)
>>>
>>> Signed-off-by: Dong Xu Wang <wdon...@linux.vnet.ibm.com>
>>
>> You should not make any changes to generic code, except maybe add
>> something to bdrv_get_info(). In particular you shouldn't need to touch
>> bdrv_open() or bdrv_create() at all.
>>
>> The one required change in the approach for this to work is that you
>> shouldn't view raw+add_cow as a unit, but add_cow should be treated as
>> something separate that happens to be stacked on a raw file (which is
>> created separately).
>>
>> Then you can do almost everything in block/add-cow.c.
>>
>>> ---
>>> Makefile.objs | 1 +
>>> block.c | 83 ++++++++++-
>>> block.h | 2 +
>>> block/add-cow.c | 456
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> block_int.h | 6 +
>>> qemu-img.c | 10 ++
>>> 6 files changed, 555 insertions(+), 3 deletions(-)
>>> create mode 100644 block/add-cow.c
>>>
>>> diff --git a/Makefile.objs b/Makefile.objs
>>> index 26b885b..1402f9f 100644
>>> --- a/Makefile.objs
>>> +++ b/Makefile.objs
>>> @@ -31,6 +31,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>>>
>>> block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o
>>> vpc.o vvfat.o
>>> block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o
>>> qcow2-snapshot.o qcow2-cache.o
>>> +block-nested-y += add-cow.o
>>> block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o
>>> qed-cluster.o
>>> block-nested-y += qed-check.o
>>> block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>>> diff --git a/block.c b/block.c
>>> index a8c789a..c797cfc 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -369,7 +369,7 @@ static int find_image_format(const char *filename,
>>> BlockDriver **pdrv)
>>> {
>>> int ret, score, score_max;
>>> BlockDriver *drv1, *drv;
>>> - uint8_t buf[2048];
>>> + uint8_t buf[4096];
>>> BlockDriverState *bs;
>>
>> What's the reason for this change?
>>
> The size of add_cow_header in my code is larger than 2048.
Right, but the magic is in the first 8 bytes, so for probing 2048 bytes
should be more than enough.
>>> diff --git a/block/add-cow.c b/block/add-cow.c
>>> new file mode 100644
>>> index 0000000..f4b67e5
>>> --- /dev/null
>>> +++ b/block/add-cow.c
>>> @@ -0,0 +1,456 @@
>>> +#include "qemu-common.h"
>>> +#include "block_int.h"
>>> +#include "module.h"
>>> +
>>> +#define 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)
>>> +#define ADD_COW_VERSION 1
>>> +
>>> +struct add_cow_header {
>>> + uint64_t magic;
>>> + uint32_t version;
>>> + char backing_file[1024];
>>> + char image_file[1024];
>>> + uint64_t size;
>>> + uint32_t sectorsize;
>>> +} add_cow_header;
>>
>> QEMU_PACKED
> Sorry, what does QEMU_PACKED mean?
This is an on-disk structure, so you need to pack the structure.
Otherwise the compiler would be free to add padding between the fields
in order to optimise alignment.
struct add_cow_header {
...
} QEMU_PACKED add_cow_header;
Hm, actually, do you really want to declare a global variable here? Or
is a typedef missing? Also, coding style requires the struct name to be
spelled AddCowHeader.
Kevin
