[Sheepdog] [PATCH] sheep: change snapshot/clone flow

[MORITA Kazutaka]

At Tue, 27 Mar 2012 13:53:51 +0800,
yaohaiting.wujue at gmail.com wrote:
> 
> From: HaiTing Yao <wujue.yht at taobao.com>
> 
> When create snapshot for source VDI, the new created VDI used as source
> VDI, and the old source VDI used as snapshot. This flow make users
> confused about VDI and snapshot relation. The snapshot metadata maybe is
> stored on multi-VDI, so need read multi VDIs inode to get snapshot list.
> 
> When create snapshot, we does not need change new created VDI to source
> VDI. The source VDI just need use snapshot VDI ID as its object data ID.
> 
> Show one example.
> 
> Before modification:
> 
>   Name        Id    Size    Used  Shared    Creation time   VDI id   Tag
> s v1           1   64 MB   20 MB  0.0 MB 2012-03-26 16:55   709128
> s v1           2   64 MB  0.0 MB   20 MB 2012-03-26 16:56   709129   sn3
>   v1           3   64 MB  0.0 MB   20 MB 2012-03-26 16:56   70912a
> 
> After modification:
> 
>   Name        Id    Size    Used  Shared    Creation time   VDI id   Tag
>   v1           0   64 MB   20 MB  0.0 MB 2012-03-27 11:06   709128
> s v1           1   64 MB  0.0 MB   20 MB 2012-03-27 11:06   709129
> s v1           2   64 MB  0.0 MB   20 MB 2012-03-27 11:07   70912a   sn3
> 
> Signed-off-by: HaiTing Yao <wujue.yht at taobao.com>
> ---
>  collie/common.c          |    2 +-
>  collie/vdi.c             |   30 +++++++++++++++++++-----------
>  include/sheepdog_proto.h |    6 +++---
>  sheep/vdi.c              |   18 ++++++++++--------
>  4 files changed, 33 insertions(+), 23 deletions(-)

It seems that 'collie vdi tree' and 'collie vdi graph' don't work
correctly with this patch.

Other comments are below.

