Re: [Qemu-devel] migration: new sections and backward compatibility.

[Gerd Hoffmann <kraxel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

   Hi,

> Not so fast :)
>
> I agree that throwing away unrecognized migration data is unsafe, and
> should not be done.  Now let me present my little problem.
>
> I'm working on making migration preserve "tray status": open/closed,
> locked/unlocked.
>
> For ide-cd, I can stick a subsection "ide_drive/tray_state" into section
> "ide_drive".  Needed only if the tray is open or locked.  This gives
> users a chance to migrate to older versions, and is perfectly safe.
>
> scsi-cd doesn't have a section, yet.  What now?

Experimental patch for usb attached (actually two, the first is pure 
code motion though so the second with the actual changes becomes more 
readable).

That makes migration support switchable using a property, so we can use 
compatibility properties to disable sending the section to an older version.

That requires usb-hid.c call vmstate_register manually.  Not that nice.
We could move the needed() callback from VMStateSubsection to 
VMStateDescription, so it is possible to send complete sections 
conditionally.

Comments?

cheers,
   Gerd

[-- Attachment #2: 0001-move-code.patch --]
[-- Type: text/plain, Size: 3112 bytes --]

>From adf3fd9f870c5ce5566223e1edfb033c81bd01e4 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 7 Jul 2011 15:49:54 +0200
Subject: [PATCH 1/2] move code

---
 hw/usb-hid.c |   86 +++++++++++++++++++++++++++++-----------------------------
 1 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index d711b5c..f30c96b 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -854,49 +854,6 @@ static void usb_hid_handle_destroy(USBDevice *dev)
     }
 }
 
-static int usb_hid_initfn(USBDevice *dev, int kind)
-{
-    USBHIDState *s = DO_UPCAST(USBHIDState, dev, dev);
-
-    usb_desc_init(dev);
-    s->kind = kind;
-
-    if (s->kind == USB_MOUSE) {
-        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_pointer_event, s,
-                                                       0, "QEMU USB Mouse");
-    } else if (s->kind == USB_TABLET) {
-        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_pointer_event, s,
-                                                       1, "QEMU USB Tablet");
-    }
-
-    /* Force poll routine to be run and grab input the first time.  */
-    s->changed = 1;
-    return 0;
-}
-
-static int usb_tablet_initfn(USBDevice *dev)
-{
-    return usb_hid_initfn(dev, USB_TABLET);
-}
-
-static int usb_mouse_initfn(USBDevice *dev)
-{
-    return usb_hid_initfn(dev, USB_MOUSE);
-}
-
-static int usb_keyboard_initfn(USBDevice *dev)
-{
-    return usb_hid_initfn(dev, USB_KEYBOARD);
-}
-
-void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *))
-{
-    USBHIDState *s = (USBHIDState *)dev;
-
-    s->datain_opaque = opaque;
-    s->datain = datain;
-}
-
 static int usb_hid_post_load(void *opaque, int version_id)
 {
     USBHIDState *s = opaque;
@@ -956,6 +913,49 @@ static const VMStateDescription vmstate_usb_kbd = {
     }
 };
 
