From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52024) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R2Bpj-0000dP-IF for qemu-devel@nongnu.org; Fri, 09 Sep 2011 20:54:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R2Bpi-00087i-FD for qemu-devel@nongnu.org; Fri, 09 Sep 2011 20:54:11 -0400 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:51588) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R2Bph-00087X-Sv for qemu-devel@nongnu.org; Fri, 09 Sep 2011 20:54:10 -0400 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by e28smtp04.in.ibm.com (8.14.4/8.13.1) with ESMTP id p8A0s5F0008404 for ; Sat, 10 Sep 2011 06:24:05 +0530 Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8A0s5AN4460644 for ; Sat, 10 Sep 2011 06:24:05 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p8A0s4OK023639 for ; Sat, 10 Sep 2011 06:24:05 +0530 Message-ID: <4E6AB52A.7000303@linux.vnet.ibm.com> Date: Sat, 10 Sep 2011 08:54:02 +0800 From: Dong Xu Wang MIME-Version: 1.0 References: <1315547296-22066-1-git-send-email-wdongxu@linux.vnet.ibm.com> <4E6A224E.2020309@redhat.com> In-Reply-To: <4E6A224E.2020309@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed 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: Kevin Wolf Cc: wdongxu@cn.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com =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 fea= ture, so=20 >> I add COW to it to support backing_file option. I store dirty bitmap i= n an=20 >> add-cow file. When executed, it looks like this: >> qemu-img create -f add-cow -o backing_file=3Dubuntu.img,image_file=3Dt= est.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 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) +=3D linux-aio.o >> =20 >> block-nested-y +=3D raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o boc= hs.o vpc.o vvfat.o >> block-nested-y +=3D qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-sn= apshot.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 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. >> 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? > >> +typedef struct BDRVAddCowState { >> + CoMutex lock; >> + CoMutex bitmap_lock; >> +} BDRVAddCowState; >> + >> +typedef struct AddCowAIOCB { >> + BlockDriverAIOCB common; >> + int64_t sector_num; >> + QEMUIOVector *qiov; >> + int remaining_sectors; >> + int cur_nr_sectors; >> + uint64_t bytes_done; >> + bool is_write; >> + QEMUIOVector hd_qiov; >> + QEMUBH *bh; >> + >> +} AddCowAIOCB; > > You shouldn't be using AIOCBs with a coroutine-based block driver. > Instead you should just use variables on the stack and function paramet= ers. > >> +static int add_cow_flush(BlockDriverState *bs) >> +{ >> + return bdrv_flush(bs->file); >> +} > > What about bs->image_hd? > >> @@ -208,6 +209,11 @@ struct BlockDriverState { >> int in_use; /* users other than guest access, eg. block migration= */ >> QTAILQ_ENTRY(BlockDriverState) list; >> void *private; >> + >> + char image_file[1024]; >> + BlockDriverState *image_hd; >> + uint8_t *bitmap; >> + uint64_t bitmap_size; >> }; > > These belong in BDRVAddCowState. > > Kevin > > Thanks for your comments Kevin, I will produce a second version soon.