[libvirt] [PATCH 10/21] conf: Sort cache banks in capabilities XML

Martin Kletzander posted 21 patches 7 years, 6 months ago
Only 20 patches received!
[libvirt] [PATCH 10/21] conf: Sort cache banks in capabilities XML
Posted by Martin Kletzander 7 years, 6 months ago
Because the cache banks are initialized based on the order in which their
respective directories exist on the filesystem, they can appear in diferrent
order.  This is here mainly for tests because the cache directory might have
different order of children nodes and tests would fail otherwise.  It should not
be the case with sysfs, but one can never be sure.  And this does not take
almost any extra time, mainly because it gets initialized once per driver.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/conf/capabilities.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 9920a675aca3..095ef51c424a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1561,6 +1561,20 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
     VIR_FREE(ptr);
 }
 
+static int
+virCapsHostCacheBankSorter(const void *a, const void *b)
+{
+    virCapsHostCacheBankPtr ca = *(virCapsHostCacheBankPtr *)a;
+    virCapsHostCacheBankPtr cb = *(virCapsHostCacheBankPtr *)b;
+
+    if (ca->level < cb->level)
+        return -1;
+    if (ca->level > cb->level)
+        return 1;
+
+    return ca->id - cb->id;
+}
+
 int
 virCapabilitiesInitCaches(virCapsPtr caps)
 {
@@ -1700,6 +1714,12 @@ virCapabilitiesInitCaches(virCapsPtr caps)
             goto cleanup;
     }
 
+    /* Sort the array in order for the tests to be predicable.  This way we can
+     * still traverse the directory instead of guessing names (in case there is
+     * 'index1' and 'index3' but no 'index2'). */
+    qsort(caps->host.caches, caps->host.ncaches,
+          sizeof(*caps->host.caches), virCapsHostCacheBankSorter);
+
     ret = 0;
  cleanup:
     VIR_FREE(type);
-- 
2.15.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/21] conf: Sort cache banks in capabilities XML
Posted by John Ferlan 7 years, 6 months ago

On 11/13/2017 03:50 AM, Martin Kletzander wrote:
> Because the cache banks are initialized based on the order in which their
> respective directories exist on the filesystem, they can appear in diferrent

s/diferrent/different/

> order.  This is here mainly for tests because the cache directory might have
> different order of children nodes and tests would fail otherwise.  It should not
> be the case with sysfs, but one can never be sure.  And this does not take
> almost any extra time, mainly because it gets initialized once per driver.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/conf/capabilities.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 

A couple of fingers not typing what the brain told them to type and a
couple of other nits, for

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 9920a675aca3..095ef51c424a 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -1561,6 +1561,20 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
>      VIR_FREE(ptr);
>  }
>  

Two blank lines new functions

> +static int
> +virCapsHostCacheBankSorter(const void *a, const void *b)

One line per argument

> +{
> +    virCapsHostCacheBankPtr ca = *(virCapsHostCacheBankPtr *)a;
> +    virCapsHostCacheBankPtr cb = *(virCapsHostCacheBankPtr *)b;
> +
> +    if (ca->level < cb->level)
> +        return -1;
> +    if (ca->level > cb->level)
> +        return 1;
> +
> +    return ca->id - cb->id;
> +}
> +
>  int
>  virCapabilitiesInitCaches(virCapsPtr caps)
>  {
> @@ -1700,6 +1714,12 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>              goto cleanup;
>      }
>  
> +    /* Sort the array in order for the tests to be predicable.  This way we can

predictable

(weird, my email browser didn't flag the other spelling)

> +     * still traverse the directory instead of guessing names (in case there is
> +     * 'index1' and 'index3' but no 'index2'). */
> +    qsort(caps->host.caches, caps->host.ncaches,
> +          sizeof(*caps->host.caches), virCapsHostCacheBankSorter);
> +
>      ret = 0;
>   cleanup:
>      VIR_FREE(type);
> 

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