* [Qemu-devel] [RFC 0/6] converting some of virtio to VMState @ 2016-08-24 13:42 Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 1/6] migration: report an error giving the failed field Dr. David Alan Gilbert (git) ` (6 more replies) 0 siblings, 7 replies; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Hi, This series converts two parts of virtio to VMState - the device load/save for virtio-balloon and virtio-net. It's only been smoke tested (which it passes); but if anyone has a good suggestion for testing virtio-net migration I'd be greatful. The first couple of patches are just general error reporting improvements for vmstate; and could go in straight away. The 3rd adds a new vmstate macro for skipping chunks of input data The 4th wires in a call to vmstate_{load|save}_state in virtio_{load|save} calling the vmsd on virtio device class; this is instead of the current ->load/->save methods. The idea is that eventually I'll be able to kill off the ->load/->save. Also my intention is to add the recursion into vdc->vmsd into the main vmstate_virtio vmsd via some new macro I've not figured out yet. The meat is in the 5th and 6th patches that do balloon and virtio-net. My plan is to attack virtio_blk and virtio_serial_bus next; both of which have loops walking along lists (in subtly different ways); I intend to see how Jianjun Duan's loop migration code would fit with those and try and come up with something general that does at least all 3 cases. Dave Dr. David Alan Gilbert (6): migration: report an error giving the failed field migration: Report values for comparisons migration: Add VMSTATE_UNUSED_VARRAY_UINT32 virtio/migration: Add VMStateDescription to VirtioDeviceClass virtio/migration: Migrate balloon to VMState virtio/migration: Migrate virtio-net to VMState hw/net/virtio-net.c | 256 ++++++++++++++++++++++++----------------- hw/virtio/virtio-balloon.c | 31 +++-- hw/virtio/virtio.c | 11 ++ include/hw/virtio/virtio-net.h | 10 +- include/hw/virtio/virtio.h | 2 + include/migration/vmstate.h | 11 ++ migration/vmstate.c | 10 ++ 7 files changed, 206 insertions(+), 125 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC 1/6] migration: report an error giving the failed field 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 ` Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 2/6] migration: Report values for comparisons Dr. David Alan Gilbert (git) ` (5 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> When a field fails to load (typically due to a limit check, or a call to a get/put) report the device and field to give an indication of the cause. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- migration/vmstate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/migration/vmstate.c b/migration/vmstate.c index fc29acf..1d637b2 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -130,6 +130,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, } if (ret < 0) { qemu_file_set_error(f, ret); + error_report("Failed to load %s:%s", vmsd->name, + field->name); trace_vmstate_load_field_error(field->name, ret); return ret; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC 2/6] migration: Report values for comparisons 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 1/6] migration: report an error giving the failed field Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 ` Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 3/6] migration: Add VMSTATE_UNUSED_VARRAY_UINT32 Dr. David Alan Gilbert (git) ` (4 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Report the values when a comparison fails; together with the previous patch that prints the device and field names this should give a good idea of why loading the migration failed. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- migration/vmstate.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/migration/vmstate.c b/migration/vmstate.c index 1d637b2..ec7fafc 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -557,6 +557,7 @@ static int get_int32_equal(QEMUFile *f, void *pv, size_t size) if (*v == v2) { return 0; } + error_report("%" PRId32 " != %" PRId32, *v, v2); return -EINVAL; } @@ -580,6 +581,9 @@ static int get_int32_le(QEMUFile *f, void *pv, size_t size) *cur = loaded; return 0; } + error_report("Invalid value %" PRId32 + " expecting postiive value <= %" PRId32, + loaded, *cur); return -EINVAL; } @@ -685,6 +689,7 @@ static int get_uint32_equal(QEMUFile *f, void *pv, size_t size) if (*v == v2) { return 0; } + error_report("%" PRIu32 " != %" PRIu32, *v, v2); return -EINVAL; } @@ -727,6 +732,7 @@ static int get_uint64_equal(QEMUFile *f, void *pv, size_t size) if (*v == v2) { return 0; } + error_report("%" PRIu64 " != %" PRIu64, *v, v2); return -EINVAL; } @@ -748,6 +754,7 @@ static int get_uint8_equal(QEMUFile *f, void *pv, size_t size) if (*v == v2) { return 0; } + error_report("%u != %u", *v, v2); return -EINVAL; } @@ -769,6 +776,7 @@ static int get_uint16_equal(QEMUFile *f, void *pv, size_t size) if (*v == v2) { return 0; } + error_report("%u != %u", *v, v2); return -EINVAL; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC 3/6] migration: Add VMSTATE_UNUSED_VARRAY_UINT32 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 1/6] migration: report an error giving the failed field Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 2/6] migration: Report values for comparisons Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 ` Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass Dr. David Alan Gilbert (git) ` (3 subsequent siblings) 6 siblings, 0 replies; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> VMSTATE_UNUSED_VARRAY_UINT32 is used to skip a chunk of the stream that's an n-element array; note the array size and the dynamic value read never get multiplied so there's no overflow risk. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- include/migration/vmstate.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 1638ee5..a0eff5e 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -643,6 +643,17 @@ extern const VMStateInfo vmstate_info_bitmap; .flags = VMS_BUFFER, \ } +/* Discard size * field_num bytes, where field_num is a uint32 member */ +#define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\ + .name = "unused", \ + .field_exists = (_test), \ + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\ + .version_id = (_version), \ + .size = (_size), \ + .info = &vmstate_info_unused_buffer, \ + .flags = VMS_VARRAY_UINT32 | VMS_BUFFER, \ +} + /* _field_size should be a int32_t field in the _state struct giving the * size of the bitmap _field in bits. */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) ` (2 preceding siblings ...) 2016-08-24 13:42 ` [Qemu-devel] [RFC 3/6] migration: Add VMSTATE_UNUSED_VARRAY_UINT32 Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 ` Dr. David Alan Gilbert (git) 2016-08-24 15:29 ` Cornelia Huck 2016-08-24 13:42 ` [Qemu-devel] [RFC 5/6] virtio/migration: Migrate balloon to VMState Dr. David Alan Gilbert (git) ` (2 subsequent siblings) 6 siblings, 1 reply; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Provide a vmsd pointer for VirtIO devices to use instead of the load/save methods. We'll eventually kill off the load/save methods. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- hw/virtio/virtio.c | 11 +++++++++++ include/hw/virtio/virtio.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 15ee3a7..3a4fa93 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1470,6 +1470,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) vdc->save(vdev, f); } + if (vdc->vmsd) { + vmstate_save_state(f, vdc->vmsd, vdev, NULL); + } + /* Subsections */ vmstate_save_state(f, &vmstate_virtio, vdev, NULL); } @@ -1601,6 +1605,13 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) } } + if (vdc->vmsd) { + ret = vmstate_load_state(f, vdc->vmsd, vdev, version_id); + if (ret) { + return ret; + } + } + /* Subsections */ ret = vmstate_load_state(f, &vmstate_virtio, vdev, 1); if (ret) { diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index d2490c1..fd386ac 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -124,8 +124,10 @@ typedef struct VirtioDeviceClass { * must mask in frontend instead. */ void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask); + /* Saving and loading of a device; use *either* save/load OR vmsd */ void (*save)(VirtIODevice *vdev, QEMUFile *f); int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id); + const VMStateDescription *vmsd; } VirtioDeviceClass; void virtio_instance_init_common(Object *proxy_obj, void *data, -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass 2016-08-24 13:42 ` [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass Dr. David Alan Gilbert (git) @ 2016-08-24 15:29 ` Cornelia Huck 2016-08-24 15:42 ` Dr. David Alan Gilbert 0 siblings, 1 reply; 12+ messages in thread From: Cornelia Huck @ 2016-08-24 15:29 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, mst, amit.shah, quintela, duanj On Wed, 24 Aug 2016 14:42:31 +0100 "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Provide a vmsd pointer for VirtIO devices to use instead of the > load/save methods. > > We'll eventually kill off the load/save methods. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > hw/virtio/virtio.c | 11 +++++++++++ > include/hw/virtio/virtio.h | 2 ++ > 2 files changed, 13 insertions(+) > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index d2490c1..fd386ac 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -124,8 +124,10 @@ typedef struct VirtioDeviceClass { > * must mask in frontend instead. > */ > void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask); > + /* Saving and loading of a device; use *either* save/load OR vmsd */ Should we try to enforce this in some way? Then virtio_{save,load} can call either/or instead of fallthrough which may have unintended consequences... > void (*save)(VirtIODevice *vdev, QEMUFile *f); > int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id); > + const VMStateDescription *vmsd; > } VirtioDeviceClass; > > void virtio_instance_init_common(Object *proxy_obj, void *data, ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass 2016-08-24 15:29 ` Cornelia Huck @ 2016-08-24 15:42 ` Dr. David Alan Gilbert 2016-08-24 15:51 ` Cornelia Huck 0 siblings, 1 reply; 12+ messages in thread From: Dr. David Alan Gilbert @ 2016-08-24 15:42 UTC (permalink / raw) To: Cornelia Huck; +Cc: qemu-devel, mst, amit.shah, quintela, duanj * Cornelia Huck (cornelia.huck@de.ibm.com) wrote: > On Wed, 24 Aug 2016 14:42:31 +0100 > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > Provide a vmsd pointer for VirtIO devices to use instead of the > > load/save methods. > > > > We'll eventually kill off the load/save methods. > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > hw/virtio/virtio.c | 11 +++++++++++ > > include/hw/virtio/virtio.h | 2 ++ > > 2 files changed, 13 insertions(+) > > > > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > index d2490c1..fd386ac 100644 > > --- a/include/hw/virtio/virtio.h > > +++ b/include/hw/virtio/virtio.h > > @@ -124,8 +124,10 @@ typedef struct VirtioDeviceClass { > > * must mask in frontend instead. > > */ > > void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask); > > + /* Saving and loading of a device; use *either* save/load OR vmsd */ > > Should we try to enforce this in some way? Then virtio_{save,load} can > call either/or instead of fallthrough which may have unintended > consequences... I was thinking of doing that; but my intention is to kill off the save/load methods ASAP. Dave > > > void (*save)(VirtIODevice *vdev, QEMUFile *f); > > int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id); > > + const VMStateDescription *vmsd; > > } VirtioDeviceClass; > > > > void virtio_instance_init_common(Object *proxy_obj, void *data, > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass 2016-08-24 15:42 ` Dr. David Alan Gilbert @ 2016-08-24 15:51 ` Cornelia Huck 0 siblings, 0 replies; 12+ messages in thread From: Cornelia Huck @ 2016-08-24 15:51 UTC (permalink / raw) To: Dr. David Alan Gilbert; +Cc: qemu-devel, mst, amit.shah, quintela, duanj On Wed, 24 Aug 2016 16:42:05 +0100 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote: > > On Wed, 24 Aug 2016 14:42:31 +0100 > > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > > index d2490c1..fd386ac 100644 > > > --- a/include/hw/virtio/virtio.h > > > +++ b/include/hw/virtio/virtio.h > > > @@ -124,8 +124,10 @@ typedef struct VirtioDeviceClass { > > > * must mask in frontend instead. > > > */ > > > void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask); > > > + /* Saving and loading of a device; use *either* save/load OR vmsd */ > > > > Should we try to enforce this in some way? Then virtio_{save,load} can > > call either/or instead of fallthrough which may have unintended > > consequences... > > I was thinking of doing that; but my intention is to kill off the save/load > methods ASAP. Fair enough, but doing a check for "did you by accident supply both?" might be helpful during development. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC 5/6] virtio/migration: Migrate balloon to VMState 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) ` (3 preceding siblings ...) 2016-08-24 13:42 ` [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 ` Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 6/6] virtio/migration: Migrate virtio-net " Dr. David Alan Gilbert (git) 2016-08-24 15:34 ` [Qemu-devel] [RFC 0/6] converting some of virtio " Cornelia Huck 6 siblings, 0 replies; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Replace the load/save with a vmsd. Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- hw/virtio/virtio-balloon.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 5af429a..7b0ad20 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -396,26 +396,14 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target) trace_virtio_balloon_to_target(target, dev->num_pages); } -static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f) -{ - VirtIOBalloon *s = VIRTIO_BALLOON(vdev); - - qemu_put_be32(f, s->num_pages); - 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) +static int virtio_balloon_post_load_device(void *opaque, int version_id) { - VirtIOBalloon *s = VIRTIO_BALLOON(vdev); - - s->num_pages = qemu_get_be32(f); - s->actual = qemu_get_be32(f); + VirtIOBalloon *s = VIRTIO_BALLOON(opaque); if (balloon_stats_enabled(s)) { balloon_stats_change_timer(s, s->stats_poll_interval); @@ -423,6 +411,18 @@ static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f, return 0; } +static const VMStateDescription vmstate_virtio_balloon_device = { + .name = "virtio-balloon-device", + .version_id = 1, + .minimum_version_id = 1, + .post_load = virtio_balloon_post_load_device, + .fields = (VMStateField[]) { + VMSTATE_UINT32(num_pages, VirtIOBalloon), + VMSTATE_UINT32(actual, VirtIOBalloon), + VMSTATE_END_OF_LIST() + }, +}; + static void virtio_balloon_device_realize(DeviceState *dev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); @@ -503,8 +503,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data) vdc->get_config = virtio_balloon_get_config; vdc->set_config = virtio_balloon_set_config; vdc->get_features = virtio_balloon_get_features; - vdc->save = virtio_balloon_save_device; - vdc->load = virtio_balloon_load_device; + vdc->vmsd = &vmstate_virtio_balloon_device; } static const TypeInfo virtio_balloon_info = { -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC 6/6] virtio/migration: Migrate virtio-net to VMState 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) ` (4 preceding siblings ...) 2016-08-24 13:42 ` [Qemu-devel] [RFC 5/6] virtio/migration: Migrate balloon to VMState Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 ` Dr. David Alan Gilbert (git) 2016-08-24 15:34 ` [Qemu-devel] [RFC 0/6] converting some of virtio " Cornelia Huck 6 siblings, 0 replies; 12+ messages in thread From: Dr. David Alan Gilbert (git) @ 2016-08-24 13:42 UTC (permalink / raw) To: qemu-devel, mst, amit.shah, quintela; +Cc: duanj, cornelia.huck From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Only lightly smoke-tested so far Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> --- hw/net/virtio-net.c | 256 ++++++++++++++++++++++++----------------- include/hw/virtio/virtio-net.h | 10 +- 2 files changed, 157 insertions(+), 109 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 01f1351..a06d07e 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1503,127 +1503,44 @@ static void virtio_net_save(QEMUFile *f, void *opaque, size_t size) virtio_save(vdev, f); } -static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f) -{ - VirtIONet *n = VIRTIO_NET(vdev); - int i; - - qemu_put_buffer(f, n->mac, ETH_ALEN); - qemu_put_be32(f, n->vqs[0].tx_waiting); - qemu_put_be32(f, n->mergeable_rx_bufs); - qemu_put_be16(f, n->status); - qemu_put_byte(f, n->promisc); - qemu_put_byte(f, n->allmulti); - qemu_put_be32(f, n->mac_table.in_use); - qemu_put_buffer(f, n->mac_table.macs, n->mac_table.in_use * ETH_ALEN); - qemu_put_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3); - qemu_put_be32(f, n->has_vnet_hdr); - qemu_put_byte(f, n->mac_table.multi_overflow); - qemu_put_byte(f, n->mac_table.uni_overflow); - qemu_put_byte(f, n->alluni); - qemu_put_byte(f, n->nomulti); - qemu_put_byte(f, n->nouni); - qemu_put_byte(f, n->nobcast); - qemu_put_byte(f, n->has_ufo); - if (n->max_queues > 1) { - qemu_put_be16(f, n->max_queues); - qemu_put_be16(f, n->curr_queues); - for (i = 1; i < n->curr_queues; i++) { - qemu_put_be32(f, n->vqs[i].tx_waiting); - } - } - - if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { - qemu_put_be64(f, n->curr_guest_offloads); - } -} - -static int virtio_net_load(QEMUFile *f, void *opaque, size_t size) +static int virtio_net_post_load_device(void *opaque, int version_id) { 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) -{ - VirtIONet *n = VIRTIO_NET(vdev); int i, link_down; + bool ufo_on_wire; - qemu_get_buffer(f, n->mac, ETH_ALEN); - n->vqs[0].tx_waiting = qemu_get_be32(f); - - virtio_net_set_mrg_rx_bufs(n, qemu_get_be32(f), - virtio_vdev_has_feature(vdev, - VIRTIO_F_VERSION_1)); - - n->status = qemu_get_be16(f); - - n->promisc = qemu_get_byte(f); - n->allmulti = qemu_get_byte(f); - - n->mac_table.in_use = qemu_get_be32(f); - /* MAC_TABLE_ENTRIES may be different from the saved image */ - if (n->mac_table.in_use <= MAC_TABLE_ENTRIES) { - qemu_get_buffer(f, n->mac_table.macs, - n->mac_table.in_use * ETH_ALEN); - } else { - int64_t i; - - /* Overflow detected - can happen if source has a larger MAC table. - * We simply set overflow flag so there's no need to maintain the - * table of addresses, discard them all. - * Note: 64 bit math to avoid integer overflow. - */ - for (i = 0; i < (int64_t)n->mac_table.in_use * ETH_ALEN; ++i) { - qemu_get_byte(f); - } - n->mac_table.multi_overflow = n->mac_table.uni_overflow = 1; - n->mac_table.in_use = 0; - } - - qemu_get_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3); - - if (qemu_get_be32(f) && !peer_has_vnet_hdr(n)) { + /* The received has_vnet_hdr is only compared against what the configured + * state, we always end up using the configured state we stashed. + */ + if (n->has_vnet_hdr && !n->mig_has_vnet_hdr) { error_report("virtio-net: saved image requires vnet_hdr=on"); return -1; } + n->has_vnet_hdr = n->mig_has_vnet_hdr; - n->mac_table.multi_overflow = qemu_get_byte(f); - n->mac_table.uni_overflow = qemu_get_byte(f); - - n->alluni = qemu_get_byte(f); - n->nomulti = qemu_get_byte(f); - n->nouni = qemu_get_byte(f); - n->nobcast = qemu_get_byte(f); - - if (qemu_get_byte(f) && !peer_has_ufo(n)) { + /* The read has_ufo value is only tested; the used value is either + * the value prior to migration (which preload stashed in mig_has_ufo) + * OR a value that peer_has_ufo sets during the following test. + */ + ufo_on_wire = n->has_ufo; + n->has_ufo = n->mig_has_ufo; + if (ufo_on_wire && !peer_has_ufo(n)) { error_report("virtio-net: saved image requires TUN_F_UFO support"); return -1; } - if (n->max_queues > 1) { - if (n->max_queues != qemu_get_be16(f)) { - error_report("virtio-net: different max_queues "); - return -1; - } + virtio_net_set_mrg_rx_bufs(n, n->mergeable_rx_bufs, + virtio_vdev_has_feature(vdev, + VIRTIO_F_VERSION_1)); - n->curr_queues = qemu_get_be16(f); - if (n->curr_queues > n->max_queues) { - error_report("virtio-net: curr_queues %x > max_queues %x", - n->curr_queues, n->max_queues); - return -1; - } - for (i = 1; i < n->curr_queues; i++) { - n->vqs[i].tx_waiting = qemu_get_be32(f); - } + /* MAC_TABLE_ENTRIES may be different from the saved image */ + if (n->mac_table.in_use > MAC_TABLE_ENTRIES) { + /* TODO: Should we or in multi_overflow/uni_overflow ? */ + n->mac_table.in_use = 0; } - if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { - n->curr_guest_offloads = qemu_get_be64(f); - } else { + if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { n->curr_guest_offloads = virtio_net_supported_guest_offloads(n); } @@ -1657,6 +1574,132 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, return 0; } +static int virtio_net_pre_load_device(void *opaque) +{ + VirtIONet *n = opaque; + n->mig_has_vnet_hdr = peer_has_vnet_hdr(n); + n->mig_has_ufo = n->has_ufo; + + return 0; +} + +/* tx_waiting field of a VirtIONetQueue */ +static const VMStateDescription vmstate_virtio_net_queue_tx_waiting = { + .name = "virtio-net-queue-tx_waiting", + .fields = (VMStateField[]) { + VMSTATE_UINT32(tx_waiting, VirtIONetQueue), + VMSTATE_END_OF_LIST() + }, +}; + +static bool max_queues_gt_1(void *opaque, int version_id) +{ + return VIRTIO_NET(opaque)->max_queues > 1; +} + +static bool has_ctrl_guest_offloads(void *opaque, int version_id) +{ + return virtio_vdev_has_feature(VIRTIO_DEVICE(opaque), + VIRTIO_NET_F_CTRL_GUEST_OFFLOADS); +} + +static bool mac_table_fits(void *opaque, int version_id) +{ + return VIRTIO_NET(opaque)->mac_table.in_use <= MAC_TABLE_ENTRIES; +} + +static bool mac_table_doesnt_fit(void *opaque, int version_id) +{ + return !mac_table_fits(opaque, version_id); +} + +/* NOTE! Has side effect - it updates mig_curr_queues_1 for the + * varray macro after it. + */ +static bool validate_curr_queues(void *opaque, int version_id) +{ + VirtIONet *n = opaque; + + /* This pair make it easy to migrate all-but-the-first element + * of vqs. + */ + n->mig_vqs_1 = n->vqs + 1; + n->mig_curr_queues_1 = 0; + if (n->curr_queues > 1) { + if (n->curr_queues > n->max_queues) { + error_report("virtio-net: curr_queues %x > max_queues %x", + n->curr_queues, n->max_queues); + return false; + } + + n->mig_curr_queues_1 = n->curr_queues - 1; + } + + return true; /* All good */ +} + +static const VMStateDescription vmstate_virtio_net_device = { + .name = "virtio-net-device", + .version_id = VIRTIO_NET_VM_VERSION, + .minimum_version_id = VIRTIO_NET_VM_VERSION, + .pre_load = virtio_net_pre_load_device, + .post_load = virtio_net_post_load_device, + .fields = (VMStateField[]) { + VMSTATE_UINT8_ARRAY(mac, VirtIONet, ETH_ALEN), + VMSTATE_STRUCT_POINTER(vqs, VirtIONet, + vmstate_virtio_net_queue_tx_waiting, + VirtIONetQueue), + VMSTATE_UINT32(mergeable_rx_bufs, VirtIONet), + VMSTATE_UINT16(status, VirtIONet), + VMSTATE_UINT8(promisc, VirtIONet), + VMSTATE_UINT8(allmulti, VirtIONet), + VMSTATE_UINT32(mac_table.in_use, VirtIONet), + + /* Guarded pair: If it fits we load it, else we throw it away + * - can happen if source has a larger MAC table.; post-load + * sets flags in this case. + */ + VMSTATE_VBUFFER_MULTIPLY(mac_table.macs, VirtIONet, + 0, mac_table_fits, 0, mac_table.in_use, + ETH_ALEN), + VMSTATE_UNUSED_VARRAY_UINT32(VirtIONet, mac_table_doesnt_fit, 0, + mac_table.in_use, ETH_ALEN), + + /* Note: This is an array of uint32's that's always been saved as a + * buffer; hold onto your endiannesses; it's actually used as a bitmap + * but based on the uint. + */ + VMSTATE_BUFFER_POINTER_UNSAFE(vlans, VirtIONet, 0, MAX_VLAN >> 3), + VMSTATE_UINT32(has_vnet_hdr, VirtIONet), + VMSTATE_UINT8(mac_table.multi_overflow, VirtIONet), + VMSTATE_UINT8(mac_table.uni_overflow, VirtIONet), + VMSTATE_UINT8(alluni, VirtIONet), + VMSTATE_UINT8(nomulti, VirtIONet), + VMSTATE_UINT8(nouni, VirtIONet), + VMSTATE_UINT8(nobcast, VirtIONet), + VMSTATE_UINT8(has_ufo, VirtIONet), + VMSTATE_SINGLE_TEST(max_queues, VirtIONet, max_queues_gt_1, 0, + vmstate_info_uint16_equal, uint16_t), + VMSTATE_UINT16_TEST(curr_queues, VirtIONet, max_queues_gt_1), + VMSTATE_VALIDATE("curr_queues <= max_queues", validate_curr_queues), + VMSTATE_STRUCT_VARRAY_POINTER_UINT16(mig_vqs_1, VirtIONet, + mig_curr_queues_1, + vmstate_virtio_net_queue_tx_waiting, + VirtIONetQueue), + VMSTATE_UINT64_TEST(curr_guest_offloads, VirtIONet, + has_ctrl_guest_offloads), + VMSTATE_END_OF_LIST() + }, +}; + +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 NetClientInfo net_virtio_info = { .type = NET_CLIENT_DRIVER_NIC, .size = sizeof(NICState), @@ -1902,8 +1945,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data) vdc->set_status = virtio_net_set_status; vdc->guest_notifier_mask = virtio_net_guest_notifier_mask; vdc->guest_notifier_pending = virtio_net_guest_notifier_pending; - vdc->load = virtio_net_load_device; - vdc->save = virtio_net_save_device; + vdc->vmsd = &vmstate_virtio_net_device; } static const TypeInfo virtio_net_info = { diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 91ed97c..d86ee19 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -45,7 +45,7 @@ typedef struct VirtIONetQueue { VirtQueue *tx_vq; QEMUTimer *tx_timer; QEMUBH *tx_bh; - int tx_waiting; + uint32_t tx_waiting; struct { VirtQueueElement *elem; } async_tx; @@ -66,7 +66,7 @@ typedef struct VirtIONet { size_t guest_hdr_len; uint32_t host_features; uint8_t has_ufo; - int mergeable_rx_bufs; + uint32_t mergeable_rx_bufs; uint8_t promisc; uint8_t allmulti; uint8_t alluni; @@ -95,6 +95,12 @@ typedef struct VirtIONet { QEMUTimer *announce_timer; int announce_counter; bool needs_vnet_hdr_swap; + + /* Helper variables just for migration - see validate_curr_queues */ + VirtIONetQueue *mig_vqs_1; /* vqs+1 */ + uint16_t mig_curr_queues_1; /* vqs-1 */ + bool mig_has_vnet_hdr; /* Stashed copy during load */ + bool mig_has_ufo; /* Stashed copy during load */ } VirtIONet; void virtio_net_set_netclient_name(VirtIONet *n, const char *name, -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC 0/6] converting some of virtio to VMState 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) ` (5 preceding siblings ...) 2016-08-24 13:42 ` [Qemu-devel] [RFC 6/6] virtio/migration: Migrate virtio-net " Dr. David Alan Gilbert (git) @ 2016-08-24 15:34 ` Cornelia Huck 2016-08-24 16:20 ` Dr. David Alan Gilbert 6 siblings, 1 reply; 12+ messages in thread From: Cornelia Huck @ 2016-08-24 15:34 UTC (permalink / raw) To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, mst, amit.shah, quintela, duanj On Wed, 24 Aug 2016 14:42:27 +0100 "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Hi, > This series converts two parts of virtio to VMState - the > device load/save for virtio-balloon and virtio-net. > > It's only been smoke tested (which it passes); but if anyone > has a good suggestion for testing virtio-net migration > I'd be greatful. > > The first couple of patches are just general error reporting improvements > for vmstate; and could go in straight away. > The 3rd adds a new vmstate macro for skipping chunks of input data > > The 4th wires in a call to vmstate_{load|save}_state in virtio_{load|save} > calling the vmsd on virtio device class; this is instead of the current ->load/->save > methods. The idea is that eventually I'll be able to kill off the ->load/->save. > Also my intention is to add the recursion into vdc->vmsd into the main vmstate_virtio > vmsd via some new macro I've not figured out yet. > > The meat is in the 5th and 6th patches that do balloon and virtio-net. > > My plan is to attack virtio_blk and virtio_serial_bus next; both of which > have loops walking along lists (in subtly different ways); I intend > to see how Jianjun Duan's loop migration code would fit with those and > try and come up with something general that does at least all 3 cases. I've had a quick glance at your changes and they look sane (at least up to the balloon patch; I'll need some quiet time to look at the virtio-net changes). I'll try to see when I can give it a bit of testing. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC 0/6] converting some of virtio to VMState 2016-08-24 15:34 ` [Qemu-devel] [RFC 0/6] converting some of virtio " Cornelia Huck @ 2016-08-24 16:20 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 12+ messages in thread From: Dr. David Alan Gilbert @ 2016-08-24 16:20 UTC (permalink / raw) To: Cornelia Huck; +Cc: qemu-devel, mst, amit.shah, quintela, duanj * Cornelia Huck (cornelia.huck@de.ibm.com) wrote: > On Wed, 24 Aug 2016 14:42:27 +0100 > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > Hi, > > This series converts two parts of virtio to VMState - the > > device load/save for virtio-balloon and virtio-net. > > > > It's only been smoke tested (which it passes); but if anyone > > has a good suggestion for testing virtio-net migration > > I'd be greatful. > > > > The first couple of patches are just general error reporting improvements > > for vmstate; and could go in straight away. > > The 3rd adds a new vmstate macro for skipping chunks of input data > > > > The 4th wires in a call to vmstate_{load|save}_state in virtio_{load|save} > > calling the vmsd on virtio device class; this is instead of the current ->load/->save > > methods. The idea is that eventually I'll be able to kill off the ->load/->save. > > Also my intention is to add the recursion into vdc->vmsd into the main vmstate_virtio > > vmsd via some new macro I've not figured out yet. > > > > The meat is in the 5th and 6th patches that do balloon and virtio-net. > > > > My plan is to attack virtio_blk and virtio_serial_bus next; both of which > > have loops walking along lists (in subtly different ways); I intend > > to see how Jianjun Duan's loop migration code would fit with those and > > try and come up with something general that does at least all 3 cases. > > I've had a quick glance at your changes and they look sane (at least up > to the balloon patch; I'll need some quiet time to look at the > virtio-net changes). I'll try to see when I can give it a bit of testing. I wouldn't worry about testing it too much yet; but if you've got some suggestions for how I should test it, I'd be greatful. Dave > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-08-24 16:20 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-24 13:42 [Qemu-devel] [RFC 0/6] converting some of virtio to VMState Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 1/6] migration: report an error giving the failed field Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 2/6] migration: Report values for comparisons Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 3/6] migration: Add VMSTATE_UNUSED_VARRAY_UINT32 Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 4/6] virtio/migration: Add VMStateDescription to VirtioDeviceClass Dr. David Alan Gilbert (git) 2016-08-24 15:29 ` Cornelia Huck 2016-08-24 15:42 ` Dr. David Alan Gilbert 2016-08-24 15:51 ` Cornelia Huck 2016-08-24 13:42 ` [Qemu-devel] [RFC 5/6] virtio/migration: Migrate balloon to VMState Dr. David Alan Gilbert (git) 2016-08-24 13:42 ` [Qemu-devel] [RFC 6/6] virtio/migration: Migrate virtio-net " Dr. David Alan Gilbert (git) 2016-08-24 15:34 ` [Qemu-devel] [RFC 0/6] converting some of virtio " Cornelia Huck 2016-08-24 16:20 ` Dr. David Alan Gilbert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.