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@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list