Re: [Qemu-devel] [PATCH v4] sheepdog: selectable object size support

[Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp>]

(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 <ishizaki.teruaki@lab.ntt.co.jp>
>> ---
>> 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