[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