[PATCH] audio: Never send migration section

[PATCH] audio: Never send migration section

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The audio migration vmstate is empty, and always has been; we can't
just remove it though because an old qemu might send it us.
Changes with -audiodev now mean it's sometimes created when it didn't
used to be, and can confuse migration to old qemu.

Change it so that vmstate_audio is never sent; if it's received it
should still be accepted, and old qemu's shouldn't be too upset if it's
missing.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 audio/audio.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 59453ef856..54a153c0ef 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1622,10 +1622,20 @@ void audio_cleanup(void)
     }
 }
 
+static bool vmstate_audio_needed(void *opaque)
+{
+    /*
+     * Never needed, this vmstate only exists in case
+     * an old qemu sends it to us.
+     */
+    return false;
+}
+
 static const VMStateDescription vmstate_audio = {
     .name = "audio",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = vmstate_audio_needed,
     .fields = (VMStateField[]) {
         VMSTATE_END_OF_LIST()
     }
-- 
2.31.1

Re: [PATCH] audio: Never send migration section

[Daniel P. Berrangé]

On Mon, Aug 09, 2021 at 06:09:56PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The audio migration vmstate is empty, and always has been; we can't
> just remove it though because an old qemu might send it us.
> Changes with -audiodev now mean it's sometimes created when it didn't
> used to be, and can confuse migration to old qemu.
> 
> Change it so that vmstate_audio is never sent; if it's received it
> should still be accepted, and old qemu's shouldn't be too upset if it's
> missing.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  audio/audio.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Daniel P. Berrangé <berrange@redhat.com>


For testing I have a VM with -audiodev, but *WITHOUT* any sound
frontend devices. Live migrating to a QEMU using QEMU_AUDIO_DRV
would previously fail. With this applied it now works, showing
that we dont uncessarily send this.

I also tested migration to a QEMU with -audiodev, but without
this patch, and that still works as before, showing that QEMU
is happy if this section is not sent.

> 
> diff --git a/audio/audio.c b/audio/audio.c
> index 59453ef856..54a153c0ef 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1622,10 +1622,20 @@ void audio_cleanup(void)
>      }
>  }
>  
> +static bool vmstate_audio_needed(void *opaque)
> +{
> +    /*
> +     * Never needed, this vmstate only exists in case
> +     * an old qemu sends it to us.
> +     */
> +    return false;
> +}
> +
>  static const VMStateDescription vmstate_audio = {
>      .name = "audio",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = vmstate_audio_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_END_OF_LIST()
>      }
> -- 
> 2.31.1
> 

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: [PATCH] audio: Never send migration section

[Philippe Mathieu-Daudé]

On 8/9/21 7:12 PM, Daniel P. Berrangé wrote:
> On Mon, Aug 09, 2021 at 06:09:56PM +0100, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>
>> The audio migration vmstate is empty, and always has been; we can't
>> just remove it though because an old qemu might send it us.
>> Changes with -audiodev now mean it's sometimes created when it didn't
>> used to be, and can confuse migration to old qemu.

Not a 6.1 regression but easy change for rc3 IMO.

>> Change it so that vmstate_audio is never sent; if it's received it
>> should still be accepted, and old qemu's shouldn't be too upset if it's
>> missing.

Worth Cc: stable@ maybe?

