[libvirt PATCH] capabilities: report full external snapshot support

Pavel Hrdina posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/96386335016deb084c08e15bfe156a68ead5d3af.1692890863.git.phrdina@redhat.com
docs/formatcaps.rst                            | 6 ++++++
src/conf/capabilities.c                        | 1 +
src/conf/capabilities.h                        | 1 +
src/conf/schemas/capability.rng                | 5 +++++
src/qemu/qemu_capabilities.c                   | 1 +
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.aarch64.xml     | 1 +
tests/qemucaps2xmloutdata/caps.ppc.xml         | 1 +
tests/qemucaps2xmloutdata/caps.ppc64.xml       | 1 +
tests/qemucaps2xmloutdata/caps.riscv64.xml     | 1 +
tests/qemucaps2xmloutdata/caps.s390x.xml       | 1 +
tests/qemucaps2xmloutdata/caps.sparc.xml       | 1 +
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml  | 1 +
tests/qemucaps2xmloutdata/caps.x86_64.xml      | 1 +
14 files changed, 23 insertions(+)
[libvirt PATCH] capabilities: report full external snapshot support
Posted by Pavel Hrdina 8 months, 2 weeks ago
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 docs/formatcaps.rst                            | 6 ++++++
 src/conf/capabilities.c                        | 1 +
 src/conf/capabilities.h                        | 1 +
 src/conf/schemas/capability.rng                | 5 +++++
 src/qemu/qemu_capabilities.c                   | 1 +
 tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
 tests/qemucaps2xmloutdata/caps.aarch64.xml     | 1 +
 tests/qemucaps2xmloutdata/caps.ppc.xml         | 1 +
 tests/qemucaps2xmloutdata/caps.ppc64.xml       | 1 +
 tests/qemucaps2xmloutdata/caps.riscv64.xml     | 1 +
 tests/qemucaps2xmloutdata/caps.s390x.xml       | 1 +
 tests/qemucaps2xmloutdata/caps.sparc.xml       | 1 +
 tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml  | 1 +
 tests/qemucaps2xmloutdata/caps.x86_64.xml      | 1 +
 14 files changed, 23 insertions(+)

diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index f7e5342654..0e056914b8 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,6 +134,11 @@ The ``<guest/>`` element will typically wrap up the following elements:
       external disk snapshots are supported. If absent, external snapshots may
       still be supported, but it requires attempting the API and checking for an
       error to find out for sure. :since:`Since 1.2.3`
+   ``externalSnapshot``
+      If this element is present, the hypervisor supports creating, deleting and
+      reverting external snapshots including memory state and management
+      applications can switch from internal snapshots to external snapshots.
+      :since:`Since 9.7.0`
 
 Examples
 ~~~~~~~~
@@ -318,6 +323,7 @@ capabilities enabled in the chip and BIOS you will see:
         <cpuselection/>
         <deviceboot/>
         <disksnapshot default='on' toggle='no'/>
