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

[MORITA Kazutaka]

At Wed, 19 Oct 2011 11:03:50 +0800,
Liu Yuan wrote:
> 
> 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.

Let's change it when we really need it.  I think we can live with 16
bit flags.

Thanks,

Kazutaka

> 
> Anyway, I am okay to make all flags uint16_t.
> 
> Thanks,
> Yuan
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> http://lists.wpkg.org/mailman/listinfo/sheepdog