Re: [Qemu-devel] [PATCH V2 09/12] qmp: add interface blockdev-snapshot-delete-internal-sync

[Wenchao Xia]

于 2013-6-19 22:24, Stefan Hajnoczi 写道:
> On Wed, Jun 19, 2013 at 04:53:38PM +0800, Wenchao Xia wrote:
> > 于 2013-6-19 15:46, Stefan Hajnoczi 写道:
> > > On Wed, Jun 19, 2013 at 02:18:48PM +0800, Wenchao Xia wrote:
> > > > 于 2013-6-18 22:20, Stefan Hajnoczi 写道:
> > > > > On Mon, Jun 17, 2013 at 11:25:26AM +0800, Wenchao Xia wrote:
> > > > > > 于 2013-6-15 17:55, Eric Blake 写道:
> > > > > > > Should this command be made available via 'transaction'?  That is, if I
> > > > > > > have a two-disk VM, and use 'transaction' to take a snapshot of both
> > > > > > > disks at once, shouldn't I also have a way to delete the snapshots of
> > > > > > > both at once, or gracefully fail without data loss if the second one has
> > > > > > > problems?
> > > > > >
> > > > > >    I think adding it in transaction is not very useful but brings more
> > > > > > complexity. Transcation is used to guareentee all operations are taken
> > > > > > in one time point, for example, snapshot creation use it to make sure
> > > > > > all are consistent to VM. But for deletion, this requirement do not
> > > > > > exist.
> > > > >
> > > > > I guess the problem is: can we make internal snapshot deletion
> > > > > transactional?  It's hard to do rollback for snapshot deletion.
> > > >    Several deletion in transaction equals to several calls of
> > > > 'blockdev-snapshot-delete-internal-sync', unlike creation, so I hope
> > > > not add it which have rollback issue.
> > > >
> > > >
> > > > > But batching is definitely useful for doing 'delvm' in QMP.  I just
> > > > > don't think transactions help.  We just need a 'delvm' equivalent in
> > > > > QMP.
> > > >    Maybe the caller can encapsulate a batch interface at its level.
> > >
> > > 'delvm' is a batch interface - it deletes internal snapshots that have
> > > the same name across multiple devices.
> > >
> > > It's not as flexible as:
> > > blkdev-internal-snapshot-delete drive0 drive2 drive4
> > >
> > > Because that would allow you to specify specific drives.
> >    Do you mean this interface should be changed as
> > blkdev-internal-snapshot-delete devices_array name *id?
> >
> >    It seems not much difference with following methods:
> >
> > for device in device_list:
> >    blkdev-internal-snapshot-delete device name
>
> The ability to batch in a single QMP command feels a little nicer.
> There are less opportunities for the operation to stop half-way through.
>
> It would be usable as a QMP 'delvm' and in a more flexible way to delete
> multiple internal snapshots.
>
> Would be interesting to see what Eric Blake thinks from a libvirt
> perspective.
>
> Stefan
  Just want to mention: other interfaces for block device take single
device as parameter now, use array here would be a little strange, and
increase the complexity in qmp and hmp interface. My opinion is
let caller do:

for device in device_list:
    blkdev-internal-snapshot-delete device name

Still, hope to know Eric's opinion.

--
Best Regards

Wenchao Xia