+        <externalSnapshot/>
       </features>
     </guest>
 
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 56768ce6e0..34f04cb7d3 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo virCapsGuestFeatureInfos[VIR_CAPS_GU
     [VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
     [VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
     [VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot", false },
 };
 
 
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c78e3e52fa..9eaf6e2807 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,6 +40,7 @@ typedef enum {
     VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
     VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
     VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+    VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
 
     VIR_CAPS_GUEST_FEATURE_TYPE_LAST
 } virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index 83b414961a..b1968df258 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,6 +493,11 @@
             <empty/>
           </element>
         </optional>
+        <optional>
+          <element name="externalSnapshot">
+            <empty/>
+          </element>
+        </optional>
       </interleave>
     </element>
   </define>
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40eacf0f4d..05cc11218a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1121,6 +1121,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
     virCapabilitiesAddGuestFeature(guest, VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
     virCapabilitiesAddGuestFeatureWithToggle(guest, VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
                                              true, false);
+    virCapabilitiesAddGuestFeature(guest, VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
 
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
         virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b53a886b90..b9a5b5a1d6 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,6 +21,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 5dca6d3102..61512ed740 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,6 +21,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml b/tests/qemucaps2xmloutdata/caps.ppc.xml
index e7ba391795..86d6b85ae0 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,6 +19,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.ppc64.xml b/tests/qemucaps2xmloutdata/caps.ppc64.xml
index 85623f3980..90859f9594 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc64.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc64.xml
@@ -20,6 +20,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.riscv64.xml b/tests/qemucaps2xmloutdata/caps.riscv64.xml
index 09b7eb7f2f..c6fa950211 100644
--- a/tests/qemucaps2xmloutdata/caps.riscv64.xml
+++ b/tests/qemucaps2xmloutdata/caps.riscv64.xml
@@ -19,6 +19,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.s390x.xml b/tests/qemucaps2xmloutdata/caps.s390x.xml
index bb82a15040..6379ab73e6 100644
--- a/tests/qemucaps2xmloutdata/caps.s390x.xml
+++ b/tests/qemucaps2xmloutdata/caps.s390x.xml
@@ -20,6 +20,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.sparc.xml b/tests/qemucaps2xmloutdata/caps.sparc.xml
index 9d977c4102..b5b158e430 100644
--- a/tests/qemucaps2xmloutdata/caps.sparc.xml
+++ b/tests/qemucaps2xmloutdata/caps.sparc.xml
@@ -19,6 +19,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml b/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml
index 356819a6c6..f5e49ba4db 100644
--- a/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml
@@ -22,6 +22,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
diff --git a/tests/qemucaps2xmloutdata/caps.x86_64.xml b/tests/qemucaps2xmloutdata/caps.x86_64.xml
index 35359780c4..8dd1439a80 100644
--- a/tests/qemucaps2xmloutdata/caps.x86_64.xml
+++ b/tests/qemucaps2xmloutdata/caps.x86_64.xml
@@ -22,6 +22,7 @@
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
+      <externalSnapshot/>
     </features>
   </guest>
 
-- 
2.41.0
Re: [libvirt PATCH] capabilities: report full external snapshot support
Posted by Peter Krempa 8 months, 2 weeks ago
On Thu, Aug 24, 2023 at 17:29:49 +0200, Pavel Hrdina wrote:
> Now that deleting and reverting external snapshots is implemented we can
> report that in capabilities so management applications can use that
> information and start using external snapshots.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  docs/formatcaps.rst                            | 6 ++++++
>  src/conf/capabilities.c                        | 1 +
>  src/conf/capabilities.h                        | 1 +
>  src/conf/schemas/capability.rng                | 5 +++++
>  src/qemu/qemu_capabilities.c                   | 1 +
>  tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
>  tests/qemucaps2xmloutdata/caps.aarch64.xml     | 1 +
>  tests/qemucaps2xmloutdata/caps.ppc.xml         | 1 +
>  tests/qemucaps2xmloutdata/caps.ppc64.xml       | 1 +
>  tests/qemucaps2xmloutdata/caps.riscv64.xml     | 1 +
>  tests/qemucaps2xmloutdata/caps.s390x.xml       | 1 +
>  tests/qemucaps2xmloutdata/caps.sparc.xml       | 1 +
>  tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml  | 1 +
>  tests/qemucaps2xmloutdata/caps.x86_64.xml      | 1 +
>  14 files changed, 23 insertions(+)
> 
> diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
> index f7e5342654..0e056914b8 100644
> --- a/docs/formatcaps.rst
> +++ b/docs/formatcaps.rst
> @@ -134,6 +134,11 @@ The ``<guest/>`` element will typically wrap up the following elements:
>        external disk snapshots are supported. If absent, external snapshots may
>        still be supported, but it requires attempting the API and checking for an
>        error to find out for sure. :since:`Since 1.2.3`
> +   ``externalSnapshot``
> +      If this element is present, the hypervisor supports creating, deleting and
> +      reverting external snapshots including memory state and management
> +      applications can switch from internal snapshots to external snapshots.
> +      :since:`Since 9.7.0`

This implies that 'creation' of external snapshots is also included in
the "Sice 9.7.0". Reword it such that you explicitly mention that
creation of external snapshots is a long-existing feature.

You can also entirely omit 'creation' from the text as it's covered by
'disksnapshot' above.
Re: [libvirt PATCH] capabilities: report full external snapshot support
Posted by Pavel Hrdina 8 months, 2 weeks ago
On Thu, Aug 24, 2023 at 05:44:32PM +0200, Peter Krempa wrote:
> On Thu, Aug 24, 2023 at 17:29:49 +0200, Pavel Hrdina wrote:
> > Now that deleting and reverting external snapshots is implemented we can
> > report that in capabilities so management applications can use that
> > information and start using external snapshots.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  docs/formatcaps.rst                            | 6 ++++++
> >  src/conf/capabilities.c                        | 1 +
> >  src/conf/capabilities.h                        | 1 +
> >  src/conf/schemas/capability.rng                | 5 +++++
> >  src/qemu/qemu_capabilities.c                   | 1 +
> >  tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
> >  tests/qemucaps2xmloutdata/caps.aarch64.xml     | 1 +
> >  tests/qemucaps2xmloutdata/caps.ppc.xml         | 1 +
> >  tests/qemucaps2xmloutdata/caps.ppc64.xml       | 1 +
> >  tests/qemucaps2xmloutdata/caps.riscv64.xml     | 1 +
> >  tests/qemucaps2xmloutdata/caps.s390x.xml       | 1 +
> >  tests/qemucaps2xmloutdata/caps.sparc.xml       | 1 +
> >  tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml  | 1 +
> >  tests/qemucaps2xmloutdata/caps.x86_64.xml      | 1 +
> >  14 files changed, 23 insertions(+)
> > 
> > diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
> > index f7e5342654..0e056914b8 100644
> > --- a/docs/formatcaps.rst
> > +++ b/docs/formatcaps.rst
> > @@ -134,6 +134,11 @@ The ``<guest/>`` element will typically wrap up the following elements:
> >        external disk snapshots are supported. If absent, external snapshots may
> >        still be supported, but it requires attempting the API and checking for an
> >        error to find out for sure. :since:`Since 1.2.3`
> > +   ``externalSnapshot``
> > +      If this element is present, the hypervisor supports creating, deleting and
> > +      reverting external snapshots including memory state and management
> > +      applications can switch from internal snapshots to external snapshots.
> > +      :since:`Since 9.7.0`
> 
> This implies that 'creation' of external snapshots is also included in
> the "Sice 9.7.0". Reword it such that you explicitly mention that
> creation of external snapshots is a long-existing feature.
> 
> You can also entirely omit 'creation' from the text as it's covered by
> 'disksnapshot' above.

Yeah the fact the feature name is `disksnapshot` is a bit unfortunate
and it doesn't mention that only creation is supported if that feature
is available. I'll reword it and post v2.

Thanks,
Pavel