<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 5, 2018 at 9:38 PM Peter Krempa <<a href="mailto:pkrempa@redhat.com">pkrempa@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Nov 02, 2018 at 15:52:54 +0800, Han Han wrote:<br>
> Hello,<br>
> I found snapshot APIs(like virDomainSnapshotNum) invoked with<br>
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA will return 0 even there are internal<br>
> no-metadata snapshots in the domain.<br>
<br>
We've never implemented the support for loading internal snapshots<br>
without libvirt metadata, thus none will be listed.<br>
<br>
> <br>
> Then I find the reason is in virDomainSnapshotObjListGetNames():<br>
>    937     /* If this common code is being used, we assume that all<br>
> snapshots<br>
>    938     ┆* have metadata, and thus can handle METADATA up front as<br>
> an<br>
>    939     ┆* all-or-none filter.  XXX This might not always be true, if<br>
> we<br>
>    940     ┆* add the ability to track qcow2 internal snapshots without<br>
> the<br>
>    941     ┆* use of metadata.<br>
> */<br>
>    942     if ((data.flags & VIR_DOMAIN_SNAPSHOT_FILTERS_METADATA)<br>
> ==<br>
>    943     ┆<br>
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA)<br>
>    944     ┆   return 0;<br>
> <br>
> So currently, with VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA, no snapshot will<br>
> be returned.<br>
> <br>
> I checked the commit. It is really old(6478ec1673a    Eric Blake<br>
> 2012-08-13 18:09:12 -0600) . I guess it was initially designed for the<br>
> function to list internal snapshots without metadata.<br>
> <br>
> However, now internal snapshot seems lower prioritized than external<br>
> snapshot.<br>
> Do we have plan to design this function? Since the APIs invoked with this<br>
> flag don't return the **right** result, if the function for<br>
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA will not be designed, how will we deal<br>
> with the flag? Remove it from code OR update the docs to mark it<br>
> unsupported?<br>
<br>
We cannot remove that flag. It will return no results until somebody<br>
actually implements the support for snapshots with no libvirt metadata.<br></blockquote><div>Since we cannot remove it, it's better to note it in the virsh manual and code comments</div><div>in case it makes users confused about the none result.  <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also note that for other hypervisor drivers there certainly is more<br>
possibility for modelling that as well so removing the flag does not<br>
make sense.<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Best regards,</div><div dir="ltr">-----------------------------------<br></div><div dir="ltr">Han Han<br>Quality Engineer<br>Redhat.<br><br>Email: <a href="mailto:hhan@redhat.com" target="_blank">hhan@redhat.com</a><br>Phone: +861065339333<br></div></div></div></div></div></div></div>