From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R31Nk-0007uq-Mo for qemu-devel@nongnu.org; Mon, 12 Sep 2011 03:56:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R31Nj-0006ZJ-G8 for qemu-devel@nongnu.org; Mon, 12 Sep 2011 03:56:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46103) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R31Nj-0006ZA-9B for qemu-devel@nongnu.org; Mon, 12 Sep 2011 03:56:43 -0400 Message-ID: <4E6DBBCB.4040308@redhat.com> Date: Mon, 12 Sep 2011 09:59:07 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1315547296-22066-1-git-send-email-wdongxu@linux.vnet.ibm.com> <4E6A224E.2020309@redhat.com> <4E6AB52A.7000303@linux.vnet.ibm.com> In-Reply-To: <4E6AB52A.7000303@linux.vnet.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] support add-cow file format List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dong Xu Wang Cc: wdongxu@cn.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com Am 10.09.2011 02:54, schrieb Dong Xu Wang: > =E4=BA=8EFri 09 Sep 2011 10:27:26 PM CST,Kevin Wolf=E5=86=99=E5=88=B0: >> Am 09.09.2011 07:48, schrieb Dong Xu Wang: >>> As raw file format does not support backing_file and copy on write fe= ature, so=20 >>> I add COW to it to support backing_file option. I store dirty bitmap = in an=20 >>> add-cow file. When executed, it looks like this: >>> qemu-img create -f add-cow -o backing_file=3Dubuntu.img,image_file=3D= test.img test.add-cow >>> qemu -drive if=3Dvirtio,file=3Dtest.add-cow -m 1024=20 >>> >>> (test.img is a raw format file; test.add-cow stores bitmap) >>> >>> Signed-off-by: Dong Xu Wang >> >> You should not make any changes to generic code, except maybe add >> something to bdrv_get_info(). In particular you shouldn't need to touc= h >> 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) +=3D linux-aio.o >>> =20 >>> block-nested-y +=3D raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bo= chs.o vpc.o vvfat.o >>> block-nested-y +=3D qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-s= napshot.o qcow2-cache.o >>> +block-nested-y +=3D add-cow.o >>> block-nested-y +=3D qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed= -cluster.o >>> block-nested-y +=3D qed-check.o >>> block-nested-y +=3D parallels.o nbd.o blkdebug.o sheepdog.o blkverif= y.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