δΊ 2013-4-3 17:02, Wenchao Xia ει:
No, if bdrv_snapshot_delete() can fail, you need to split it in two
parts: one that can fail, and one that cannot. If you cannot, then
there are two possibilities:
- if the failures are minor and could be repaired with "qemu-img check
-r"
(e.g. lost clusters), then this is not optimal but can still be done;
- otherwise, the operation simply cannot be made transactionable.
In the case of qcow2_snapshot_delete, everything except
ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
&header_data, sizeof(header_data));
if (ret < 0) {
goto fail;
}
must be in the prepare phase. Everything after "fail" (which right now
is nothing, but it should at least undo the qcow2_alloc_clusters
operation)
must be in the rollback phase. Everything in the middle is the commit
phase.
Paolo
Sorry I haven't state it clearly. What about bdrv_snapshot_create()
operation? If it need to be rolled back, I think bdrv_snapshot_delete()
will get called and it may fail. But in most case if
bdrv_snapshot_create() succeed before, the bdrv_snapshot_delete should
succeed also, so if fail there may be unexpected error below, could
assert be used for this?
After consideration again, I think bdrv_snapshot_create() should be
split apart like above, thank u for the tip.
--
Best Regards
Wenchao Xia