[libvirt] [PATCH] docs: Add notes for VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA
When listing snapshot with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, it always returns 0 or no snapshot. Because we never implement funtions to list no-metadata snapshot in virDomainSnapshotObjListGetNames(): if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) == VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) return 0; Add notes for that flag. Please update the comment and man page of that flag when no-metadata snapshot list is implemented in the future. Signed-off-by: Han Han <hhan@redhat.com> --- include/libvirt/libvirt-domain-snapshot.h | 5 ++++- tools/virsh.pod | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain-snapshot.h b/include/libvirt/libvirt-domain-snapshot.h index 20771f9b1e..2e19a52a5c 100644 --- a/include/libvirt/libvirt-domain-snapshot.h +++ b/include/libvirt/libvirt-domain-snapshot.h @@ -93,7 +93,10 @@ char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, * of flag (1<<0) depends on which function it is passed to; but serves * to toggle the per-call default of whether the listing is shallow or * recursive. Remaining bits come in groups; if all bits from a group are - * 0, then that group is not used to filter results. */ + * 0, then that group is not used to filter results. Internal functions + * for listing no-metadata snapshots aren't implemented. Functions above + * will return 0 when VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA is used. + * */ typedef enum { VIR_DOMAIN_SNAPSHOT_LIST_ROOTS = (1 << 0), /* Filter by snapshots with no parents, when diff --git a/tools/virsh.pod b/tools/virsh.pod index 86c041d575..b3d3840c2b 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -4689,6 +4689,9 @@ B<undefine> of a persistent domain, or be lost on B<destroy> of a transient domain. Likewise, if I<--no-metadata> is specified, the list will be filtered to just snapshots that exist without the need for libvirt metadata. +Note that - It will return no snapshot when I<--no-metadata> is +used since internal functions for listing no-metadata snapshot +are not implemented. If I<--inactive> is specified, the list will be filtered to snapshots that were taken when the domain was shut off. If I<--active> is -- 2.19.1
On Wed, 2018-11-14 at 15:19 +0800, Han Han wrote:
When listing snapshot with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, it always returns 0 or no snapshot. Because we never implement funtions to list no-metadata snapshot in virDomainSnapshotObjListGetNames():
if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) == VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) return 0;
Add notes for that flag.
Please update the comment and man page of that flag when no-metadata snapshot list is implemented in the future.
I could be missing some information, but from a quick look at the commit message and the patch it looks to me like you're documenting a known limitation instead of, you know, addressing it :) If you are able to fix the issue yourself, then please do so; otherwise, filing a bug seems like it would be a more appropriate course of action. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Nov 15, 2018 at 12:36 AM Andrea Bolognani <abologna@redhat.com> wrote:
On Wed, 2018-11-14 at 15:19 +0800, Han Han wrote:
When listing snapshot with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, it always returns 0 or no snapshot. Because we never implement funtions to list no-metadata snapshot in virDomainSnapshotObjListGetNames():
if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) == VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) return 0;
Add notes for that flag.
Please update the comment and man page of that flag when no-metadata snapshot list is implemented in the future.
I could be missing some information, but from a quick look at the commit message and the patch it looks to me like you're documenting a known limitation instead of, you know, addressing it :)
Bug filed as : https://bugzilla.redhat.com/show_bug.cgi?id=1650419
If you are able to fix the issue yourself, then please do so; otherwise, filing a bug seems like it would be a more appropriate course of action.
-- Andrea Bolognani / Red Hat / Virtualization
-- Best regards, ----------------------------------- Han Han Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
On Wed, Nov 14, 2018 at 15:19:20 +0800, Han Han wrote:
When listing snapshot with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, it always returns 0 or no snapshot. Because we never implement funtions to list no-metadata snapshot in virDomainSnapshotObjListGetNames():
if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA) == VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA) return 0;
Add notes for that flag.
Please update the comment and man page of that flag when no-metadata snapshot list is implemented in the future.
Signed-off-by: Han Han <hhan@redhat.com> --- include/libvirt/libvirt-domain-snapshot.h | 5 ++++- tools/virsh.pod | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain-snapshot.h b/include/libvirt/libvirt-domain-snapshot.h index 20771f9b1e..2e19a52a5c 100644 --- a/include/libvirt/libvirt-domain-snapshot.h +++ b/include/libvirt/libvirt-domain-snapshot.h @@ -93,7 +93,10 @@ char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, * of flag (1<<0) depends on which function it is passed to; but serves * to toggle the per-call default of whether the listing is shallow or * recursive. Remaining bits come in groups; if all bits from a group are - * 0, then that group is not used to filter results. */ + * 0, then that group is not used to filter results. Internal functions + * for listing no-metadata snapshots aren't implemented. Functions above + * will return 0 when VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA is used. + * */
This really is hypervisor dependent. E.g. virtualbox driver does not even have a concept of no-metadata snapshots as they are always tracked with virtualbox. In this case the comment is valid only for qemu driver since: 1) it supports (internal) snapshots 2) libvirt is required to have additional data, since qemu itself can't trac everything internally 3) it's not implemented. On the other hand. I don't ever expect us to implement support for no-metadata snapshots as it's a very narrow corner case which was created by the users either externally or knowingly and internally poses a lot of challenges (e.g. we won't have the XML definition from the snapshot-time and thus can't verify ABI stability). Additionally some hypervisor drivers don't even support snapshots at all. I'd go with a message along "Some hypervisors may not support metadata-less snapshots." or similar. Note that we can't just reject the flag for the qemu implementation as it might break some users.
participants (3)
-
Andrea Bolognani -
Han Han -
Peter Krempa