+static int usb_hid_initfn(USBDevice *dev, int kind)
+{
+    USBHIDState *s = DO_UPCAST(USBHIDState, dev, dev);
+
+    usb_desc_init(dev);
+    s->kind = kind;
+
+    if (s->kind == USB_MOUSE) {
+        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_pointer_event, s,
+                                                       0, "QEMU USB Mouse");
+    } else if (s->kind == USB_TABLET) {
+        s->ptr.eh_entry = qemu_add_mouse_event_handler(usb_pointer_event, s,
+                                                       1, "QEMU USB Tablet");
+    }
+
+    /* Force poll routine to be run and grab input the first time.  */
+    s->changed = 1;
+    return 0;
+}
+
+static int usb_tablet_initfn(USBDevice *dev)
+{
+    return usb_hid_initfn(dev, USB_TABLET);
+}
+
+static int usb_mouse_initfn(USBDevice *dev)
+{
+    return usb_hid_initfn(dev, USB_MOUSE);
+}
+
+static int usb_keyboard_initfn(USBDevice *dev)
+{
+    return usb_hid_initfn(dev, USB_KEYBOARD);
+}
+
+void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *))
+{
+    USBHIDState *s = (USBHIDState *)dev;
+
+    s->datain_opaque = opaque;
+    s->datain = datain;
+}
+
 static struct USBDeviceInfo hid_info[] = {
     {
         .product_desc   = "QEMU USB Tablet",
-- 
1.7.1


[-- Attachment #3: 0002-usb-hid-make-migration-support-conditional.patch --]
[-- Type: text/plain, Size: 3490 bytes --]

>From 8d918be8c56adc099fe4a12e01c5fdfa735df349 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 7 Jul 2011 15:51:40 +0200
Subject: [PATCH 2/2] usb-hid: make migration support conditional

---
 hw/usb-hid.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index f30c96b..81dff27 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -80,6 +80,7 @@ typedef struct USBHIDState {
     int32_t protocol;
     uint8_t idle;
     int64_t next_idle_clock;
+    uint32_t migration;
     int changed;
     void *datain_opaque;
     void (*datain)(void *);
@@ -928,6 +929,17 @@ static int usb_hid_initfn(USBDevice *dev, int kind)
                                                        1, "QEMU USB Tablet");
     }
 
+    if (s->migration) {
+        if (s->kind == USB_KEYBOARD) {
+            vmstate_register(&dev->qdev, -1, &vmstate_usb_kbd, dev);
+        } else {
+            vmstate_register(&dev->qdev, -1, &vmstate_usb_ptr, dev);
+        }
+    } else {
+        /* disable remote-wakeup */
+        usb_desc_set_string(dev, STR_SERIALNUMBER, "1");
+    }
+
     /* Force poll routine to be run and grab input the first time.  */
     s->changed = 1;
     return 0;
@@ -956,13 +968,17 @@ void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *))
     s->datain = datain;
 }
 
+static struct Property hid_props[] = {
+    DEFINE_PROP_UINT32("migration", USBHIDState, migration, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static struct USBDeviceInfo hid_info[] = {
     {
         .product_desc   = "QEMU USB Tablet",
         .qdev.name      = "usb-tablet",
         .usbdevice_name = "tablet",
         .qdev.size      = sizeof(USBHIDState),
-        .qdev.vmsd      = &vmstate_usb_ptr,
         .usb_desc       = &desc_tablet,
         .init           = usb_tablet_initfn,
         .handle_packet  = usb_generic_handle_packet,
@@ -970,12 +986,12 @@ static struct USBDeviceInfo hid_info[] = {
         .handle_control = usb_hid_handle_control,
         .handle_data    = usb_hid_handle_data,
         .handle_destroy = usb_hid_handle_destroy,
+        .qdev.props     = hid_props,
     },{
         .product_desc   = "QEMU USB Mouse",
         .qdev.name      = "usb-mouse",
         .usbdevice_name = "mouse",
         .qdev.size      = sizeof(USBHIDState),
-        .qdev.vmsd      = &vmstate_usb_ptr,
         .usb_desc       = &desc_mouse,
         .init           = usb_mouse_initfn,
         .handle_packet  = usb_generic_handle_packet,
@@ -983,12 +999,12 @@ static struct USBDeviceInfo hid_info[] = {
         .handle_control = usb_hid_handle_control,
         .handle_data    = usb_hid_handle_data,
         .handle_destroy = usb_hid_handle_destroy,
+        .qdev.props     = hid_props,
     },{
         .product_desc   = "QEMU USB Keyboard",
         .qdev.name      = "usb-kbd",
         .usbdevice_name = "keyboard",
         .qdev.size      = sizeof(USBHIDState),
-        .qdev.vmsd      = &vmstate_usb_kbd,
         .usb_desc       = &desc_keyboard,
         .init           = usb_keyboard_initfn,
         .handle_packet  = usb_generic_handle_packet,
@@ -996,6 +1012,7 @@ static struct USBDeviceInfo hid_info[] = {
         .handle_control = usb_hid_handle_control,
         .handle_data    = usb_hid_handle_data,
         .handle_destroy = usb_hid_handle_destroy,
+        .qdev.props     = hid_props,
     },{
         /* end of list */
     }
-- 
1.7.1