From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBdZE-0004Do-2A for qemu-devel@nongnu.org; Thu, 15 Jan 2015 01:06:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YBdZ9-0003qI-1j for qemu-devel@nongnu.org; Thu, 15 Jan 2015 01:06:03 -0500 Received: from mail-pa0-x22d.google.com ([2607:f8b0:400e:c03::22d]:42631) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBdZ8-0003q4-R8 for qemu-devel@nongnu.org; Thu, 15 Jan 2015 01:05:59 -0500 Received: by mail-pa0-f45.google.com with SMTP id lf10so15448745pab.4 for ; Wed, 14 Jan 2015 22:05:57 -0800 (PST) Sender: Hitoshi Mitake Date: Thu, 15 Jan 2015 15:05:52 +0900 Message-ID: <8761c837en.wl%mitake.hitoshi@lab.ntt.co.jp> From: Hitoshi Mitake In-Reply-To: <1421138472-7679-1-git-send-email-ishizaki.teruaki@lab.ntt.co.jp> References: <1421138472-7679-1-git-send-email-ishizaki.teruaki@lab.ntt.co.jp> MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Qemu-devel] [sheepdog] [PATCH] sheepdog: selectable object size support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Teruaki Ishizaki Cc: kwolf@redhat.com, sheepdog@lists.wpkg.org, mitake.hitoshi@lab.ntt.co.jp, qemu-devel@nongnu.org, stefanha@redhat.com, namei.unix@gmail.com At Tue, 13 Jan 2015 17:41:12 +0900, Teruaki Ishizaki wrote: > > Previously, qemu block driver of sheepdog used hard-coded VDI object size. > This patch enables users to handle "block_size_shift" value for > calculating VDI object size. > > When you start qemu, you don't need to specify additional command option. > > But when you create the VDI which doesn't have default object size > with qemu-img command, you specify block_size_shift option. > > If you want to create a VDI of 8MB(1 << 23) object size, > you need to specify following command option. > > # qemu-img create -o block_size_shift=23 sheepdog:test1 100M > > In addition, when you don't specify qemu-img command option, > a default value of sheepdog cluster is used for creating VDI. > > # qemu-img create sheepdog:test2 100M > > Signed-off-by: Teruaki Ishizaki > --- > block/sheepdog.c | 138 +++++++++++++++++++++++++++++++++++++------- > include/block/block_int.h | 1 + > 2 files changed, 117 insertions(+), 22 deletions(-) > > @@ -1610,7 +1633,8 @@ out_with_err_set: > if (bs) { > bdrv_unref(bs); > } > - g_free(buf); > + if (buf) The above line has a style problem (white space). > @@ -1669,6 +1693,17 @@ static int parse_redundancy(BDRVSheepdogState *s, const char *opt) > return 0; > } > > +static int parse_block_size_shift(BDRVSheepdogState *s, const char *opt) > +{ > + struct SheepdogInode *inode = &s->inode; > + inode->block_size_shift = (uint8_t)atoi(opt); This patch cannot create VDI with specified block size shift. Below is the reason: Initializing block_size_shift of the inode object here (in parse_block_size_shift()) doesn't make sense. Because the block_size_shift of newly created VDI is specified by block_size_shift of SheepdogVdiReq (in sheepdog source code, sd_req.vdi). You need to synchronize the struct and initialize the parameter. Below is a slack solution: diff --git a/block/sheepdog.c b/block/sheepdog.c index 525254e..3dc3359 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -168,7 +168,8 @@ typedef struct SheepdogVdiReq { uint32_t base_vdi_id; uint8_t copies; uint8_t copy_policy; - uint8_t reserved[2]; + uint8_t store_policy; + uint8_t block_size_shift; uint32_t snapid; uint32_t type; uint32_t pad[2]; @@ -1560,6 +1561,7 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot, hdr.vdi_size = s->inode.vdi_size; hdr.copy_policy = s->inode.copy_policy; hdr.copies = s->inode.nr_copies; + hdr.block_size_shift = s->inode.block_size_shift; ret = do_req(fd, s->aio_context, (SheepdogReq *)&hdr, buf, &wlen, &rlen); # This is a very slack solution. You need to avoid using inode as a # temporal space of the parameter. Thanks, Hitoshi