[Qemu-devel] [PATCH v2 00/12] virtio migration: simplify vmstate helper

[Qemu-devel] [PATCH v2 00/12] virtio migration: simplify vmstate helper

[Halil Pasic]

As a part of the long term  effort to convert migration to vmstate the
migration of virtio devices was recently partially switched to vmstate
starting with the outer layer (commit 5943124cc "virtio: Migration
helper function and macro" and the subsequent "virtio-*: Wrap in
vmstate" commits). This was done by introducing a vmstate based wrapper
driving the well know virtio migration process. As this transition is
still in progress, some synergies were left unexploited, and some things
can be expressed in a more vmstatish way.

Let us simplify a couple of things and get rid of some code duplication.

NOTE: This series is exploring the suggestions of Paolo (I did my best
to do everything as requested). I still think that we are better of with
a macro that with spelling out the VMStateDescription for each device
separately and redundantly. The LOC balance of the previous version was
-41, this version is at +14 because of the expanded macros.  IMHO the
readability benefit of spelling out the vmsd definitions is questionabe
(but it is beneficial if using ctags or grep). I hope for a good
discussion, but I can live with this version too.

v1 --> v2:
* export VMStateInfo instead of helpers
* change semantic of VMSTATE_VIRTIO_DEVICE
* drop VIRTIO_DEF_DEVICE_VMSD macro, use its expansion instead

Halil Pasic (12):
  virtio: prepare change VMSTATE_VIRTIO_DEVICE macro
  virtio-blk: convert VMSTATE_VIRTIO_DEVICE
  virtio-net: convert VMSTATE_VIRTIO_DEVICE
  virtio-9p: convert VMSTATE_VIRTIO_DEVICE
  virtio-serial: convert VMSTATE_VIRTIO_DEVICE
  virtio-gpu: convert VMSTATE_VIRTIO_DEVICE
  virtio-input: convert VMSTATE_VIRTIO_DEVICE
  virtio-scsi: convert VMSTATE_VIRTIO_DEVICE
  virtio-balloon: convert VMSTATE_VIRTIO_DEVICE
  virtio-rng: convert VMSTATE_VIRTIO_DEVICE
  vhost-vsock: convert VMSTATE_VIRTIO_DEVICE
  virtio: cleanup VMSTATE_VIRTIO_DEVICE

 hw/9pfs/virtio-9p-device.c  | 15 +++++++++------
 hw/block/virtio-blk.c       | 25 +++++++++----------------
 hw/char/virtio-serial-bus.c | 16 +++++++++-------
 hw/display/virtio-gpu.c     | 39 +++++++++++++++++++++++++++------------
 hw/input/virtio-input.c     | 21 +++++++++++----------
 hw/net/virtio-net.c         | 40 +++++++++++++++++++---------------------
 hw/scsi/virtio-scsi.c       | 26 +++++++++-----------------
 hw/virtio/vhost-vsock.c     | 42 +++++++++++++++++++++---------------------
 hw/virtio/virtio-balloon.c  | 15 +++++++++------
 hw/virtio/virtio-rng.c      | 19 +++++++++++--------
 hw/virtio/virtio.c          | 17 ++++++++++++++++-
 include/hw/virtio/virtio.h  | 27 ++++++++-------------------
 12 files changed, 158 insertions(+), 144 deletions(-)

-- 
2.8.4

[Qemu-devel] [PATCH v2 01/12] virtio: prepare change VMSTATE_VIRTIO_DEVICE macro

[Halil Pasic]

In most cases the functions passed to VMSTATE_VIRTIO_DEVICE
only call the virtio_load and virtio_save wrappers. Some include some
pre- and post- massaging too. The massaging is better expressed
as such in the VMStateDescription.

Let us prepare for changing the semantic of the VMSTATE_VIRTIO_DEVICE
macro so that it is more similar to the other VMSTATE_*_DEVICE macros
in a sense that it is a field definition.

The preprocessor conditionals are going to be removed as soon as
every usage is converted to the new semantic.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/virtio/virtio.c         | 21 +++++++++++++++++++++
 include/hw/virtio/virtio.h | 16 ++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 18ce333..2c807dd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1622,6 +1622,27 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size)
     virtio_save(VIRTIO_DEVICE(opaque), f);
 }
 
+/* A wrapper for use as a VMState .put function */
+static void virtio_device_put(QEMUFile *f, void *opaque, size_t size)
+{
+    virtio_save(VIRTIO_DEVICE(opaque), f);
+}
+
+/* A wrapper for use as a VMState .get function */
+static int virtio_device_get(QEMUFile *f, void *opaque, size_t size)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
+    DeviceClass *dc = DEVICE_CLASS(VIRTIO_DEVICE_GET_CLASS(vdev));
+
+    return virtio_load(vdev, f, dc->vmsd->version_id);
+}
+
+const VMStateInfo  virtio_vmstate_info = {
+    .name = "virtio",
+    .get = virtio_device_get,
+    .put = virtio_device_put,
+};
+
 static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 888c8de..db4c125 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -177,6 +177,20 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
 void virtio_save(VirtIODevice *vdev, QEMUFile *f);
 void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
 
+extern const VMStateInfo virtio_vmstate_info;
+
+#ifdef VMSTATE_VIRTIO_DEVICE_USE_NEW
+
+#define VMSTATE_VIRTIO_DEVICE \
+    {                                         \
+        .name = "virtio",                     \
+        .info = &virtio_vmstate_info,         \
+        .flags = VMS_SINGLE,                  \
+    }
+
+#else
+/* TODO remove conditional as soon as all users are converted */
+
 #define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \
     static const VMStateDescription vmstate_virtio_ ## devname = { \
         .name = "virtio-" #devname ,          \
@@ -196,6 +210,8 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
         }                                     \
     }
 
+#endif
+
 int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id);
 
 void virtio_notify_config(VirtIODevice *vdev);
-- 
2.8.4

[Qemu-devel] [PATCH v2 02/12] virtio-blk: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/block/virtio-blk.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 3a6112f..d773453 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -11,6 +11,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
@@ -803,13 +805,6 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
     }
 }
 
-static void virtio_blk_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
-
-    virtio_save(vdev, f);
-}
-    
 static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
 {
     VirtIOBlock *s = VIRTIO_BLK(vdev);
@@ -828,14 +823,6 @@ static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_sbyte(f, 0);
 }
 
-static int virtio_blk_load(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIOBlock *s = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(s);
-
-    return virtio_load(vdev, f, 2);
-}
-
 static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
                                   int version_id)
 {
@@ -956,7 +943,15 @@ static void virtio_blk_instance_init(Object *obj)
                                   DEVICE(obj), NULL);
 }
 
-VMSTATE_VIRTIO_DEVICE(blk, 2, virtio_blk_load, virtio_blk_save);
+static const VMStateDescription vmstate_virtio_blk = {
+    .name = "virtio-blk",
+    .minimum_version_id = 2,
+    .version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_blk_properties[] = {
     DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf),
-- 
2.8.4

[Qemu-devel] [PATCH v2 03/12] virtio-net: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/net/virtio-net.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6b8ae2c..8059457 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -11,6 +11,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "hw/virtio/virtio.h"
@@ -1498,17 +1500,6 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue)
     virtio_net_set_queues(n);
 }
 
-static void virtio_net_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIONet *n = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(n);
-
-    /* At this point, backend must be stopped, otherwise
-     * it might keep writing to memory. */
-    assert(!n->vhost_started);
-    virtio_save(vdev, f);
-}
-
 static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -1544,14 +1535,6 @@ static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f)
     }
 }
 
-static int virtio_net_load(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIONet *n = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(n);
-
-    return virtio_load(vdev, f, VIRTIO_NET_VM_VERSION);
-}
-
 static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f,
                                   int version_id)
 {
@@ -1854,8 +1837,25 @@ static void virtio_net_instance_init(Object *obj)
                                   DEVICE(n), NULL);
 }
 
