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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org