[Qemu-devel] [PATCH] Avoid use of QOM type name macros in VMStateDescriptions

[Qemu-devel] [PATCH] Avoid use of QOM type name macros in VMStateDescriptions

[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,
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,
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] Avoid use of QOM type name macros in VMStateDescriptions

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH] Avoid use of QOM type name macros in VMStateDescriptions

[Andreas Färber]

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,
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg