On Fri, Nov 02, 2018 at 15:52:54 +0800, Han Han wrote:
> Hello,
> I found snapshot APIs(like virDomainSnapshotNum) invoked with
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA will return 0 even there are internal
> no-metadata snapshots in the domain.

We've never implemented the support for loading internal snapshots
without libvirt metadata, thus none will be listed.

>
> Then I find the reason is in virDomainSnapshotObjListGetNames():
>    937     /* If this common code is being used, we assume that all
> snapshots
>    938     ┆* have metadata, and thus can handle METADATA up front as
> an
>    939     ┆* all-or-none filter.  XXX This might not always be true, if
> we
>    940     ┆* add the ability to track qcow2 internal snapshots without
> the
>    941     ┆* use of metadata.
> */
>    942     if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA)
> ==
>    943     ┆
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)
>    944     ┆   return 0;
>
> So currently, with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, no snapshot will
> be returned.
>
> I checked the commit. It is really old(6478ec1673a    Eric Blake
> 2012-08-13 18:09:12 -0600) . I guess it was initially designed for the
> function to list internal snapshots without metadata.
>
> However, now internal snapshot seems lower prioritized than external
> snapshot.
> Do we have plan to design this function? Since the APIs invoked with this
> flag don't return the **right** result, if the function for
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA will not be designed, how will we deal
> with the flag? Remove it from code OR update the docs to mark it
> unsupported?

We cannot remove that flag. It will return no results until somebody
actually implements the support for snapshots with no libvirt metadata.

Also note that for other hypervisor drivers there certainly is more
possibility for modelling that as well so removing the flag does not
make sense.