diff mbox

Avoid use of QOM type name macros in VMStateDescriptions

Message ID 1372331024-3783-1-git-send-email-peter.maydell@linaro.org
State Accepted
Headers show

Commit Message

Peter Maydell June 27, 2013, 11:03 a.m. UTC
The name field in a VMStateDescription is part of the migration state
versioning, so changing it will break migration.  It's therefore a
bad idea to use a QOM typename macro to initialize it, because in
general we're free to rename QOM types as part of code refactoring
and cleanup.  For the handful of devices that were doing this by
mistake, replace the QOM typenames with the corresponding literal
strings.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
As per recent discussion. There are also a few devices which use
the typename in memory_region_init_io(). Since that is suboptimal
but not a problem in the way that possible migration breaks would
be, I haven't fixed those since they'd just clash with Paolo's
memory-region-owner patches.

The one I didn't touch was hw/usb/host-linux.c, since that changes
the QOM typename and the VMStateDescription name depending on
whether QEMU was built with CONFIG_USB_LIBUSB defined or not.
That seems a bit fishy to me but I've left it alone.

 hw/i2c/exynos4210_i2c.c       |    2 +-
 hw/scsi/vmw_pvscsi.c          |    2 +-
 hw/timer/imx_epit.c           |    2 +-
 hw/timer/imx_gpt.c            |    2 +-
 hw/usb/ccid-card-passthru.c   |    2 +-
 hw/usb/dev-smartcard-reader.c |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

Comments

Gerd Hoffmann June 27, 2013, 11:41 a.m. UTC | #1
On 06/27/13 13:03, Peter Maydell wrote:
> The one I didn't touch was hw/usb/host-linux.c, since that changes
> the QOM typename and the VMStateDescription name depending on
> whether QEMU was built with CONFIG_USB_LIBUSB defined or not.
> That seems a bit fishy to me but I've left it alone.

vmstate probably should get a fixed "usb-host" name.

QOM changing is intentional and should stay that way, I want host-libusb
be the default when available, and host-linux be
available under another name, basically to simplify regression
testing in the phase of transitioning to host-libusb.

Long-term the whole host-linux (and host-bsd) code will simply
go away, so don't worry too much ;)

cheers,
  Gerd
Andreas Färber June 30, 2013, 7:20 a.m. UTC | #2
Am 27.06.2013 13:03, schrieb Peter Maydell:
> The name field in a VMStateDescription is part of the migration state
> versioning, so changing it will break migration.  It's therefore a
> bad idea to use a QOM typename macro to initialize it, because in
> general we're free to rename QOM types as part of code refactoring
> and cleanup.  For the handful of devices that were doing this by
> mistake, replace the QOM typenames with the corresponding literal
> strings.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> As per recent discussion. There are also a few devices which use
> the typename in memory_region_init_io(). Since that is suboptimal
> but not a problem in the way that possible migration breaks would
> be, I haven't fixed those since they'd just clash with Paolo's
> memory-region-owner patches.
> 
> The one I didn't touch was hw/usb/host-linux.c, since that changes
> the QOM typename and the VMStateDescription name depending on
> whether QEMU was built with CONFIG_USB_LIBUSB defined or not.
> That seems a bit fishy to me but I've left it alone.
> 
>  hw/i2c/exynos4210_i2c.c       |    2 +-
>  hw/scsi/vmw_pvscsi.c          |    2 +-
>  hw/timer/imx_epit.c           |    2 +-
>  hw/timer/imx_gpt.c            |    2 +-
>  hw/usb/ccid-card-passthru.c   |    2 +-
>  hw/usb/dev-smartcard-reader.c |    2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i2c/exynos4210_i2c.c b/hw/i2c/exynos4210_i2c.c
> index 196f889..a75abef 100644
> --- a/hw/i2c/exynos4210_i2c.c
> +++ b/hw/i2c/exynos4210_i2c.c
> @@ -271,7 +271,7 @@ static const MemoryRegionOps exynos4210_i2c_ops = {
>  };
>  
>  static const VMStateDescription exynos4210_i2c_vmstate = {
> -    .name = TYPE_EXYNOS4_I2C,
> +    .name = "exynos4210.i2c",
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 7cf4044..f2f0c00 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -1136,7 +1136,7 @@ pvscsi_post_load(void *opaque, int version_id)
>  }
>  
>  static const VMStateDescription vmstate_pvscsi = {
> -    .name = TYPE_PVSCSI,
> +    .name = "pvscsi",
>      .version_id = 0,
>      .minimum_version_id = 0,
>      .minimum_version_id_old = 0,

Apparently someone confused TypeInfo and VMStateDescription here, the
TypeInfo .name is by contrast not using the constant - fixing up.

Rest is verified to match constants.

Applied to my new qom-next staging tree:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

> diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
> index 7cdb006..8cefd74a 100644
> --- a/hw/timer/imx_epit.c
> +++ b/hw/timer/imx_epit.c
> @@ -370,7 +370,7 @@ static const MemoryRegionOps imx_epit_ops = {
>  };
>  
>  static const VMStateDescription vmstate_imx_timer_epit = {
> -    .name = TYPE_IMX_EPIT,
> +    .name = "imx.epit",
>      .version_id = 2,
>      .minimum_version_id = 2,
>      .minimum_version_id_old = 2,
> diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
> index de53b13..eebd2b7 100644
> --- a/hw/timer/imx_gpt.c
> +++ b/hw/timer/imx_gpt.c
> @@ -142,7 +142,7 @@ typedef struct {
>  } IMXGPTState;
>  
>  static const VMStateDescription vmstate_imx_timer_gpt = {
> -    .name = TYPE_IMX_GPT,
> +    .name = "imx.gpt",
>      .version_id = 3,
>      .minimum_version_id = 3,
>      .minimum_version_id_old = 3,
> diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
> index 01c7e6f..5f01ff1 100644
> --- a/hw/usb/ccid-card-passthru.c
> +++ b/hw/usb/ccid-card-passthru.c
> @@ -364,7 +364,7 @@ static int passthru_exitfn(CCIDCardState *base)
>  }
>  
>  static VMStateDescription passthru_vmstate = {
> -    .name = PASSTHRU_DEV_NAME,
> +    .name = "ccid-card-passthru",
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index 125cc2c..b33eb25 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -1397,7 +1397,7 @@ static VMStateDescription usb_device_vmstate = {
>  };
>  
>  static VMStateDescription ccid_vmstate = {
> -    .name = CCID_DEV_NAME,
> +    .name = "usb-ccid",
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .post_load = ccid_post_load,
>
diff mbox

Patch

diff --git a/hw/i2c/exynos4210_i2c.c b/hw/i2c/exynos4210_i2c.c
index 196f889..a75abef 100644
--- a/hw/i2c/exynos4210_i2c.c
+++ b/hw/i2c/exynos4210_i2c.c
@@ -271,7 +271,7 @@  static const MemoryRegionOps exynos4210_i2c_ops = {
 };
 
 static const VMStateDescription exynos4210_i2c_vmstate = {
-    .name = TYPE_EXYNOS4_I2C,
+    .name = "exynos4210.i2c",
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 7cf4044..f2f0c00 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1136,7 +1136,7 @@  pvscsi_post_load(void *opaque, int version_id)
 }
 
 static const VMStateDescription vmstate_pvscsi = {
-    .name = TYPE_PVSCSI,
+    .name = "pvscsi",
     .version_id = 0,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index 7cdb006..8cefd74a 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -370,7 +370,7 @@  static const MemoryRegionOps imx_epit_ops = {
 };
 
 static const VMStateDescription vmstate_imx_timer_epit = {
-    .name = TYPE_IMX_EPIT,
+    .name = "imx.epit",
     .version_id = 2,
     .minimum_version_id = 2,
     .minimum_version_id_old = 2,
diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
index de53b13..eebd2b7 100644
--- a/hw/timer/imx_gpt.c
+++ b/hw/timer/imx_gpt.c
@@ -142,7 +142,7 @@  typedef struct {
 } IMXGPTState;
 
 static const VMStateDescription vmstate_imx_timer_gpt = {
-    .name = TYPE_IMX_GPT,
+    .name = "imx.gpt",
     .version_id = 3,
     .minimum_version_id = 3,
     .minimum_version_id_old = 3,
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 01c7e6f..5f01ff1 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -364,7 +364,7 @@  static int passthru_exitfn(CCIDCardState *base)
 }
 
 static VMStateDescription passthru_vmstate = {
-    .name = PASSTHRU_DEV_NAME,
+    .name = "ccid-card-passthru",
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 125cc2c..b33eb25 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1397,7 +1397,7 @@  static VMStateDescription usb_device_vmstate = {
 };
 
 static VMStateDescription ccid_vmstate = {
-    .name = CCID_DEV_NAME,
+    .name = "usb-ccid",
     .version_id = 1,
     .minimum_version_id = 1,
     .post_load = ccid_post_load,