Re: [Qemu-devel] [PATCH 4/6] snapshot: implemention of common API to take snapshots

[Wenchao Xia]

于 2012-12-17 15:52, Dietmar Maurer 写道:
> > 于 2012-12-17 14:36, Dietmar Maurer 写道:
> > > >     This patch is the implemention of transaction based snapshot internal
> > API.
> > > What exactly is the advantage of using qmp transactions (as opposed to the
> > pve snapshot patches)?
> >     Which pve snapshot patches you referring?
>
> I refer to:
>
> https://git.proxmox.com/?p=pve-qemu-kvm.git;a=blob;f=debian/patches/internal-snapshot-async.patch;h=6c86de3a6160c58d77baa41a7774c4a80e63639e;hb=HEAD
>
> > In group case a check should be
> > done for all snapshot first, and I need to support qmp transaction interface
> > which is already there, so I build an internal transaction layer below to make
> > the interface unified and relative easier to add in qmp transaction.
>
> IMHO qmp transactions are a bit clumsy and inflexible. And as you can see with 
> my patches,
> there is no real need for them.
  There are two layers, qmp transaction and BlkTransactionStates, user
can use qmp_transaction interface or simply qmp_snapshot_create
interface, which are both consumer of BlkTransactionStates.

  Compared to the pre patch, I think what different is for group
use case and integration with external snapshot code. mainly reason
are:
  1 in group case, it need a queue with element records each one's
states, so it can revert on fail, that date structure is
BlkTransactionStatesList.
  2 in group case check should be done first, so separate check,
execution, cancel functions should be there. And there are two
type of snapshot there with two type of operation(create and delete),
so implement code will be fragment, need to packaged them in unified
way, then upper level function such as savevm can use it gracefully.
BlkTransactionStates can hide the implement details.
  3 group internal and external snapshot have same logic, they can be
merged with callback functions, which is embbed in BlkTransactionStates,
otherwise there will be "if" in many place makes code ugly which way I
have tried and falled back from.

some other reason not related to pre patch:
  4 qmp_transaction interface is already there for external snapshot,
so I think internal snapshot should also support it.
BlkTransactionStates make it easier.
  5 BlkTransactionStatesList use qemu internal data types instead data
exposed in BlockDevAction, this allows other code inside qemu pass in
data not observable to user and hide some functions to user.


> > > Seems this makes it impossible to use external commands to make
> > snapshots?
> > >
> >     what command? can you tip more about the case?
>
> For example, nexenta storage provides an API to create snapshots. We want to use
> that. Another example would be to use lvcreate to create lvm snapshots.
  I am not familar with those tools, Patch 5 and 6 provides hmp and qmp
API, hope you can enlight more what is  missing if it can't work with
external tool.

> - Dietmar


--
Best Regards

Wenchao Xia