Re: [Qemu-devel] [PATCH] Migration compatibility for serial

[Paolo Bonzini]

On 16/06/2015 20:54, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> Older QEMUs dont understand the new (sub)sections that
> may be generated in the serial device.   Limit their generation
> to newer machine types.
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>

No, please.  Upstream QEMU doesn't want to get into judgement about when
migration quality might be "good enough" that you can drop subsections.
 It's one thing to perfect the .needed functions to make the appearance
of subsections as unlikely as possible, but adding flags is not
something we've done so far---and not something at least *I* want to do.

Paolo


> ---
>  hw/char/serial.c         | 19 +++++++++++++------
>  hw/i386/pc_piix.c        |  2 ++
>  hw/i386/pc_q35.c         |  2 ++
>  hw/ppc/spapr.c           |  2 ++
>  include/hw/char/serial.h |  3 +++
>  5 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 513d73c..ef31df3 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -103,6 +103,9 @@ do { fprintf(stderr, "serial: " fmt , ## __VA_ARGS__); } 
> while (0)
>  do {} while (0)
>  #endif
>  
> +/* Force migration compatibility of pre-2.2 machine types */
> +bool serial_migrate_pre_2_2;
> +
>  static void serial_receive1(void *opaque, const uint8_t *buf, int size);
>  
>  static inline void recv_fifo_put(SerialState *s, uint8_t chr)
> @@ -646,6 +649,10 @@ static bool serial_thr_ipending_needed(void *opaque)
>  {
>      SerialState *s = opaque;
>  
> +    if (serial_migrate_pre_2_2) {
> +        return false;
> +    }
> +
>      if (s->ier & UART_IER_THRI) {
>          bool expected_value = ((s->iir & UART_IIR_ID) == UART_IIR_THRI);
>          return s->thr_ipending != expected_value;
> @@ -672,7 +679,7 @@ static const VMStateDescription 
> vmstate_serial_thr_ipending = {
>  static bool serial_tsr_needed(void *opaque)
>  {
>      SerialState *s = (SerialState *)opaque;
> -    return s->tsr_retry != 0;
> +    return !serial_migrate_pre_2_2 && s->tsr_retry != 0;
>  }
>  
>  static const VMStateDescription vmstate_serial_tsr = {
> @@ -691,7 +698,7 @@ static const VMStateDescription vmstate_serial_tsr = {
>  static bool serial_recv_fifo_needed(void *opaque)
>  {
>      SerialState *s = (SerialState *)opaque;
> -    return !fifo8_is_empty(&s->recv_fifo);
> +    return !serial_migrate_pre_2_2 && !fifo8_is_empty(&s->recv_fifo);
>  
>  }
>  
> @@ -709,7 +716,7 @@ static const VMStateDescription vmstate_serial_recv_fifo 
> = {
>  static bool serial_xmit_fifo_needed(void *opaque)
>  {
>      SerialState *s = (SerialState *)opaque;
> -    return !fifo8_is_empty(&s->xmit_fifo);
> +    return !serial_migrate_pre_2_2 && !fifo8_is_empty(&s->xmit_fifo);
>  }
>  
>  static const VMStateDescription vmstate_serial_xmit_fifo = {
> @@ -726,7 +733,7 @@ static const VMStateDescription vmstate_serial_xmit_fifo 
> = {
>  static bool serial_fifo_timeout_timer_needed(void *opaque)
>  {
>      SerialState *s = (SerialState *)opaque;
> -    return timer_pending(s->fifo_timeout_timer);
> +    return !serial_migrate_pre_2_2 && timer_pending(s->fifo_timeout_timer);
>  }
>  
>  static const VMStateDescription vmstate_serial_fifo_timeout_timer = {
> @@ -743,7 +750,7 @@ static const VMStateDescription 
> vmstate_serial_fifo_timeout_timer = {
>  static bool serial_timeout_ipending_needed(void *opaque)
>  {
>      SerialState *s = (SerialState *)opaque;
> -    return s->timeout_ipending != 0;
> +    return !serial_migrate_pre_2_2 && s->timeout_ipending != 0;
>  }
>  
>  static const VMStateDescription vmstate_serial_timeout_ipending = {
> @@ -760,7 +767,7 @@ static const VMStateDescription 
> vmstate_serial_timeout_ipending = {
>  static bool serial_poll_needed(void *opaque)
>  {
>      SerialState *s = (SerialState *)opaque;
> -    return s->poll_msl >= 0;
> +    return !serial_migrate_pre_2_2 && s->poll_msl >= 0;
>  }
>  
>  static const VMStateDescription vmstate_serial_poll = {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index e142f75..d6d596c 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -39,6 +39,7 @@
>  #include "hw/kvm/clock.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/sysbus.h"
> +#include "hw/char/serial.h"
>  #include "hw/cpu/icc_bus.h"
>  #include "sysemu/arch_init.h"
>  #include "sysemu/block-backend.h"
> @@ -344,6 +345,7 @@ static void pc_compat_2_1(MachineState *machine)
>      x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
>      x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
>      pcms->enforce_aligned_dimm = false;
> +    serial_migrate_pre_2_2 = true;
>  }
>  
>  static void pc_compat_2_0(MachineState *machine)
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b68263d..a211f21 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -32,6 +32,7 @@
>  #include "sysemu/arch_init.h"
>  #include "hw/i2c/smbus.h"
>  #include "hw/boards.h"
> +#include "hw/char/serial.h"
>  #include "hw/timer/mc146818rtc.h"
>  #include "hw/xen/xen.h"
>  #include "sysemu/kvm.h"
> @@ -328,6 +329,7 @@ static void pc_compat_2_1(MachineState *machine)
>      x86_cpu_compat_set_features("coreduo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
>      x86_cpu_compat_set_features("core2duo", FEAT_1_ECX, CPUID_EXT_VMX, 0);
>      x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
> +    serial_migrate_pre_2_2 = true;
>  }
>  
>  static void pc_compat_2_0(MachineState *machine)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 01f8da8..f2673da 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -39,6 +39,7 @@
>  #include "qom/cpu.h"
>  
>  #include "hw/boards.h"
> +#include "hw/char/serial.h"
>  #include "hw/ppc/ppc.h"
>  #include "hw/loader.h"
>  
> @@ -1863,6 +1864,7 @@ static void spapr_compat_2_2(Object *obj)
>  static void spapr_compat_2_1(Object *obj)
>  {
>      spapr_compat_2_2(obj);
> +    serial_migrate_pre_2_2 = true;
>  }
>  
>  static void spapr_machine_2_3_instance_init(Object *obj)
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index 15beb6b..527d728 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -94,4 +94,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>  #define TYPE_ISA_SERIAL "isa-serial"
>  void serial_hds_isa_init(ISABus *bus, int n);
>  
> +/* Force migration compatibility of pre-2.2 machine types */
> +extern bool serial_migrate_pre_2_2;
> +
>  #endif
>