[libvirt PATCH] qemu: remove default audio backend for migratable XML

Daniel P. Berrangé posted 1 patch 2 years, 9 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210708100154.1453894-1-berrange@redhat.com
src/libvirt_private.syms |   1 +
src/qemu/qemu_domain.c   | 128 ++++++++++++++++++++++++++++++---------
2 files changed, 101 insertions(+), 28 deletions(-)
[libvirt PATCH] qemu: remove default audio backend for migratable XML
Posted by Daniel P. Berrangé 2 years, 9 months ago
When seeing a guest with a sound device, and no audio backend, we
automatically add an audio backend XML element based on the historical
QEMU driver behaviour. Unfortunately when we live migrate back to an
old libvirt, it may not understand the audio driver type we configured.
We thus need to strip the default audio backend when migrating.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_domain.c   | 128 ++++++++++++++++++++++++++++++---------
 2 files changed, 101 insertions(+), 28 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 43e6398ae5..cc7533a707 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -226,6 +226,7 @@ virDiskNameParse;
 virDiskNameToBusDeviceIndex;
 virDiskNameToIndex;
 virDomainActualNetDefFree;
+virDomainAudioDefFree;
 virDomainAudioFormatTypeFromString;
 virDomainAudioFormatTypeToString;
 virDomainAudioIOCommonIsSet;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8488f58e09..3af678a283 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3587,18 +3587,19 @@ qemuDomainDefAddImplicitInputDevice(virDomainDef *def)
 }
 
 static int
-qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
-                                    virDomainDef *def)
+qemuDomainDefSuggestDefaultAudioBackend(virQEMUDriver *driver,
+                                        virDomainDef *def,
+                                        bool *addAudio,
+                                        int *audioBackend,
+                                        int *audioSDLDriver)
 {
     size_t i;
-    bool addAudio = false;
     bool audioPassthrough = false;
-    int audioBackend = VIR_DOMAIN_AUDIO_TYPE_NONE;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
-    if (def->naudios > 0) {
-        return 0;
-    }
+    *addAudio = false;
+    *audioBackend = VIR_DOMAIN_AUDIO_TYPE_NONE;
+    *audioSDLDriver = VIR_DOMAIN_AUDIO_SDL_DRIVER_DEFAULT;
 
     for (i = 0; i < def->ngraphics; i++) {
         virDomainGraphicsDef *graph = def->graphics[i];
@@ -3609,18 +3610,18 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
                 audioPassthrough = true;
             } else {
                 audioPassthrough = false;
-                audioBackend = VIR_DOMAIN_AUDIO_TYPE_NONE;
+                *audioBackend = VIR_DOMAIN_AUDIO_TYPE_NONE;
             }
-            addAudio = true;
+            *addAudio = true;
             break;
         case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
             audioPassthrough = false;
-            audioBackend = VIR_DOMAIN_AUDIO_TYPE_SPICE;
-            addAudio = true;
+            *audioBackend = VIR_DOMAIN_AUDIO_TYPE_SPICE;
+            *addAudio = true;
             break;
         case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
             audioPassthrough = true;
-            addAudio = true;
+            *addAudio = true;
             break;
 
         case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
@@ -3639,24 +3640,24 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
             audioPassthrough = true;
         } else {
             audioPassthrough = false;
-            audioBackend = VIR_DOMAIN_AUDIO_TYPE_NONE;
+            *audioBackend = VIR_DOMAIN_AUDIO_TYPE_NONE;
         }
-        addAudio = true;
+        *addAudio = true;
     }
 
