[libvirt] [PATCH 4/4] snapshot: implement new APIs for qemu, esx, and vbox

Eric Blake eblake at redhat.com
Mon Jun 11 14:39:43 UTC 2012


On 06/11/2012 08:31 AM, Osier Yang wrote:
> On 2012年06月11日 20:41, Eric Blake wrote:
>> On 06/11/2012 01:14 AM, Osier Yang wrote:
>>> On 2012年05月25日 11:33, Eric Blake wrote:
>>>> The two APIs are rather trivial; based on bits and pieces of other
>>>> existing APIs.  Rather than blindly return 0 or 1 for HasMetadata,
>>>> I chose to first validate that the snapshot in question in fact
>>>> exists.

> But not sure if checking whether the snapshot exists or not
> in hasMetadata is good or not for esx and vbox driver.

I think it _is_ good - we are returning 0 or -1 (snapshot exists, or
snapshot does not exist), as opposed to blindly returning 0 (snapshot
exists, even when it doesn't).

> It
> seems to me duplicate with domainSnapshotLookupByName, in
> case of it's very likely no changes (support to have metadata)
> for vbox and esx driver in future. Perhaps simply returns 0
> is better.

I don't see the point in taking the shortcut of always returning 0, as
that is no longer correct when the snapshot does not exist.

And yes, that means I think our current implementation of
esxDomainIsPersistent() is broken (it always returns 1 instead of
checking for existence).

But I guess that means I should either write a followup patch that fixes
the other broken functions that don't check for existence, or that I can
be convinced to change HasMetadata to always return 0 because it is no
more broken than the existing functions.

Anyone else have an opinion on whether it is better to do existence
checks rather than blindly succeeding?

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120611/7aa56837/attachment-0001.sig>


More information about the libvir-list mailing list