[libvirt] RFC: VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA is not functional?
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. 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? -- Best regards, ----------------------------------- Han Han Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
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.
On Mon, Nov 5, 2018 at 9:38 PM Peter Krempa <pkrempa@redhat.com> wrote:
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.
Since we cannot remove it, it's better to note it in the virsh manual and code comments in case it makes users confused about the none result.
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.
-- Best regards, ----------------------------------- Han Han Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
participants (2)
-
Han Han -
Peter Krempa