Patchwork [2/9] vmstate: complain about devices without vmstate

login
register
mail settings
Submitter Gerd Hoffmann
Date July 20, 2011, 10:09 a.m.
Message ID <1311156579-9814-3-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/105658/
State New
Headers show

Comments

Gerd Hoffmann - July 20, 2011, 10:09 a.m.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/hw.h   |    2 ++
 hw/qdev.c |    7 ++++++-
 2 files changed, 8 insertions(+), 1 deletions(-)
Peter Maydell - July 20, 2011, 12:40 p.m.
On 20 July 2011 11:09, Gerd Hoffmann <kraxel@redhat.com> wrote:
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -283,7 +283,12 @@ int qdev_init(DeviceState *dev)
>         qdev_free(dev);
>         return rc;
>     }
> -    if (dev->info->vmsd) {
> +    if (dev->info->vmsd == NULL) {
> +        /* TODO: fixup qemu source code, then make this an assert() */
> +        error_report("WARNING: device %s has no vmstate\n", dev->info->name);
> +    } else if (dev->info->vmsd == VMSD_NONE) {
> +        /* device doesn't need vmstate */;
> +    } else {

I would prefer it if we didn't add this sort of targeted-at-qemu-developers
warning unless there was a reasonable period of time before the next release
where devices which provoke the warning message can be fixed. In particular,
should we postpone putting in the warning message until after 0.15 branches?

-- PMM
Gerd Hoffmann - July 20, 2011, 12:51 p.m.
On 07/20/11 14:40, Peter Maydell wrote:
> On 20 July 2011 11:09, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -283,7 +283,12 @@ int qdev_init(DeviceState *dev)
>>          qdev_free(dev);
>>          return rc;
>>      }
>> -    if (dev->info->vmsd) {
>> +    if (dev->info->vmsd == NULL) {
>> +        /* TODO: fixup qemu source code, then make this an assert() */
>> +        error_report("WARNING: device %s has no vmstate\n", dev->info->name);
>> +    } else if (dev->info->vmsd == VMSD_NONE) {
>> +        /* device doesn't need vmstate */;
>> +    } else {
>
> I would prefer it if we didn't add this sort of targeted-at-qemu-developers
> warning unless there was a reasonable period of time before the next release
> where devices which provoke the warning message can be fixed. In particular,
> should we postpone putting in the warning message until after 0.15 branches?

Agree, makes sense to postpone that one to 0.16-devel, otherwise we'll 
just bug the users for no reason.

Pushed migration.2 branch with that patch dropped.

cheers,
   Gerd

Patch

diff --git a/hw/hw.h b/hw/hw.h
index df6ca65..d839af1 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -336,6 +336,8 @@  struct VMStateDescription {
     const VMStateSubsection *subsections;
 };
 
+#define VMSD_NONE ((const VMStateDescription*)1)
+
 extern const VMStateInfo vmstate_info_bool;
 
 extern const VMStateInfo vmstate_info_int8;
diff --git a/hw/qdev.c b/hw/qdev.c
index 292b52f..fafbbae 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -283,7 +283,12 @@  int qdev_init(DeviceState *dev)
         qdev_free(dev);
         return rc;
     }
-    if (dev->info->vmsd) {
+    if (dev->info->vmsd == NULL) {
+        /* TODO: fixup qemu source code, then make this an assert() */
+        error_report("WARNING: device %s has no vmstate\n", dev->info->name);
+    } else if (dev->info->vmsd == VMSD_NONE) {
+        /* device doesn't need vmstate */;
+    } else {
         vmstate_register_with_alias_id(dev, -1, dev->info->vmsd, dev,
                                        dev->instance_id_alias,
                                        dev->alias_required_for_version);