>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>  audio/audio.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Tested-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
> For testing I have a VM with -audiodev, but *WITHOUT* any sound
> frontend devices. Live migrating to a QEMU using QEMU_AUDIO_DRV
> would previously fail. With this applied it now works, showing
> that we dont uncessarily send this.
> 
> I also tested migration to a QEMU with -audiodev, but without
> this patch, and that still works as before, showing that QEMU
> is happy if this section is not sent.
> 
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 59453ef856..54a153c0ef 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -1622,10 +1622,20 @@ void audio_cleanup(void)
>>      }
>>  }
>>  
>> +static bool vmstate_audio_needed(void *opaque)
>> +{
>> +    /*
>> +     * Never needed, this vmstate only exists in case
>> +     * an old qemu sends it to us.
>> +     */
>> +    return false;
>> +}
>> +
>>  static const VMStateDescription vmstate_audio = {
>>      .name = "audio",
>>      .version_id = 1,
>>      .minimum_version_id = 1,
>> +    .needed = vmstate_audio_needed,
>>      .fields = (VMStateField[]) {
>>          VMSTATE_END_OF_LIST()
>>      }
>> -- 
>> 2.31.1
>>
> 
> Regards,
> Daniel
> 

Re: [PATCH] audio: Never send migration section

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 8/9/21 7:12 PM, Daniel P. Berrangé wrote:
> > On Mon, Aug 09, 2021 at 06:09:56PM +0100, Dr. David Alan Gilbert (git) wrote:
> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >>
> >> The audio migration vmstate is empty, and always has been; we can't
> >> just remove it though because an old qemu might send it us.
> >> Changes with -audiodev now mean it's sometimes created when it didn't
> >> used to be, and can confuse migration to old qemu.
> 
> Not a 6.1 regression but easy change for rc3 IMO.
> 
> >> Change it so that vmstate_audio is never sent; if it's received it
> >> should still be accepted, and old qemu's shouldn't be too upset if it's
> >> missing.
> 
> Worth Cc: stable@ maybe?

Hmm yes, cc'd this reply.

Dave

> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> ---
> >>  audio/audio.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Tested-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > 
> > For testing I have a VM with -audiodev, but *WITHOUT* any sound
> > frontend devices. Live migrating to a QEMU using QEMU_AUDIO_DRV
> > would previously fail. With this applied it now works, showing
> > that we dont uncessarily send this.
> > 
> > I also tested migration to a QEMU with -audiodev, but without
> > this patch, and that still works as before, showing that QEMU
> > is happy if this section is not sent.
> > 
> >>
> >> diff --git a/audio/audio.c b/audio/audio.c
> >> index 59453ef856..54a153c0ef 100644
> >> --- a/audio/audio.c
> >> +++ b/audio/audio.c
> >> @@ -1622,10 +1622,20 @@ void audio_cleanup(void)
> >>      }
> >>  }
> >>  
> >> +static bool vmstate_audio_needed(void *opaque)
> >> +{
> >> +    /*
> >> +     * Never needed, this vmstate only exists in case
> >> +     * an old qemu sends it to us.
> >> +     */
> >> +    return false;
> >> +}
> >> +
> >>  static const VMStateDescription vmstate_audio = {
> >>      .name = "audio",
> >>      .version_id = 1,
> >>      .minimum_version_id = 1,
> >> +    .needed = vmstate_audio_needed,
> >>      .fields = (VMStateField[]) {
> >>          VMSTATE_END_OF_LIST()
> >>      }
> >> -- 
> >> 2.31.1
> >>
> > 
> > Regards,
> > Daniel
> > 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] audio: Never send migration section

[Gerd Hoffmann]

On Mon, Aug 09, 2021 at 06:09:56PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The audio migration vmstate is empty, and always has been; we can't
> just remove it though because an old qemu might send it us.
> Changes with -audiodev now mean it's sometimes created when it didn't
> used to be, and can confuse migration to old qemu.
> 
> Change it so that vmstate_audio is never sent; if it's received it
> should still be accepted, and old qemu's shouldn't be too upset if it's
> missing.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Queued for 6.1

thanks,
  Gerd

Re: [PATCH] audio: Never send migration section

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The audio migration vmstate is empty, and always has been; we can't
> just remove it though because an old qemu might send it us.
> Changes with -audiodev now mean it's sometimes created when it didn't
> used to be, and can confuse migration to old qemu.
>
> Change it so that vmstate_audio is never sent; if it's received it
> should still be accepted, and old qemu's shouldn't be too upset if it's
> missing.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>