-    if (addAudio && audioPassthrough) {
+    if (*addAudio && audioPassthrough) {
         const char *audioenv = g_getenv("QEMU_AUDIO_DRV");
         if (audioenv == NULL) {
-            addAudio = false;
+            *addAudio = false;
         } else {
             /*
              * QEMU audio driver names are mostly the same as
              * libvirt XML audio backend names
              */
             if (STREQ(audioenv, "pa")) {
-                audioBackend = VIR_DOMAIN_AUDIO_TYPE_PULSEAUDIO;
+                *audioBackend = VIR_DOMAIN_AUDIO_TYPE_PULSEAUDIO;
             } else {
-                if ((audioBackend = virDomainAudioTypeTypeFromString(audioenv)) < 0) {
+                if (((*audioBackend) = virDomainAudioTypeTypeFromString(audioenv)) < 0) {
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                    _("unknown QEMU_AUDIO_DRV setting %s"), audioenv);
                     return -1;
@@ -3664,6 +3665,79 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
             }
         }
     }
+
+    if (*addAudio && *audioBackend == VIR_DOMAIN_AUDIO_TYPE_SDL) {
+        const char *sdldriver = g_getenv("SDL_AUDIODRIVER");
+        if (sdldriver != NULL &&
+            (((*audioSDLDriver) =
+              virDomainAudioSDLDriverTypeFromString(sdldriver)) <= 0)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("unknown SDL_AUDIODRIVER setting %s"), sdldriver);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static int
+qemuDomainDefClearDefaultAudioBackend(virQEMUDriver *driver,
+                                      virDomainDef *def)
+{
+    bool addAudio;
+    int audioBackend;
+    int audioSDLDriver;
+    virDomainAudioDef *audio;
+
+    if (def->naudios != 1) {
+        return 0;
+    }
+
+    if (qemuDomainDefSuggestDefaultAudioBackend(driver,
+                                                def,
+                                                &addAudio,
+                                                &audioBackend,
+                                                &audioSDLDriver) < 0)
+        return -1;
+
+    if (!addAudio)
+        return 0;
+
+    audio = def->audios[0];
+    if (audio->type != audioBackend)
+        return 0;
+
+    if (audio->type == VIR_DOMAIN_AUDIO_TYPE_SDL &&
+        audio->backend.sdl.driver != audioSDLDriver)
+        return 0;
+
+    virDomainAudioDefFree(audio);
+    g_free(def->audios);
+    def->naudios = 0;
+    def->audios = NULL;
+
+    return 0;
+}
+
+static int
+qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
+                                    virDomainDef *def)
+{
+    bool addAudio;
+    int audioBackend;
+    int audioSDLDriver;
+
+    if (def->naudios > 0) {
+        return 0;
+    }
+
+    if (qemuDomainDefSuggestDefaultAudioBackend(driver,
+                                                def,
+                                                &addAudio,
+                                                &audioBackend,
+                                                &audioSDLDriver) < 0)
+        return -1;
+
     if (addAudio) {
         virDomainAudioDef *audio = g_new0(virDomainAudioDef, 1);
 
@@ -3674,16 +3748,8 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
         def->audios = g_new0(virDomainAudioDef *, def->naudios);
         def->audios[0] = audio;
 
-        if (audioBackend == VIR_DOMAIN_AUDIO_TYPE_SDL) {
-            const char *sdldriver = g_getenv("SDL_AUDIODRIVER");
-            if (sdldriver != NULL &&
-                ((audio->backend.sdl.driver =
-                  virDomainAudioSDLDriverTypeFromString(sdldriver)) <= 0)) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("unknown SDL_AUDIODRIVER setting %s"), sdldriver);
-                return -1;
-            }
-        }
+        if (audioBackend == VIR_DOMAIN_AUDIO_TYPE_SDL)
+            audio->backend.sdl.driver = audioSDLDriver;
     }
 
     return 0;
@@ -6298,6 +6364,12 @@ qemuDomainDefFormatBufInternal(virQEMUDriver *driver,
 
         if (def->cpu && qemuDomainMakeCPUMigratable(def->cpu) < 0)
             return -1;
+
+        /* Old libvirt doesn't understand <audio> elements so
+         * remove any we added by default
+         */
+        if (qemuDomainDefClearDefaultAudioBackend(driver, def) < 0)
+            return -1;
     }
 
  format:
-- 
2.31.1

Re: [libvirt PATCH] qemu: remove default audio backend for migratable XML
Posted by Michal Prívozník 2 years, 9 months ago
On 7/8/21 12:01 PM, Daniel P. Berrangé wrote:
> When seeing a guest with a sound device, and no audio backend, we
> automatically add an audio backend XML element based on the historical
> QEMU driver behaviour. Unfortunately when we live migrate back to an
> old libvirt, it may not understand the audio driver type we configured.
> We thus need to strip the default audio backend when migrating.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/libvirt_private.syms |   1 +
>  src/qemu/qemu_domain.c   | 128 ++++++++++++++++++++++++++++++---------
>  2 files changed, 101 insertions(+), 28 deletions(-)


You'll need to squash this in:

diff --git i/tests/qemumigrationcookiexmldata/full-xml2xml-out.xml w/tests/qemumigrationcookiexmldata/full-xml2xml-out.xml
index 013cfb1eb2..a07dd1f147 100644
--- i/tests/qemumigrationcookiexmldata/full-xml2xml-out.xml
+++ w/tests/qemumigrationcookiexmldata/full-xml2xml-out.xml
@@ -142,7 +142,6 @@
       <sound model='ich6'>
         <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
       </sound>
-      <audio id='1' type='spice'/>
       <video>
         <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
         <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>


But of course qemumigrationcookietest fails if you run just one specific test case, because it depends on previous test cases to prepare domain object. Le sigh.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal