Re: [PATCH v2 04/42] esp: add vmstate_esp version to embedded ESPState

[Laurent Vivier]

Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> The QOM object representing ESPState is currently embedded within both the
> SYSBUS_ESP and PCI_ESP devices with migration state handled by embedding
> vmstate_esp within each device using VMSTATE_STRUCT.
> 
> Since the vmstate_esp fields are embedded directly within the migration
> stream, the incoming vmstate_esp version_id is lost. The only version 
> information
> available is that from vmstate_sysbus_esp_scsi and vmstate_esp_pci_scsi, but
> those versions represent their respective devices and not that of the 
> underlying
> ESPState.
> 
> Resolve this by adding a new version-dependent field in 
> vmstate_sysbus_esp_scsi
> and vmstate_esp_pci_scsi which stores the vmstate_esp version_id field within
> ESPState to be used to allow migration from older QEMU versions.
> 
> Finally bump the vmstate_esp version to 5 to cover the upcoming ESPState 
> changes
> within this patch series.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/scsi/esp-pci.c     |  3 ++-
>  hw/scsi/esp.c         | 23 +++++++++++++++++++++--
>  include/hw/scsi/esp.h |  2 ++
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index ccf73bb901..8a82404853 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -330,11 +330,12 @@ static void esp_pci_hard_reset(DeviceState *dev)
>  
>  static const VMStateDescription vmstate_esp_pci_scsi = {
>      .name = "pciespscsi",
> -    .version_id = 1,
> +    .version_id = 2,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, PCIESPState),
>          VMSTATE_BUFFER_UNSAFE(dma_regs, PCIESPState, 0, 8 * 
> sizeof(uint32_t)),
> +        VMSTATE_UINT8_V(esp.mig_version_id, PCIESPState, 2),
>          VMSTATE_STRUCT(esp, PCIESPState, 0, vmstate_esp, ESPState),
>          VMSTATE_END_OF_LIST()
>      }
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 1635f86622..9427c55d1d 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -796,10 +796,28 @@ static const VMStateDescription vmstate_esp_pdma = {
>      }
>  };
>  
> +static int esp_pre_save(void *opaque)
> +{
> +    ESPState *s = ESP(opaque);
> +
> +    s->mig_version_id = vmstate_esp.version_id;
> +    return 0;
> +}
> +
> +static int esp_post_load(void *opaque, int version_id)
> +{
> +    ESPState *s = ESP(opaque);
> +
> +    s->mig_version_id = vmstate_esp.version_id;
> +    return 0;
> +}
> +
>  const VMStateDescription vmstate_esp = {
>      .name = "esp",
> -    .version_id = 4,
> +    .version_id = 5,
>      .minimum_version_id = 3,
> +    .pre_save = esp_pre_save,
> +    .post_load = esp_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_BUFFER(rregs, ESPState),
>          VMSTATE_BUFFER(wregs, ESPState),
> @@ -996,9 +1014,10 @@ static void sysbus_esp_init(Object *obj)
>  
>  static const VMStateDescription vmstate_sysbus_esp_scsi = {
>      .name = "sysbusespscsi",
> -    .version_id = 1,
> +    .version_id = 2,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> +        VMSTATE_UINT8_V(esp.mig_version_id, SysBusESPState, 2),
>          VMSTATE_STRUCT(esp, SysBusESPState, 0, vmstate_esp, ESPState),
>          VMSTATE_END_OF_LIST()
>      }
> diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
> index 11c799d91e..7d92471c5b 100644
> --- a/include/hw/scsi/esp.h
> +++ b/include/hw/scsi/esp.h
> @@ -68,6 +68,8 @@ struct ESPState {
>      uint32_t pdma_start;
>      uint32_t pdma_cur;
>      void (*pdma_cb)(ESPState *s);
> +
> +    uint8_t mig_version_id;
>  };
>  
>  #define TYPE_SYSBUS_ESP "sysbus-esp"
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>