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

OZAWA Tsuyoshi ozawa.tsuyoshi at lab.ntt.co.jp
Wed Apr 21 04:53:23 CEST 2010


> Some commenets are below:
> 
> - save_vm with no parameter causes segmentation fault.

The 1st problem seems to be caused by Qemu, because del_existing_snapshots() 
is called when do_savem() without NULL checking against snapshot tag. Fixing 
qemu is very easy, but this is another work. To avoid this problem, type

$ savevm bar

or something. Note that specified tag here is going to be ignored,
because the collie daemon names its id.

> - load_vm fails to load snapshot occasionally and qemu hungs up

The 2nd problems seems to be caused by Qemu,
because Qcow2 format cause this too.

> - please add "\n" to the end of the format of printf
> - *vdi_index should be unsigned 32 bit number.
> - If overflow has happened, write only overflowed data to the next object.

These points are my patch bug, so my next patch will fix this.
Thank you for your pointing out :D

> > +	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.

Yeah, it's better.

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

Oh, I got it wrong. My next patch will also fix this.
Note that parse_vdiname() also calls strtol(snapshot_id, NULL, 16).
We also have to fix it.

Very Truly Yours
OZAWA Tsuyoshi




More information about the sheepdog mailing list