From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33010) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R3IXc-0006mH-M9 for qemu-devel@nongnu.org; Mon, 12 Sep 2011 22:16:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R3IXb-0006J0-6m for qemu-devel@nongnu.org; Mon, 12 Sep 2011 22:16:04 -0400 Received: from mail-gx0-f181.google.com ([209.85.161.181]:35052) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R3IXa-0006Ie-Vy for qemu-devel@nongnu.org; Mon, 12 Sep 2011 22:16:03 -0400 Received: by gxk9 with SMTP id 9so97461gxk.12 for ; Mon, 12 Sep 2011 19:16:02 -0700 (PDT) Message-ID: <4E6EBCDC.9010903@gmail.com> Date: Tue, 13 Sep 2011 10:15:56 +0800 From: Donald 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> <4E6DBBCB.4040308@redhat.com> In-Reply-To: <4E6DBBCB.4040308@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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: Dong Xu Wang , wdongxu@cn.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com 于Mon 12 Sep 2011 03:59:07 PM CST,Kevin Wolf写到: > 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 >>> >>> 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 > Thank you,Kevin. I will consider these advice in the second version.