[Sheepdog] [PATCH] sheepdog: add live snapshot feature

[MORITA Kazutaka]

Hi,

On 2010/04/19 17:39, OZAWA Tsuyoshi wrote:
> This patch provide sheepdog with live snapshot.
> 
> NOTE: To work this patch correctly, it's needed to apply the patch
> which adds vm_clock_nsec and vm_state_size to sd_inode to collie.
> 
> Signed-off-by: OZAWA Tsuyoshi <ozawa.tsuyoshi at lab.ntt.co.jp>
> ---
>  block/sheepdog.c |  284 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 280 insertions(+), 4 deletions(-)
> 

This looks great, thanks!

Some commenets are below:

- save_vm with no parameter causes segmentation fault.
- load_vm fails to load snapshot occasionally and qemu hungs up
- please add "\n" to the end of the format of printf

>  
> +static void set_vdi_index_and_offset(int *vdi_index, int *offset,
> +				int64_t pos, int size)
> +{
> +	int next_offset, overflow;
> +
> +	*vdi_index = pos / SD_DATA_OBJ_SIZE;
> +	*offset = pos % SD_DATA_OBJ_SIZE;
> +	next_offset = *offset + size;
> +	overflow = next_offset / SD_DATA_OBJ_SIZE;
> +	if (overflow && (next_offset % SD_DATA_OBJ_SIZE != 0)) {
> +		/* change to write data to next vdi */
> +		(*vdi_index)++;
> +		*offset = 0;
> +	}
> +	return;
> +}
> +

- *vdi_index should be unsigned 32 bit number.
- If overflow has happened, write only overflowed data to the next object.

> @@ -1587,12 +1615,156 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
>  
>  	dprintf("%s %s\n", sn_info->name, sn_info->id_str);
>  
> -	ret = do_sd_create(s->name, sn_info->name, s->inode.vdi_size >> 9,
> -			   s->inode.oid, NULL, 1);
> +	s->inode.vm_state_size = sn_info->vm_state_size;
> +	s->inode.vm_clock_nsec = sn_info->vm_clock_nsec;
> +
> +	/* refresh inode. */
> +	fd = connect_to_vost();
> +	if (fd < 0) {
> +		ret = -EIO;
> +		goto cleanup;
> +	}
> +
> +	memset(&hdr, 0, sizeof(hdr));
> +	hdr.opcode = SD_OP_WRITE_OBJ;
> +
> +	hdr.oid = s->inode.oid;
> +	hdr.copies = s->inode.nr_copies;
> +
> +	hdr.flags |= SD_FLAG_CMD_WRITE;
> +	hdr.data_length = SD_INODE_SIZE;
> +	hdr.offset = 0;
> +	wlen = SD_INODE_SIZE;
> +	rlen = 0;
> +
> +	ret = do_req(fd, (struct sd_req *)&hdr, &s->inode, &wlen, &rlen);
> +	if (ret < 0) {
> +		eprintf("do_req write\n");
> +		ret = -EIO;
> +		goto cleanup;
> +	}
> +
> +	if (fd < 0) {
> +		ret = -EIO;
> +		goto cleanup;
> +	}
> +
> +	ret = do_sd_create(s->name, NULL, s->inode.vdi_size >> 9,
> +			   s->inode.oid, &new_oid, 1);
> +	if (ret < 0) {
> +		eprintf("do_sd_create %m");
> +		ret = -EIO;
> +		goto cleanup;
> +	}
> +
> +	inode = (struct sd_inode *)malloc(sizeof(struct sd_inode));
> +	if (!inode) {
> +		eprintf("malloc %m");
> +		goto cleanup;
> +	}
> +
> +	memset(&hdr, 0, sizeof(hdr));
> +
> +	hdr.opcode = SD_OP_READ_OBJ;
> +	hdr.oid = new_oid;
> +	hdr.data_length = SD_INODE_SIZE;
> +	hdr.offset = 0;
> +
> +	wlen = 0;
> +	rlen = SD_INODE_SIZE;
> +
> +	ret = do_req(fd, (struct sd_req *)&hdr, inode, &wlen, &rlen);
> +	if (ret < 0) {
> +		eprintf("do_req read\n");
> +		ret = -EIO;
> +		goto cleanup;
> +	}

- use read_vdi_obj here.

> +
> +	memcpy(&s->inode, inode, sizeof(struct sd_inode));
> +	eprintf("s->inode: name %s snap_id %x oid %lxn",
> +		s->inode.name, s->inode.snap_id, s->inode.oid);
> +
> +cleanup:
> +	close(fd);
> +	return ret;
> +}
> +
> +static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
> +{
> +	struct bdrv_sd_state *s = bs->opaque;
> +	struct bdrv_sd_state *old_s;
> +	char vdi[256];
> +	char *buf = NULL;
> +	uint64_t oid;
> +	uint32_t snapid = 0;
> +	int ret = -ENOENT, dummy;
> +
> +
> +	old_s = malloc(sizeof(struct bdrv_sd_state));
> +	if (!old_s) {
> +		eprintf("malloc");
> +		goto out;
> +	}
> +
> +	memcpy(old_s, s, sizeof(struct bdrv_sd_state));
> +	sd_release(bs);
> +
> +	snapid = strtol(snapshot_id, NULL, 16);

- This should be "strtol(snapshot_id, NULL, 10);".
  It is because qemu shows a snapshot index in decimal.

Thanks,

Kazutaka Morita