> 
> diff --git a/collie/common.c b/collie/common.c
> index f4301c4..636b821 100644
> --- a/collie/common.c
> +++ b/collie/common.c
> @@ -13,7 +13,7 @@
>  
>  int is_current(struct sheepdog_inode *i)
>  {
> -	return !i->snap_ctime;
> +	return !i->snap_id;
>  }
>  
>  char *size_to_str(uint64_t _size, char *str, int str_size)
> diff --git a/collie/vdi.c b/collie/vdi.c
> index e0678a6..8a30c99 100644
> --- a/collie/vdi.c
> +++ b/collie/vdi.c
> @@ -90,7 +90,9 @@ static void print_vdi_list(uint32_t vid, char *name, char *tag, uint32_t snapid,
>  	for (idx = 0; idx < MAX_DATA_OBJS; idx++) {
>  		if (!i->data_vdi_id[idx])
>  			continue;
> -		if (is_data_obj_writeable(i, idx))
> +		if (!i->parent_vdi_id)
> +			my_objs++;
> +		else if ((!i->snap_id) && (i->io_vdi_id == i->data_vdi_id[idx]))

Use is_data_obj_writeable().

>  			my_objs++;
>  		else
>  			cow_objs++;
> @@ -511,7 +513,7 @@ out:
>  static int vdi_snapshot(int argc, char **argv)
>  {
>  	char *vdiname = argv[optind++];
> -	uint32_t vid;
> +	uint32_t vid, own_vid;
>  	int ret;
>  	char buf[SD_INODE_HEADER_SIZE];
>  	struct sheepdog_inode *inode = (struct sheepdog_inode *)buf;
> @@ -528,20 +530,26 @@ static int vdi_snapshot(int argc, char **argv)
>  		return EXIT_FAILURE;
>  	}
>  
> -	ret = sd_read_object(vid_to_vdi_oid(vid), inode, SD_INODE_HEADER_SIZE, 0);
> -	if (ret != SD_RES_SUCCESS) {
> -		fprintf(stderr, "Failed to read an inode header\n");
> +	ret = do_vdi_create(vdiname, inode->vdi_size, vid, &own_vid, 1);
> +
> +	if (ret < 0) {
> +		fprintf(stderr, "Failed to write VDI %s\n", vdiname);
>  		return EXIT_FAILURE;
>  	}
>  
>  	if (vdi_cmd_data.snapshot_tag[0]) {
> -		ret = sd_write_object(vid_to_vdi_oid(vid), 0, vdi_cmd_data.snapshot_tag,
> +		ret = sd_read_object(vid_to_vdi_oid(own_vid), inode, SD_INODE_HEADER_SIZE, 0);
> +		if (ret != SD_RES_SUCCESS) {
> +			fprintf(stderr, "Failed to read an inode header\n");
> +			return EXIT_FAILURE;
> +		}
> +		ret = sd_write_object(vid_to_vdi_oid(own_vid), 0, vdi_cmd_data.snapshot_tag,
>  				      SD_MAX_VDI_TAG_LEN,
>  				      offsetof(struct sheepdog_inode, tag),
>  				      0, inode->nr_copies, 0);
>  	}
>  
> -	return do_vdi_create(vdiname, inode->vdi_size, vid, NULL, 1);
> +	return ret;
>  }
>  
>  static int vdi_clone(int argc, char **argv)
> @@ -1150,7 +1158,7 @@ static int vdi_write(int argc, char **argv)
>  			remain -= ret;
>  		}
>  
> -		inode->data_vdi_id[idx] = inode->vdi_id;
> +		inode->data_vdi_id[idx] = inode->io_vdi_id;
>  		oid = vid_to_data_oid(inode->data_vdi_id[idx], idx);
>  		ret = sd_write_object(oid, old_oid, buf, len, offset, flags,
>  				      inode->nr_copies, create);
> @@ -1161,9 +1169,9 @@ static int vdi_write(int argc, char **argv)
>  		}
>  
>  		if (create) {
> -			ret = sd_write_object(vid_to_vdi_oid(vid), 0, &vid, sizeof(vid),
> -					      SD_INODE_HEADER_SIZE + sizeof(vid) * idx, 0,
> -					      inode->nr_copies, 0);
> +			ret = sd_write_object(vid_to_vdi_oid(vid), 0, &inode->io_vdi_id,
> +				sizeof(inode->io_vdi_id), SD_INODE_HEADER_SIZE + sizeof(vid) * idx,
> +				0, inode->nr_copies, 0);
>  			if (ret) {
>  				ret = EXIT_FAILURE;
>  				goto out;
> diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h
> index 2d0d5ec..a4d4093 100644
> --- a/include/sheepdog_proto.h
> +++ b/include/sheepdog_proto.h
> @@ -176,7 +176,6 @@ struct sheepdog_inode {
>  	char name[SD_MAX_VDI_LEN];
>  	char tag[SD_MAX_VDI_TAG_LEN];
>  	uint64_t ctime;
> -	uint64_t snap_ctime;
>  	uint64_t vm_clock_nsec;
>  	uint64_t vdi_size;
>  	uint64_t vm_state_size;
> @@ -186,9 +185,10 @@ struct sheepdog_inode {
>  	uint32_t snap_id;
>  	uint32_t vdi_id;
>  	uint32_t parent_vdi_id;
> +	uint32_t io_vdi_id;

What do you mean by 'io_'?  Isn't there a better name?


>  	uint32_t child_vdi_id[MAX_CHILDREN];
>  	uint32_t data_vdi_id[MAX_DATA_OBJS];
> -};
> +} __attribute__((packed));

Align to 64 bit instead of using the 'packed' attribute.


Thanks,

Kazutaka