-VMSTATE_VIRTIO_DEVICE(net, VIRTIO_NET_VM_VERSION, virtio_net_load,
-                      virtio_net_save);
+static void virtio_net_pre_save(void *opaque)
+{
+    VirtIONet *n = opaque;
+
+    /* At this point, backend must be stopped, otherwise
+     * it might keep writing to memory. */
+    assert(!n->vhost_started);
+}
+
+static const VMStateDescription vmstate_virtio_net = {
+    .name = "virtio-net",
+    .minimum_version_id = VIRTIO_NET_VM_VERSION,
+    .version_id = VIRTIO_NET_VM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .pre_save = virtio_net_pre_save,
+};
 
 static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
-- 
2.8.4

[Qemu-devel] [PATCH v2 04/12] virtio-9p: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p-device.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 009b43f..c51dd9f 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -11,6 +11,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
 #include "qemu/sockets.h"
@@ -97,11 +99,6 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config)
     g_free(cfg);
 }
 
-static int virtio_9p_load(QEMUFile *f, void *opaque, size_t size)
-{
-    return virtio_load(VIRTIO_DEVICE(opaque), f, 1);
-}
-
 static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -168,7 +165,15 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
 
 /* virtio-9p device */
 
-VMSTATE_VIRTIO_DEVICE(9p, 1, virtio_9p_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_9p = {
+    .name = "virtio-9p",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_9p_properties[] = {
     DEFINE_PROP_STRING("mount_tag", V9fsVirtioState, state.fsconf.tag),
-- 
2.8.4

[Qemu-devel] [PATCH v2 05/12] virtio-serial: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/char/virtio-serial-bus.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index db57a38..5566e94 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -18,6 +18,8 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
@@ -728,12 +730,6 @@ static int fetch_active_ports_list(QEMUFile *f,
     return 0;
 }
 
-static int virtio_serial_load(QEMUFile *f, void *opaque, size_t size)
-{
-    /* The virtio device */
-    return virtio_load(VIRTIO_DEVICE(opaque), f, 3);
-}
-
 static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
                                      int version_id)
 {
@@ -1075,7 +1071,15 @@ static void virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
 }
 
 /* Note: 'console' is used for backwards compatibility */
-VMSTATE_VIRTIO_DEVICE(console, 3, virtio_serial_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_console = {
+    .name = "virtio-console",
+    .minimum_version_id = 3,
+    .version_id = 3,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_serial_properties[] = {
     DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,
-- 
2.8.4

[Qemu-devel] [PATCH v2 06/12] virtio-gpu: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro. The device virtio-gpu is
special because it actually does not adhere to the virtio migration
schema, because device state is last.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/display/virtio-gpu.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 7fe6ed8..4fcd63c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -11,6 +11,8 @@
  * See the COPYING file in the top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/iov.h"
@@ -990,12 +992,9 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
 static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
 {
     VirtIOGPU *g = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(g);
     struct virtio_gpu_simple_resource *res;
     int i;
 
-    virtio_save(vdev, f);
-
     /* in 2d mode we should never find unprocessed commands here */
     assert(QTAILQ_EMPTY(&g->cmdq));
 
@@ -1020,16 +1019,10 @@ static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
 static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
 {
     VirtIOGPU *g = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(g);
     struct virtio_gpu_simple_resource *res;
     struct virtio_gpu_scanout *scanout;
     uint32_t resource_id, pformat;
-    int i, ret;
-
-    ret = virtio_load(vdev, f, VIRTIO_GPU_VM_VERSION);
-    if (ret) {
-        return ret;
-    }
+    int i;
 
     resource_id = qemu_get_be32(f);
     while (resource_id != 0) {
@@ -1219,8 +1212,32 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 #endif
 }
 
-VMSTATE_VIRTIO_DEVICE(gpu, VIRTIO_GPU_VM_VERSION, virtio_gpu_load,
-                      virtio_gpu_save);
+/*
+ * For historical reasons virtio_gpu does not adhere to virtio migration
+ * scheme as described in doc/virtio-migration.txt, in a sense that no
+ * save/load callback are provided to the core. Instead the device data
+ * is saved/loaded after the core data.
+ *
+ * Because of this we need a special vmsd.
+ */
+static const VMStateDescription vmstate_virtio_gpu = {
+    .name = "virtio-gpu",
+    .minimum_version_id = VIRTIO_GPU_VM_VERSION,
+    .version_id = VIRTIO_GPU_VM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE /* core */,
+        {
+            .name = "virtio-gpu",
+            .info = &(const VMStateInfo) {
+                        .name = "virtio-gpu",
+                        .get = virtio_gpu_load,
+                        .put = virtio_gpu_save,
+            },
+            .flags = VMS_SINGLE,
+        } /* device */,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_gpu_properties[] = {
     DEFINE_PROP_UINT32("max_outputs", VirtIOGPU, conf.max_outputs, 1),
-- 
2.8.4

[Qemu-devel] [PATCH v2 07/12] virtio-input: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/input/virtio-input.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index ccdf730..5e31033 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -4,6 +4,8 @@
  * top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
@@ -217,19 +219,12 @@ static void virtio_input_reset(VirtIODevice *vdev)
     }
 }
 
-static int virtio_input_load(QEMUFile *f, void *opaque, size_t size)
+static int virtio_input_post_load(void *opaque, int version_id)
 {
     VirtIOInput *vinput = opaque;
     VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(vinput);
     VirtIODevice *vdev = VIRTIO_DEVICE(vinput);
-    int ret;
-
-    ret = virtio_load(vdev, f, VIRTIO_INPUT_VM_VERSION);
-    if (ret) {
-        return ret;
-    }
 
-    /* post_load() */
     vinput->active = vdev->status & VIRTIO_CONFIG_S_DRIVER_OK;
     if (vic->change_active) {
         vic->change_active(vinput);
@@ -296,8 +291,16 @@ static void virtio_input_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
-VMSTATE_VIRTIO_DEVICE(input, VIRTIO_INPUT_VM_VERSION, virtio_input_load,
-                      virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_input = {
+    .name = "virtio-input",
+    .minimum_version_id = VIRTIO_INPUT_VM_VERSION,
+    .version_id = VIRTIO_INPUT_VM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .post_load = virtio_input_post_load,
+};
 
 static Property virtio_input_properties[] = {
     DEFINE_PROP_STRING("serial", VirtIOInput, serial),
-- 
2.8.4

[Qemu-devel] [PATCH v2 08/12] virtio-scsi: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/scsi/virtio-scsi.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index e596b64..def316b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -13,6 +13,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "standard-headers/linux/virtio_ids.h"
@@ -663,22 +665,6 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
     s->events_dropped = false;
 }
 
-/* The device does not have anything to save beyond the virtio data.
- * Request data is saved with callbacks from SCSI devices.
- */
-static void virtio_scsi_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
-    virtio_save(vdev, f);
-}
-
-static int virtio_scsi_load(QEMUFile *f, void *opaque, size_t size)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
-
-    return virtio_load(vdev, f, 1);
-}
-
 void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
                             uint32_t event, uint32_t reason)
 {
@@ -921,7 +907,15 @@ static Property virtio_scsi_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-VMSTATE_VIRTIO_DEVICE(scsi, 1, virtio_scsi_load, virtio_scsi_save);
+static const VMStateDescription vmstate_virtio_scsi = {
+    .name = "virtio-scsi",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static void virtio_scsi_common_class_init(ObjectClass *klass, void *data)
 {
-- 
2.8.4

[Qemu-devel] [PATCH v2 09/12] virtio-balloon: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/virtio/virtio-balloon.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 49a2f4a..1014ec6 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -13,6 +13,8 @@
  *
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "qemu/timer.h"
@@ -404,11 +406,6 @@ static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_be32(f, s->actual);
 }
 
-static int virtio_balloon_load(QEMUFile *f, void *opaque, size_t size)
-{
-    return virtio_load(VIRTIO_DEVICE(opaque), f, 1);
-}
-
 static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f,
                                       int version_id)
 {
@@ -494,7 +491,15 @@ static void virtio_balloon_instance_init(Object *obj)
                         NULL, s, NULL);
 }
 
-VMSTATE_VIRTIO_DEVICE(balloon, 1, virtio_balloon_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_balloon = {
+    .name = "virtio-balloon",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static Property virtio_balloon_properties[] = {
     DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features,
-- 
2.8.4

[Qemu-devel] [PATCH v2 10/12] virtio-rng: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/virtio/virtio-rng.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index cd8ca10..62867d1 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -9,6 +9,8 @@
  * top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
@@ -120,15 +122,9 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
     return f;
 }
 
-static int virtio_rng_load(QEMUFile *f, void *opaque, size_t size)
+static int virtio_rng_post_load(void *opaque, int version_id)
 {
     VirtIORNG *vrng = opaque;
-    int ret;
-
-    ret = virtio_load(VIRTIO_DEVICE(vrng), f, 1);
-    if (ret != 0) {
-        return ret;
-    }
 
     /* We may have an element ready but couldn't process it due to a quota
      * limit.  Make sure to try again after live migration when the quota may
@@ -216,7 +212,16 @@ static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
-VMSTATE_VIRTIO_DEVICE(rng, 1, virtio_rng_load, virtio_vmstate_save);
+static const VMStateDescription vmstate_virtio_rng = {
+    .name = "virtio-rng",
+    .minimum_version_id = 1,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .post_load =  virtio_rng_post_load,
+};
 
 static Property virtio_rng_properties[] = {
     /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s.  If
-- 
2.8.4

[Qemu-devel] [PATCH v2 11/12] vhost-vsock: convert VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Use the new VMSTATE_VIRTIO_DEVICE macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 hw/virtio/vhost-vsock.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index bde2456..99cb216 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -11,6 +11,8 @@
  * top-level directory.
  */
 
+#define VMSTATE_VIRTIO_DEVICE_USE_NEW
+
 #include <sys/ioctl.h>
 #include "qemu/osdep.h"
 #include "standard-headers/linux/virtio_vsock.h"
@@ -236,17 +238,6 @@ out:
     g_free(elem);
 }
 
-static void vhost_vsock_save(QEMUFile *f, void *opaque, size_t size)
-{
-    VHostVSock *vsock = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(vsock);
-
-    /* At this point, backend must be stopped, otherwise
-     * it might keep writing to memory. */
-    assert(!vsock->vhost_dev.started);
-    virtio_save(vdev, f);
-}
-
 static void vhost_vsock_post_load_timer_cleanup(VHostVSock *vsock)
 {
     if (!vsock->post_load_timer) {
@@ -266,16 +257,19 @@ static void vhost_vsock_post_load_timer_cb(void *opaque)
     vhost_vsock_send_transport_reset(vsock);
 }
 
-static int vhost_vsock_load(QEMUFile *f, void *opaque, size_t size)
+static void vhost_vsock_pre_save(void *opaque)
 {
     VHostVSock *vsock = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(vsock);
-    int ret;
 
-    ret = virtio_load(vdev, f, VHOST_VSOCK_SAVEVM_VERSION);
-    if (ret) {
-        return ret;
-    }
+    /* At this point, backend must be stopped, otherwise
+     * it might keep writing to memory. */
+    assert(!vsock->vhost_dev.started);
+}
+
+static int vhost_vsock_post_load(void *opaque, int version_id)
+{
+    VHostVSock *vsock = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(vsock);
 
     if (virtio_queue_get_addr(vdev, 2)) {
         /* Defer transport reset event to a vm clock timer so that virtqueue
@@ -288,12 +282,20 @@ static int vhost_vsock_load(QEMUFile *f, void *opaque, size_t size)
                          vsock);
         timer_mod(vsock->post_load_timer, 1);
     }
-
     return 0;
 }
 
-VMSTATE_VIRTIO_DEVICE(vhost_vsock, VHOST_VSOCK_SAVEVM_VERSION,
-                      vhost_vsock_load, vhost_vsock_save);
+static const VMStateDescription vmstate_virtio_vhost_vsock = {
+    .name = "virtio-vhost_vsock",
+    .minimum_version_id = VHOST_VSOCK_SAVEVM_VERSION,
+    .version_id = VHOST_VSOCK_SAVEVM_VERSION,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+    .pre_save = vhost_vsock_pre_save,
+    .post_load = vhost_vsock_post_load,
+};
 
 static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
 {
-- 
2.8.4

[Qemu-devel] [PATCH v2 12/12] virtio: cleanup VMSTATE_VIRTIO_DEVICE

[Halil Pasic]

Now all the usages of the old version of VMSTATE_VIRTIO_DEVICE are gone,
so we can get rid of the conditionals, and the old macro.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---

We could also make virtio_load/save static (private) at this point.
I think we should. Opinions?

---
 hw/9pfs/virtio-9p-device.c  |  2 --
 hw/block/virtio-blk.c       |  2 --
 hw/char/virtio-serial-bus.c |  2 --
 hw/display/virtio-gpu.c     |  2 --
 hw/input/virtio-input.c     |  2 --
 hw/net/virtio-net.c         |  2 --
 hw/scsi/virtio-scsi.c       |  2 --
 hw/virtio/vhost-vsock.c     |  2 --
 hw/virtio/virtio-balloon.c  |  2 --
 hw/virtio/virtio-rng.c      |  2 --
 hw/virtio/virtio.c          |  6 ------
 include/hw/virtio/virtio.h  | 27 ---------------------------
 12 files changed, 53 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index c51dd9f..81a4332 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -11,8 +11,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
 #include "qemu/sockets.h"
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index d773453..3ed3b2c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -11,8 +11,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 5566e94..d4d7abd 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -18,8 +18,6 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4fcd63c..fa6fd0e 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -11,8 +11,6 @@
  * See the COPYING file in the top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/iov.h"
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 5e31033..b678ee9 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -4,8 +4,6 @@
  * top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 8059457..4d33e3c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -11,8 +11,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "hw/virtio/virtio.h"
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index def316b..4bb1495 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -13,8 +13,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "standard-headers/linux/virtio_ids.h"
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 99cb216..b481562 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -11,8 +11,6 @@
  * top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include <sys/ioctl.h>
 #include "qemu/osdep.h"
 #include "standard-headers/linux/virtio_vsock.h"
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1014ec6..a655cf7 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -13,8 +13,6 @@
  *
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qemu/iov.h"
 #include "qemu/timer.h"
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 62867d1..9639f4e 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -9,8 +9,6 @@
  * top-level directory.
  */
 
-#define VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/iov.h"
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2c807dd..db92b1d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1617,12 +1617,6 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 }
 
 /* A wrapper for use as a VMState .put function */
-void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size)
-{
-    virtio_save(VIRTIO_DEVICE(opaque), f);
-}
-
-/* A wrapper for use as a VMState .put function */
 static void virtio_device_put(QEMUFile *f, void *opaque, size_t size)
 {
     virtio_save(VIRTIO_DEVICE(opaque), f);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db4c125..8e4dc8c 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -175,12 +175,9 @@ bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq);
 void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
 
 void virtio_save(VirtIODevice *vdev, QEMUFile *f);
-void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
 
 extern const VMStateInfo virtio_vmstate_info;
 
-#ifdef VMSTATE_VIRTIO_DEVICE_USE_NEW
-
 #define VMSTATE_VIRTIO_DEVICE \
     {                                         \
         .name = "virtio",                     \
@@ -188,30 +185,6 @@ extern const VMStateInfo virtio_vmstate_info;
         .flags = VMS_SINGLE,                  \
     }
 
-#else
-/* TODO remove conditional as soon as all users are converted */
-
-#define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \
-    static const VMStateDescription vmstate_virtio_ ## devname = { \
-        .name = "virtio-" #devname ,          \
-        .minimum_version_id = v,              \
-        .version_id = v,                      \
-        .fields = (VMStateField[]) {          \
-            {                                 \
-                .name = "virtio",             \
-                .info = &(const VMStateInfo) {\
-                        .name = "virtio",     \
-                        .get = getf,          \
-                        .put = putf,          \
-                    },                        \
-                .flags = VMS_SINGLE,          \
-            },                                \
-            VMSTATE_END_OF_LIST()             \
-        }                                     \
-    }
-
-#endif
-
 int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id);
 
 void virtio_notify_config(VirtIODevice *vdev);
-- 
2.8.4

Re: [Qemu-devel] [PATCH v2 00/12] virtio migration: simplify vmstate helper

[Paolo Bonzini]

On 06/10/2016 14:55, Halil Pasic wrote:
> 
> Let us simplify a couple of things and get rid of some code duplication.
> 
> NOTE: This series is exploring the suggestions of Paolo (I did my best
> to do everything as requested). I still think that we are better of with
> a macro that with spelling out the VMStateDescription for each device
> separately and redundantly. The LOC balance of the previous version was
> -41, this version is at +14 because of the expanded macros.  IMHO the
> readability benefit of spelling out the vmsd definitions is questionabe
> (but it is beneficial if using ctags or grep). I hope for a good
> discussion, but I can live with this version too.
> 
> v1 --> v2:
> * export VMStateInfo instead of helpers
> * change semantic of VMSTATE_VIRTIO_DEVICE
> * drop VIRTIO_DEF_DEVICE_VMSD macro, use its expansion instead

Yes, this is what I meant...  I personally like that everything is
spelled out in

+static const VMStateDescription vmstate_virtio_blk = {
+    .name = "virtio-blk",
+    .minimum_version_id = 2,
+    .version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};

but I understand that other's mileages may vary...

Paolo

Re: [Qemu-devel] [PATCH v2 00/12] virtio migration: simplify vmstate helper

[Halil Pasic]

On 10/06/2016 05:30 PM, Paolo Bonzini wrote:
> 
> 
> On 06/10/2016 14:55, Halil Pasic wrote:
>>
>> Let us simplify a couple of things and get rid of some code duplication.
>>
>> NOTE: This series is exploring the suggestions of Paolo (I did my best
>> to do everything as requested). I still think that we are better of with
>> a macro that with spelling out the VMStateDescription for each device
>> separately and redundantly. The LOC balance of the previous version was
>> -41, this version is at +14 because of the expanded macros.  IMHO the
>> readability benefit of spelling out the vmsd definitions is questionabe
>> (but it is beneficial if using ctags or grep). I hope for a good
>> discussion, but I can live with this version too.
>>
>> v1 --> v2:
>> * export VMStateInfo instead of helpers
>> * change semantic of VMSTATE_VIRTIO_DEVICE
>> * drop VIRTIO_DEF_DEVICE_VMSD macro, use its expansion instead
> 
> Yes, this is what I meant...  I personally like that everything is
> spelled out in
> 
> +static const VMStateDescription vmstate_virtio_blk = {
> +    .name = "virtio-blk",
> +    .minimum_version_id = 2,
> +    .version_id = 2,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_VIRTIO_DEVICE,
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> 

Is a valid point :). There is one important thing that is not spelled
out here, and that's the state specific to virtio_blk. In my opinion
this definition suggests that virtio_blk has no state beyond its
base's (in therms of inheritance, that is virtio core+transport) state, 
and this is wrong. That is probably the main reason why I prefer
the 'magic macro' (others are my preference for the concise, and for
making similar stuff look similar, and dissimilar things look different).

This however can be remedied if we do:
in virtio.h:
extern const VMStateField virtio_vmstate_fields[];
in virtio.c:
const VMStateField virtio_vmstate_fields[] = {
    VMSTATE_VIRTIO_DEVICE,
    VMSTATE_END_OF_LIST()
};

in virtio-blk.c:
static const VMStateDescription vmstate_virtio_blk = {
    .name = "virtio-blk",
    .minimum_version_id = 2,
    .version_id = 2,
    .fields = virtio_vmstate_fields,
};

And would need to fix const correctness for VMStateField in
vmstate.c and vmstate.h.

I have never thought of this because of the const but right
how I feel like I like this option the best:
* it works with grep and ctags
* its absolutely flexible
* the oddity of irtio_vmstate_fields can be documented at
  the declaration/definition site
* we could later use VMSTATE_VIRTIO_DEVICE with the usual VMSTATE_PCI_DEVICE
  like semantic for the new devices once we establish a new migration schema
  in which the the derived specifies its state using the fields member of
  its vmsd
* redundancy is minimal:
** having separate control over minimum_version_id and version_id
   seems appropriate to me and having it written out improves readability
** establishing a naming convention for the vmsd is not so important and
   the one line we can spare there with the macro is not worth it

What do you thing guys should I make a v3 along this path?

Cheers,
Halil