[PATCH 07/14] qemuxml2argvmock: Drop virNuma* mocks

Michal Privoznik posted 14 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH 07/14] qemuxml2argvmock: Drop virNuma* mocks
Posted by Michal Privoznik 2 years, 9 months ago
Since qemuxml2argvtest is  now using virnumamock, there's no need
for qemuxml2argvmock to offer reimplementation of virNuma*()
functions. Also, the comment about CLang and FreeBSD (introduced
in v4.3.0-40-g77ac204d14) is no longer true.  Looks like noinline
attribute was the missing culprit.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virnuma.h                            |  2 +-
 ...-unavailable-restrictive.x86_64-latest.err |  2 +-
 ...mnode-unavailable-strict.x86_64-latest.err |  2 +-
 ...umatune-static-nodeset-exceed-hostnode.err |  2 +-
 tests/qemuxml2argvmock.c                      | 41 -------------------
 5 files changed, 4 insertions(+), 45 deletions(-)

diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 21b3efee6d..f131017f85 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -32,7 +32,7 @@ int virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode,
                              virBitmap *nodeset);
 
 virBitmap *virNumaGetHostMemoryNodeset(void);
-bool virNumaNodesetIsAvailable(virBitmap *nodeset) G_NO_INLINE;
+bool virNumaNodesetIsAvailable(virBitmap *nodeset);
 bool virNumaIsAvailable(void) G_NO_INLINE;
 int virNumaGetMaxNode(void) G_NO_INLINE;
 bool virNumaNodeIsAvailable(int node) G_NO_INLINE;
diff --git a/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err b/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err
index a826c3cdeb..f872dd7e92 100644
--- a/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/numatune-memnode-unavailable-restrictive.x86_64-latest.err
@@ -1 +1 @@
-internal error: Mock: no numa node set is available at bit 999
+unsupported configuration: NUMA node 999 is unavailable
diff --git a/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err b/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err
index a826c3cdeb..f872dd7e92 100644
--- a/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/numatune-memnode-unavailable-strict.x86_64-latest.err
@@ -1 +1 @@
-internal error: Mock: no numa node set is available at bit 999
+unsupported configuration: NUMA node 999 is unavailable
diff --git a/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err b/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err
index b6b98775ee..2a33ccd791 100644
--- a/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err
+++ b/tests/qemuxml2argvdata/numatune-static-nodeset-exceed-hostnode.err
@@ -1 +1 @@
-internal error: Mock: no numa node set is available at bit 8
+unsupported configuration: NUMA node 4 is unavailable
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 85bd76c315..7169e4ca3a 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -56,47 +56,6 @@ GDateTime *g_date_time_new_now_local(void)
     return g_date_time_new_from_unix_local(1234567890);
 }
 
-bool
-virNumaIsAvailable(void)
-{
-    return true;
-}
-
-int
-virNumaGetMaxNode(void)
-{
-    return 7;
-}
-
-/* We shouldn't need to mock virNumaNodeIsAvailable() and *definitely* not
- * virNumaNodesetIsAvailable(), but it seems to be the only way to get
- * mocking to work with Clang on FreeBSD, so keep these duplicates around
- * until we figure out a cleaner solution */
-bool
-virNumaNodeIsAvailable(int node)
-{
-    return node >= 0 && node <= virNumaGetMaxNode();
-}
-
-bool
-virNumaNodesetIsAvailable(virBitmap *nodeset)
-{
-    ssize_t bit = -1;
-
-    if (!nodeset)
-        return true;
-
-    while ((bit = virBitmapNextSetBit(nodeset, bit)) >= 0) {
-        if (virNumaNodeIsAvailable(bit))
-            continue;
-
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "Mock: no numa node set is available at bit %zd", bit);
-        return false;
-    }
-
-    return true;
-}
 
 char *
 virTPMCreateCancelPath(const char *devpath)
-- 
2.39.2
Re: [PATCH 07/14] qemuxml2argvmock: Drop virNuma* mocks
Posted by Andrea Bolognani 2 years, 9 months ago
On Wed, Mar 08, 2023 at 12:14:34PM +0100, Michal Privoznik wrote:
> Since qemuxml2argvtest is  now using virnumamock, there's no need
> for qemuxml2argvmock to offer reimplementation of virNuma*()
> functions. Also, the comment about CLang and FreeBSD (introduced
> in v4.3.0-40-g77ac204d14) is no longer true.  Looks like noinline
> attribute was the missing culprit.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virnuma.h                            |  2 +-
>  ...-unavailable-restrictive.x86_64-latest.err |  2 +-
>  ...mnode-unavailable-strict.x86_64-latest.err |  2 +-
>  ...umatune-static-nodeset-exceed-hostnode.err |  2 +-
>  tests/qemuxml2argvmock.c                      | 41 -------------------
>  5 files changed, 4 insertions(+), 45 deletions(-)

Double spaces in the commit message.

Honestly I'm as baffled as to how this works as I was back when I
introduced those mocks. The situation after your changes is clearly
preferable to the hack that's currently there, so I'm more than happy
to see them merged, under the assumption of course that you've
already ensured that everything keeps working on non-Linux platforms
by running a full GitLab CI pipeline.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 07/14] qemuxml2argvmock: Drop virNuma* mocks
Posted by Kristina Hanicova 2 years, 9 months ago
On Wed, Mar 8, 2023 at 12:17 PM Michal Privoznik <mprivozn@redhat.com>
wrote:

> Since qemuxml2argvtest is  now using virnumamock, there's no need
> for qemuxml2argvmock to offer reimplementation of virNuma*()
> functions. Also, the comment about CLang and FreeBSD (introduced
> in v4.3.0-40-g77ac204d14) is no longer true.  Looks like noinline
> attribute was the missing culprit.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virnuma.h                            |  2 +-
>  ...-unavailable-restrictive.x86_64-latest.err |  2 +-
>  ...mnode-unavailable-strict.x86_64-latest.err |  2 +-
>  ...umatune-static-nodeset-exceed-hostnode.err |  2 +-
>  tests/qemuxml2argvmock.c                      | 41 -------------------
>  5 files changed, 4 insertions(+), 45 deletions(-)
>

I think you can also drop include of virnuma.h in tests/qemuxml2argvmock.c

Reviewed-by: Kristina Hanicova <khanicov@redhat.com>

Kristina