[Sheepdog] [PATCH v4 6/7] sheep: change hdr flags uint32_t from uint16_t

[Liu Yuan]

On 10/19/2011 01:01 AM, MORITA Kazutaka wrote:

> At Tue, 18 Oct 2011 16:58:57 +0800,
> Liu Yuan wrote:
>>
>> From: Liu Yuan <tailai.ly at taobao.com>
>>
>> And in passing, extend req/rsp into 14 uint32_t.
>>
>> Signed-off-by: Liu Yuan <tailai.ly at taobao.com>
>> ---
>>  include/sheep.h          |   23 ++++++++++++-----------
>>  include/sheepdog_proto.h |   23 ++++++++++++-----------
>>  2 files changed, 24 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/sheep.h b/include/sheep.h
>> index 230917f..0bacdb6 100644
>> --- a/include/sheep.h
>> +++ b/include/sheep.h
>> @@ -51,7 +51,7 @@
>>  struct sd_so_req {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> @@ -59,13 +59,13 @@ struct sd_so_req {
>>  	uint64_t	ctime;
>>  	uint32_t	copies;
>>  	uint32_t	tag;
>> -	uint32_t	opcode_specific[2];
>> +	uint32_t	opcode_specific[3];
>>  };
>>  
>>  struct sd_so_rsp {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> @@ -73,46 +73,46 @@ struct sd_so_rsp {
>>  	uint32_t	copies;
>>  	uint64_t	ctime;
>>  	uint64_t	oid;
>> -	uint32_t	opcode_specific[2];
>> +	uint32_t	opcode_specific[3];
>>  };
>>  
>>  struct sd_list_req {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>>  	uint32_t        tgt_epoch;
>> -	uint32_t        pad[7];
>> +	uint32_t        pad[8];
>>  };
>>  
>>  struct sd_list_rsp {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>>  	uint32_t        result;
>> -	uint32_t        pad[7];
>> +	uint32_t        pad[8];
>>  };
>>  
>>  struct sd_node_req {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>>  	uint32_t	request_ver;
>> -	uint32_t	pad[7];
>> +	uint32_t	pad[8];
>>  };
>>  
>>  struct sd_node_rsp {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> @@ -122,6 +122,7 @@ struct sd_node_rsp {
>>  	uint32_t	master_idx;
>>  	uint64_t	store_size;
>>  	uint64_t	store_free;
>> +	uint32_t	pad[1];
>>  };
>>  
>>  struct sheepdog_node_list_entry {
>> diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h
>> index 9467c44..1091184 100644
>> --- a/include/sheepdog_proto.h
>> +++ b/include/sheepdog_proto.h
>> @@ -95,28 +95,28 @@
>>  struct sd_req {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> -	uint32_t	opcode_specific[8];
>> +	uint32_t	opcode_specific[9];
>>  };
>>  
>>  struct sd_rsp {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>>  	uint32_t        result;
>> -	uint32_t	opcode_specific[7];
>> +	uint32_t	opcode_specific[8];
>>  };
>>  
>>  struct sd_obj_req {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> @@ -125,24 +125,25 @@ struct sd_obj_req {
>>  	uint32_t        copies;
>>  	uint32_t        tgt_epoch;
>>  	uint64_t        offset;
>> +	uint32_t	pad[1];
>>  };
>>  
>>  struct sd_obj_rsp {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>>  	uint32_t        result;
>>  	uint32_t        copies;
>> -	uint32_t        pad[6];
>> +	uint32_t        pad[7];
>>  };
>>  
>>  struct sd_vdi_req {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> @@ -150,13 +151,13 @@ struct sd_vdi_req {
>>  	uint32_t        base_vdi_id;
>>  	uint32_t	copies;
>>  	uint32_t        snapid;
>> -	uint32_t        pad[3];
>> +	uint32_t        pad[4];
>>  };
>>  
>>  struct sd_vdi_rsp {
>>  	uint8_t		proto_ver;
>>  	uint8_t		opcode;
>> -	uint16_t	flags;
>> +	uint32_t	flags;
>>  	uint32_t	epoch;
>>  	uint32_t        id;
>>  	uint32_t        data_length;
>> @@ -165,7 +166,7 @@ struct sd_vdi_rsp {
>>  	uint32_t        vdi_id;
>>  	uint32_t        attr_id;
>>  	uint32_t        copies;
>> -	uint32_t        pad[3];
>> +	uint32_t        pad[4];
>>  };
>>  
>>  struct sheepdog_inode {
> 
> I don't want to change network protocols easily.  This forces us to
> change client codes in QEMU too, and users suffer from the
> compatibility problem.
> 
> Isn't it much better to make all flags uint16_t?
> 


get/set_cluster_flags() use uint32_t and I think 16 flags would be
limited for future dev. And more, we'd better leave at least one extra
pad that is unused right now but would be ready for future use.

If we don't change it now, I guess, in the later future, we are likely
to face it again and a greater suffering to change this ABI.

Anyway, I am okay to make all flags uint16_t.

Thanks,
Yuan