[libvirt] [PATCH] domcaps: Remove function initializing domain caps as unsupported

Peter Krempa posted 1 patch 4 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/802dfefc1292a2a85b6fed6563d6e8c882f9ee82.1574328077.git.pkrempa@redhat.com
src/bhyve/bhyve_capabilities.c |  4 +++-
src/conf/domain_capabilities.c | 15 ---------------
src/conf/domain_capabilities.h |  2 --
src/libvirt_private.syms       |  1 -
src/libxl/libxl_capabilities.c |  5 +++--
src/qemu/qemu_capabilities.c   |  3 ++-
6 files changed, 8 insertions(+), 22 deletions(-)
[libvirt] [PATCH] domcaps: Remove function initializing domain caps as unsupported
Posted by Peter Krempa 4 years, 4 months ago
Commit 5751a0b6b1968bb2354b2ac21cc5938b93009590 added a helper function
called virDomainCapsFeaturesInitUnsupported which initialized all domain
capability features as unsupported.

When adding a new feature this would initialize it as unsupported also
for hypervisor drivers which the original author possibly didn't intend
to modify. To prevent accidental wrong value being reported in such case
revert back to initializing individual features in the hypervisor
drivers themselves.

This is not a straight revert as additonal patches modified how we store
the capabilities.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
CCing all people in the original conversation:
CC: Cole Robinson <crobinso@redhat.com>
CC: Ján Tomko <jtomko@redhat.com>
CC: Michal Prívozník <mprivozn@redhat.com>

 src/bhyve/bhyve_capabilities.c |  4 +++-
 src/conf/domain_capabilities.c | 15 ---------------
 src/conf/domain_capabilities.h |  2 --
 src/libvirt_private.syms       |  1 -
 src/libxl/libxl_capabilities.c |  5 +++--
 src/qemu/qemu_capabilities.c   |  3 ++-
 6 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
index f80cf7be62..fb8829d571 100644
--- a/src/bhyve/bhyve_capabilities.c
+++ b/src/bhyve/bhyve_capabilities.c
@@ -116,7 +116,9 @@ virBhyveDomainCapsFill(virDomainCapsPtr caps,
     }

     caps->hostdev.supported = VIR_TRISTATE_BOOL_NO;
-    virDomainCapsFeaturesInitUnsupported(caps);
+    caps->features[VIR_DOMAIN_CAPS_FEATURE_IOTHREADS] = VIR_TRISTATE_BOOL_NO;
+    caps->features[VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO] = VIR_TRISTATE_BOOL_NO;
+    caps->features[VIR_DOMAIN_CAPS_FEATURE_GENID] = VIR_TRISTATE_BOOL_NO;
     caps->gic.supported = VIR_TRISTATE_BOOL_NO;

     return 0;
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 0979711fea..fa27e95e1b 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -272,21 +272,6 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)
 }


-/**
- * @caps: domain caps
- *
- * Initializes all features in 'caps' as unsupported.
- */
-void
-virDomainCapsFeaturesInitUnsupported(virDomainCapsPtr caps)
-{
-    size_t i;
-
-    for (i = 0; i < VIR_DOMAIN_CAPS_FEATURE_LAST; i++)
-        caps->features[i] = VIR_TRISTATE_BOOL_NO;
-}
-
-
 static int
 virDomainCapsEnumFormat(virBufferPtr buf,
                         const virDomainCapsEnum *capsEnum,
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index d69bf8d13e..c8eeb035ac 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -227,8 +227,6 @@ int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum,
                          unsigned int *values);
 void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum);

-void virDomainCapsFeaturesInitUnsupported(virDomainCapsPtr caps);
-
 char * virDomainCapsFormat(const virDomainCaps *caps);

 int virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3416438841..44201cda14 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -200,7 +200,6 @@ virDomainCapsCPUUsableTypeToString;
 virDomainCapsDeviceDefValidate;
 virDomainCapsEnumClear;
 virDomainCapsEnumSet;
-virDomainCapsFeaturesInitUnsupported;
 virDomainCapsFormat;
 virDomainCapsNew;
 virSEVCapabilitiesFree;
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 052fbd4603..c920c2aec5 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -764,8 +764,9 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
         libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)
         return -1;

