δΊ 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. PaoloSorry 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
