From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51696) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLiwh-0000nJ-7k for qemu-devel@nongnu.org; Wed, 11 Feb 2015 20:52:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLiwc-0006Rl-5b for qemu-devel@nongnu.org; Wed, 11 Feb 2015 20:51:59 -0500 Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]:51773) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLiwb-0006IX-LV for qemu-devel@nongnu.org; Wed, 11 Feb 2015 20:51:54 -0500 Message-ID: <54DC071D.40503@lab.ntt.co.jp> Date: Thu, 12 Feb 2015 10:51:25 +0900 From: Teruaki Ishizaki MIME-Version: 1.0 References: <1422347727-13006-1-git-send-email-ishizaki.teruaki@lab.ntt.co.jp> <20150210111256.GG18727@ubuntu-trusty> In-Reply-To: <20150210111256.GG18727@ubuntu-trusty> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4] sheepdog: selectable object size support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liu Yuan Cc: kwolf@redhat.com, mitake.hitoshi@lab.ntt.co.jp, sheepdog@lists.wpkg.org, qemu-devel@nongnu.org, stefanha@redhat.com (2015/02/10 20:12), Liu Yuan wrote: > On Tue, Jan 27, 2015 at 05:35:27PM +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 >> --- >> V4: >> - Limit a read/write buffer size for creating a preallocated VDI. >> - Replace a parse function for the block_size_shift option. >> - Fix an error message. >> >> V3: >> - Delete the needless operation of buffer. >> - Delete the needless operations of request header. >> for SD_OP_GET_CLUSTER_DEFAULT. >> - Fix coding style problems. >> >> V2: >> - Fix coding style problem (white space). >> - Add members, store_policy and block_size_shift to struct SheepdogVdiReq. >> - Initialize request header to use block_size_shift specified by user. >> --- >> block/sheepdog.c | 138 ++++++++++++++++++++++++++++++++++++++------- >> include/block/block_int.h | 1 + >> 2 files changed, 119 insertions(+), 20 deletions(-) >> >> diff --git a/block/sheepdog.c b/block/sheepdog.c >> index be3176f..a43b947 100644 >> --- a/block/sheepdog.c >> +++ b/block/sheepdog.c >> @@ -37,6 +37,7 @@ >> #define SD_OP_READ_VDIS 0x15 >> #define SD_OP_FLUSH_VDI 0x16 >> #define SD_OP_DEL_VDI 0x17 >> +#define SD_OP_GET_CLUSTER_DEFAULT 0x18 > > This might not be necessary. For old qemu or the qemu-img without setting > option, the block_size_shift will be 0. > > If we make 0 to represent 4MB object, then we don't need to get the default > cluster object size. > > We migth even get rid of the idea of cluster default size. The downsize is that, > if we want to create a vdi with different size not the default 4MB, > we have to write it every time for qemu-img or dog. > > If we choose to keep the idea of cluster default size, I think we'd also try to > avoid call this request from QEMU to make backward compatibility easier. In this > scenario, 0 might be used to ask new sheep to decide to use cluster default size. > > Both old qemu and new QEMU will send 0 to sheep and both old and new sheep can > handle 0 though it has different meanings. > > Table for this bit as 0: > Qe: qemu > SD: Sheep daemon > CDS: Cluster Default Size > Ign: Ignored by the sheep daemon > > Qe/sd new old > new CDS Ign > old CDS NULL Does Ign mean that VDI is handled as 4MB object size? > > I think this approach is acceptable. The difference to your patch is that > we don't send SD_OP_GET_CLUSTER_DEFAULT to sheep daemon and > SD_OP_GET_CLUSTER_DEFAULT can be removed. When users create a new VDI with qemu-img, qemu's Sheepdog backend driver calculates max limit VDI size. But if block_size_shift option is not specified, qemu's Sheepdog backend driver can't calculate max limit VDI size. So, I think that qemu's Sheepdog backend driver must get cluster default value from sheep daemon. Thanks, Teruaki