-    virDomainCapsFeaturesInitUnsupported(domCaps);
-
+    domCaps->features[VIR_DOMAIN_CAPS_FEATURE_IOTHREADS] = VIR_TRISTATE_BOOL_NO;
+    domCaps->features[VIR_DOMAIN_CAPS_FEATURE_VMCOREINFO] = VIR_TRISTATE_BOOL_NO;
+    domCaps->features[VIR_DOMAIN_CAPS_FEATURE_GENID] = VIR_TRISTATE_BOOL_NO;
     domCaps->gic.supported = VIR_TRISTATE_BOOL_NO;

     return 0;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0a406972e7..d74bcd96b0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5447,6 +5447,8 @@ virQEMUCapsFillDomainFeaturesFromQEMUCaps(virQEMUCapsPtr qemuCaps,
     for (i = 0; i < G_N_ELEMENTS(domCapsTuples); i++) {
         if (virQEMUCapsGet(qemuCaps, domCapsTuples[i].qemucap))
             domCaps->features[domCapsTuples[i].domcap] = VIR_TRISTATE_BOOL_YES;
+        else
+            domCaps->features[domCapsTuples[i].domcap] = VIR_TRISTATE_BOOL_NO;
     }
 }

@@ -5735,7 +5737,6 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps,
     virDomainCapsDeviceVideoPtr video = &domCaps->video;
     virDomainCapsDeviceRNGPtr rng = &domCaps->rng;

-    virDomainCapsFeaturesInitUnsupported(domCaps);
     virQEMUCapsFillDomainFeaturesFromQEMUCaps(qemuCaps, domCaps);

     domCaps->maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps,
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domcaps: Remove function initializing domain caps as unsupported
Posted by Michal Privoznik 4 years, 4 months ago
On 11/21/19 10:23 AM, Peter Krempa wrote:
> Commit 5751a0b6b1968bb2354b2ac21cc5938b93009590 added a helper function
> called virDomainCapsFeaturesInitUnsupported which initialized all domain
> capability features as unsupported.
> 
> When adding a new feature this would initialize it as unsupported also
> for hypervisor drivers which the original author possibly didn't intend
> to modify. To prevent accidental wrong value being reported in such case
> revert back to initializing individual features in the hypervisor
> drivers themselves.
> 
> This is not a straight revert as additonal patches modified how we store
> the capabilities.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> CCing all people in the original conversation:
> CC: Cole Robinson <crobinso@redhat.com>
> CC: Ján Tomko <jtomko@redhat.com>
> CC: Michal Prívozník <mprivozn@redhat.com>
> 
>   src/bhyve/bhyve_capabilities.c |  4 +++-
>   src/conf/domain_capabilities.c | 15 ---------------
>   src/conf/domain_capabilities.h |  2 --
>   src/libvirt_private.syms       |  1 -
>   src/libxl/libxl_capabilities.c |  5 +++--
>   src/qemu/qemu_capabilities.c   |  3 ++-
>   6 files changed, 8 insertions(+), 22 deletions(-)

I like this better. Sorry for not catching this sooner. You can count on 
my Reviewed-by, but I will let others to express their opinion, Cole?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domcaps: Remove function initializing domain caps as unsupported
Posted by Cole Robinson 4 years, 4 months ago
On 11/21/19 4:23 AM, Peter Krempa wrote:
> Commit 5751a0b6b1968bb2354b2ac21cc5938b93009590 added a helper function
> called virDomainCapsFeaturesInitUnsupported which initialized all domain
> capability features as unsupported.
> 
> When adding a new feature this would initialize it as unsupported also
> for hypervisor drivers which the original author possibly didn't intend
> to modify. To prevent accidental wrong value being reported in such case
> revert back to initializing individual features in the hypervisor
> drivers themselves.
> 
> This is not a straight revert as additonal patches modified how we store
> the capabilities.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> CCing all people in the original conversation:
> CC: Cole Robinson <crobinso@redhat.com>
> CC: Ján Tomko <jtomko@redhat.com>
> CC: Michal Prívozník <mprivozn@redhat.com>
> 
>  src/bhyve/bhyve_capabilities.c |  4 +++-
>  src/conf/domain_capabilities.c | 15 ---------------
>  src/conf/domain_capabilities.h |  2 --
>  src/libvirt_private.syms       |  1 -
>  src/libxl/libxl_capabilities.c |  5 +++--
>  src/qemu/qemu_capabilities.c   |  3 ++-
>  6 files changed, 8 insertions(+), 22 deletions(-)

Reviewed-by: Cole Robinson <crobinso@redhat.com>

Thank you for agreeing to change this

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list