1. Patchew
  2. Libvirt
  3. View series

[libvirt PATCH v2] tests: Undo recent breakages

Andrea Bolognani posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230306162346.458697-1-abologna@redhat.com
tests/testutilsqemu.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
[libvirt PATCH v2] tests: Undo recent breakages
Posted by Andrea Bolognani 1 year, 1 month ago 
Turns out that those overrides I recently removed where actually
there for a reason, and there was a motivation behind creating
the driver config as unprivileged too O:-)

Until a solution that can both ensure predictable output and
avoid code duplication is developed, go back to the previous
approach.

Fixes: 2f56f69f7f7e ("tests: Create privileged config for QEMU driver")
Fixes: 0f49b6cc6b81 ("tests: Drop no longer necessary overrides")
Fixes: 0b464cd84ff3 ("tests: Drop more QEMU driver config overrides")
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
Pushed under the build breaker rule.

Pipeline:

  https://gitlab.com/abologna/libvirt/-/pipelines/797459727

Note that the aarch64-macos-12 job is marked as failed, but the
underlying Cirrus CI job

  https://cirrus-ci.com/task/4640605947559936

finished successfully.

 tests/testutilsqemu.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index d3cdbdb4d5..d715252aee 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -631,7 +631,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
 
     driver->hostarch = virArchFromHost();
 
-    cfg = virQEMUDriverConfigNew(true, NULL);
+    cfg = virQEMUDriverConfigNew(false, NULL);
     if (!cfg)
         goto error;
     driver->config = cfg;
@@ -641,6 +641,24 @@ int qemuTestDriverInit(virQEMUDriver *driver)
     VIR_FREE(cfg->stateDir);
     VIR_FREE(cfg->configDir);
 
+    /* Override paths to ensure predictable output
+     *
+     * FIXME Find a way to achieve the same result while avoiding
+     *       code duplication
+     */
+    VIR_FREE(cfg->libDir);
+    cfg->libDir = g_strdup("/var/lib/libvirt/qemu");
+    VIR_FREE(cfg->channelTargetDir);
+    cfg->channelTargetDir = g_strdup("/var/lib/libvirt/qemu/channel/target");
+    VIR_FREE(cfg->memoryBackingDir);
+    cfg->memoryBackingDir = g_strdup("/var/lib/libvirt/qemu/ram");
+    VIR_FREE(cfg->nvramDir);
+    cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
+    VIR_FREE(cfg->passtStateDir);
+    cfg->passtStateDir = g_strdup("/var/run/libvirt/qemu/passt");
+    VIR_FREE(cfg->dbusStateDir);
+    cfg->dbusStateDir = g_strdup("/var/run/libvirt/qemu/dbus");
+
     if (!g_mkdtemp(statedir)) {
         fprintf(stderr, "Cannot create fake stateDir");
         goto error;
-- 
2.39.2
Re: [libvirt PATCH v2] tests: Undo recent breakages
Posted by Daniel P. Berrangé 1 year, 1 month ago 
On Mon, Mar 06, 2023 at 05:23:46PM +0100, Andrea Bolognani wrote:
> Turns out that those overrides I recently removed where actually
> there for a reason, and there was a motivation behind creating
> the driver config as unprivileged too O:-)

FWIW, this is going to be a super frustrating description to
someone reading the commit message in 6 months time, wondering
if we can remove this code, as you've not told the reader what
the root cause problem actually was....

> Until a solution that can both ensure predictable output and
> avoid code duplication is developed, go back to the previous
> approach.
> 
> Fixes: 2f56f69f7f7e ("tests: Create privileged config for QEMU driver")
> Fixes: 0f49b6cc6b81 ("tests: Drop no longer necessary overrides")
> Fixes: 0b464cd84ff3 ("tests: Drop more QEMU driver config overrides")
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> Pushed under the build breaker rule.
> 
> Pipeline:
> 
>   https://gitlab.com/abologna/libvirt/-/pipelines/797459727
> 
> Note that the aarch64-macos-12 job is marked as failed, but the
> underlying Cirrus CI job
> 
>   https://cirrus-ci.com/task/4640605947559936
> 
> finished successfully.
> 
>  tests/testutilsqemu.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index d3cdbdb4d5..d715252aee 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -631,7 +631,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
>  
>      driver->hostarch = virArchFromHost();
>  
> -    cfg = virQEMUDriverConfigNew(true, NULL);
> +    cfg = virQEMUDriverConfigNew(false, NULL);
>      if (!cfg)
>          goto error;
>      driver->config = cfg;
> @@ -641,6 +641,24 @@ int qemuTestDriverInit(virQEMUDriver *driver)
>      VIR_FREE(cfg->stateDir);
>      VIR_FREE(cfg->configDir);
>  
> +    /* Override paths to ensure predictable output
> +     *
> +     * FIXME Find a way to achieve the same result while avoiding
> +     *       code duplication
> +     */
> +    VIR_FREE(cfg->libDir);
> +    cfg->libDir = g_strdup("/var/lib/libvirt/qemu");
> +    VIR_FREE(cfg->channelTargetDir);
> +    cfg->channelTargetDir = g_strdup("/var/lib/libvirt/qemu/channel/target");
> +    VIR_FREE(cfg->memoryBackingDir);
> +    cfg->memoryBackingDir = g_strdup("/var/lib/libvirt/qemu/ram");
> +    VIR_FREE(cfg->nvramDir);
> +    cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
> +    VIR_FREE(cfg->passtStateDir);
> +    cfg->passtStateDir = g_strdup("/var/run/libvirt/qemu/passt");
> +    VIR_FREE(cfg->dbusStateDir);
> +    cfg->dbusStateDir = g_strdup("/var/run/libvirt/qemu/dbus");
> +
>      if (!g_mkdtemp(statedir)) {
>          fprintf(stderr, "Cannot create fake stateDir");
>          goto error;
> -- 
> 2.39.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [libvirt PATCH v2] tests: Undo recent breakages
Posted by Andrea Bolognani 1 year, 1 month ago 
On Mon, Mar 06, 2023 at 04:29:19PM +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 06, 2023 at 05:23:46PM +0100, Andrea Bolognani wrote:
> > Turns out that those overrides I recently removed where actually
> > there for a reason, and there was a motivation behind creating
> > the driver config as unprivileged too O:-)
>
> FWIW, this is going to be a super frustrating description to
> someone reading the commit message in 6 months time, wondering
> if we can remove this code, as you've not told the reader what
> the root cause problem actually was....

Fair enough, but too late to change it now... I'll try to fix the
problem myself, and hopefully it will take less than six months :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.