Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 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