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

[Wenchao Xia]

于 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