* Eric Auger (eric.au...@redhat.com) wrote:
> Support QLIST migration using the same principle as QTAILQ:
> 94869d5c52 ("migration: migrate QTAILQ").
>
> The VMSTATE_QLIST_V macro has the same proto as VMSTATE_QTAILQ_V.
> The change mainly resides in QLIST RAW macros: QLIST_RAW_INSERT_HEAD
> and QLIST_RAW_REVERSE.
>
> Tests also are provided.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>
> ---
>
> v5 - v6:
> - by doing more advanced testing with virtio-iommu migration
> I noticed this was broken. "prev" field was not set properly.
> I improved the tests to manipulate both the next and prev
> fields.
> - Removed Peter and Juan's R-b
> ---
> include/migration/vmstate.h | 21 +++++
> include/qemu/queue.h | 39 +++++++++
> migration/trace-events | 5 ++
> migration/vmstate-types.c | 70 +++++++++++++++
> tests/test-vmstate.c | 170 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 305 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index ac4f46a67d..08683d93c6 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -227,6 +227,7 @@ extern const VMStateInfo vmstate_info_tmp;
> extern const VMStateInfo vmstate_info_bitmap;
> extern const VMStateInfo vmstate_info_qtailq;
> extern const VMStateInfo vmstate_info_gtree;
> +extern const VMStateInfo vmstate_info_qlist;
>
> #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
> /*
> @@ -796,6 +797,26 @@ extern const VMStateInfo vmstate_info_gtree;
> .offset = offsetof(_state, _field),
> \
> }
>
> +/*
> + * For migrating a QLIST
> + * Target QLIST needs be properly initialized.
> + * _type: type of QLIST element
> + * _next: name of QLIST_ENTRY entry field in QLIST element
> + * _vmsd: VMSD for QLIST element
> + * size: size of QLIST element
> + * start: offset of QLIST_ENTRY in QTAILQ element
> + */
> +#define VMSTATE_QLIST_V(_field, _state, _version, _vmsd, _type, _next) \
> +{ \
> + .name = (stringify(_field)), \
> + .version_id = (_version), \
> + .vmsd = &(_vmsd), \
> + .size = sizeof(_type), \
> + .info = &vmstate_info_qlist, \
> + .offset = offsetof(_state, _field), \
> + .start = offsetof(_type, _next), \
> +}
> +
> /* _f : field name
> _f_n : num of elements field_name
> _n : num of elements
> diff --git a/include/qemu/queue.h b/include/qemu/queue.h
> index 4764d93ea3..4d4554a7ce 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -501,4 +501,43 @@ union {
> \
> QTAILQ_RAW_TQH_CIRC(head)->tql_prev = QTAILQ_RAW_TQE_CIRC(elm,
> entry); \
> } while (/*CONSTCOND*/0)
>
> +#define QLIST_RAW_FIRST(head)
> \
> + field_at_offset(head, 0, void *)
> +
> +#define QLIST_RAW_NEXT(elm, entry)
> \
> + field_at_offset(elm, entry, void *)
> +
> +#define QLIST_RAW_PREVIOUS(elm, entry)
> \
> + field_at_offset(elm, entry + sizeof(void *), void *)
> +
> +#define QLIST_RAW_FOREACH(elm, head, entry)
> \
> + for ((elm) = *QLIST_RAW_FIRST(head);
> \
> + (elm);
> \
> + (elm) = *QLIST_RAW_NEXT(elm, entry))
> +
> +#define QLIST_RAW_INSERT_HEAD(head, elm, entry) do {
> \
> + void *first = *QLIST_RAW_FIRST(head);
> \
> + *QLIST_RAW_FIRST(head) = elm;
> \
> + *QLIST_RAW_PREVIOUS(elm, entry) = QLIST_RAW_FIRST(head);
> \
> + if (first) {
> \
> + *QLIST_RAW_NEXT(elm, entry) = first;
> \
> + *QLIST_RAW_PREVIOUS(first, entry) = QLIST_RAW_NEXT(elm, entry);
> \
> + } else {
> \
> + *QLIST_RAW_NEXT(elm, entry) = NULL;
> \
> + }
> \
> +} while (0)
> +
> +#define QLIST_RAW_REVERSE(head, elm, entry) do {
> \
> + void *iter = *QLIST_RAW_FIRST(head), *prev = NULL, *next;
> \
> + while (iter) {
> \
> + next = *QLIST_RAW_NEXT(iter, entry);
> \
> + *QLIST_RAW_PREVIOUS(iter, entry) = QLIST_RAW_NEXT(next, entry);
> \
> + *QLIST_RAW_NEXT(iter, entry) = prev;
> \
> + prev = iter;
> \
> + iter = next;
> \
> + }
> \
> + *QLIST_RAW_FIRST(head) = prev;
> \
> + *QLIST_RAW_PREVIOUS(prev, entry) = QLIST_RAW_FIRST(head);
> \
> +} while (0)
> +
> #endif /* QEMU_SYS_QUEUE_H */
> diff --git a/migration/trace-events b/migration/trace-events
> index 6dee7b5389..e0a33cffca 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -76,6 +76,11 @@ get_gtree_end(const char *field_name, const char
> *key_vmsd_name, const char *val
> put_gtree(const char *field_name, const char *key_vmsd_name, const char
> *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d"
> put_gtree_end(const char *field_name, const char *key_vmsd_name, const char
> *val_vmsd_name, int ret) "%s(%s/%s) %d"
>
> +get_qlist(const char *field_name, const char *vmsd_name, int version_id)
> "%s(%s v%d)"
> +get_qlist_end(const char *field_name, const char *vmsd_name) "%s(%s)"
> +put_qlist(const char *field_name, const char *vmsd_name, int version_id)
> "%s(%s v%d)"
> +put_qlist_end(const char *field_name, const char *vmsd_name) "%s(%s)"
> +
> # qemu-file.c
> qemu_file_fclose(void) ""
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index 7236cf92bc..1eee36773a 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -843,3 +843,73 @@ const VMStateInfo vmstate_info_gtree = {
> .get = get_gtree,
> .put = put_gtree,
> };
> +
> +static int put_qlist(QEMUFile *f, void *pv, size_t unused_size,
> + const VMStateField *field, QJSON *vmdesc)
> +{
> + const VMStateDescription *vmsd = field->vmsd;
> + /* offset of the QTAILQ entry in a QTAILQ element*/
> + size_t entry_offset = field->start;
> + void *elm;
> + int ret;
> +
> + trace_put_qlist(field->name, vmsd->name, vmsd->version_id);
> + QLIST_RAW_FOREACH(elm, pv, entry_offset) {
> + qemu_put_byte(f, true);
> + ret = vmstate_save_state(f, vmsd, elm, vmdesc);
> + if (ret) {
> + error_report("%s: failed to save %s (%d)", field->name,
> + vmsd->name, ret);
> + return ret;
> + }
> + }
> + qemu_put_byte(f, false);
> + trace_put_qlist_end(field->name, vmsd->name);
> +
> + return 0;
> +}
> +
> +static int get_qlist(QEMUFile *f, void *pv, size_t unused_size,
> + const VMStateField *field)
> +{
> + int ret = 0;
> + const VMStateDescription *vmsd = field->vmsd;
> + /* size of a QLIST element */
> + size_t size = field->size;
> + /* offset of the QLIST entry in a QLIST element */
> + size_t entry_offset = field->start;
> + int version_id = field->version_id;
> + void *elm;
> +
> + trace_get_qlist(field->name, vmsd->name, vmsd->version_id);
> + if (version_id > vmsd->version_id) {
> + error_report("%s %s", vmsd->name, "too new");
> + return -EINVAL;
> + }
> + if (version_id < vmsd->minimum_version_id) {
> + error_report("%s %s", vmsd->name, "too old");
> + return -EINVAL;
> + }
> +
> + while (qemu_get_byte(f)) {
> + elm = g_malloc(size);
> + ret = vmstate_load_state(f, vmsd, elm, version_id);
> + if (ret) {
> + error_report("%s: failed to load %s (%d)", field->name,
> + vmsd->name, ret);
> + g_free(elm);
> + return ret;
> + }
> + QLIST_RAW_INSERT_HEAD(pv, elm, entry_offset);
> + }
> + QLIST_RAW_REVERSE(pv, elm, entry_offset);
Can you explain why you need to do a REVERSE on the loaded list,
rather than using doing a QLIST_INSERT_AFTER to always insert at
the end?
Other than that it looks good.
Dave
> + trace_get_qlist_end(field->name, vmsd->name);
> +
> + return ret;
> +}
> +
> +const VMStateInfo vmstate_info_qlist = {
> + .name = "qlist",
> + .get = get_qlist,
> + .put = put_qlist,
> +};
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index 1e5be1d4ff..9660f932b9 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -926,6 +926,28 @@ static const VMStateDescription vmstate_domain = {
> }
> };
>
> +/* test QLIST Migration */
> +
> +typedef struct TestQListElement {
> + uint32_t id;
> + QLIST_ENTRY(TestQListElement) next;
> +} TestQListElement;
> +
> +typedef struct TestQListContainer {
> + uint32_t id;
> + QLIST_HEAD(, TestQListElement) list;
> +} TestQListContainer;
> +
> +static const VMStateDescription vmstate_qlist_element = {
> + .name = "test/queue list",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(id, TestQListElement),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_iommu = {
> .name = "iommu",
> .version_id = 1,
> @@ -939,6 +961,18 @@ static const VMStateDescription vmstate_iommu = {
> }
> };
>
> +static const VMStateDescription vmstate_container = {
> + .name = "test/container/qlist",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(id, TestQListContainer),
> + VMSTATE_QLIST_V(list, TestQListContainer, 1, vmstate_qlist_element,
> + TestQListElement, next),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> uint8_t first_domain_dump[] = {
> /* id */
> 0x00, 0x0, 0x0, 0x6,
> @@ -1229,6 +1263,140 @@ static void test_gtree_load_iommu(void)
> qemu_fclose(fload);
> }
>
> +static uint8_t qlist_dump[] = {
> + 0x00, 0x00, 0x00, 0x01, /* container id */
> + 0x1, /* start of a */
> + 0x00, 0x00, 0x00, 0x0a,
> + 0x1, /* start of b */
> + 0x00, 0x00, 0x0b, 0x00,
> + 0x1, /* start of c */
> + 0x00, 0x0c, 0x00, 0x00,
> + 0x1, /* start of d */
> + 0x0d, 0x00, 0x00, 0x00,
> + 0x0, /* end of list */
> + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
> +};
> +
> +static TestQListContainer *alloc_container(void)
> +{
> + TestQListElement *a = g_malloc(sizeof(TestQListElement));
> + TestQListElement *b = g_malloc(sizeof(TestQListElement));
> + TestQListElement *c = g_malloc(sizeof(TestQListElement));
> + TestQListElement *d = g_malloc(sizeof(TestQListElement));
> + TestQListContainer *container = g_malloc(sizeof(TestQListContainer));
> +
> + a->id = 0x0a;
> + b->id = 0x0b00;
> + c->id = 0xc0000;
> + d->id = 0xd000000;
> + container->id = 1;
> +
> + QLIST_INIT(&container->list);
> + QLIST_INSERT_HEAD(&container->list, d, next);
> + QLIST_INSERT_HEAD(&container->list, c, next);
> + QLIST_INSERT_HEAD(&container->list, b, next);
> + QLIST_INSERT_HEAD(&container->list, a, next);
> + return container;
> +}
> +
> +static void free_container(TestQListContainer *container)
> +{
> + TestQListElement *iter, *tmp;
> +
> + QLIST_FOREACH_SAFE(iter, &container->list, next, tmp) {
> + QLIST_REMOVE(iter, next);
> + g_free(iter);
> + }
> + g_free(container);
> +}
> +
> +static void compare_containers(TestQListContainer *c1, TestQListContainer
> *c2)
> +{
> + TestQListElement *first_item_c1, *first_item_c2;
> +
> + while (!QLIST_EMPTY(&c1->list)) {
> + first_item_c1 = QLIST_FIRST(&c1->list);
> + first_item_c2 = QLIST_FIRST(&c2->list);
> + assert(first_item_c2);
> + assert(first_item_c1->id == first_item_c2->id);
> + QLIST_REMOVE(first_item_c1, next);
> + QLIST_REMOVE(first_item_c2, next);
> + g_free(first_item_c1);
> + g_free(first_item_c2);
> + }
> + assert(QLIST_EMPTY(&c2->list));
> +}
> +
> +/*
> + * Check the prev & next fields are correct by doing list
> + * manipulations on the container. We will do that for both
> + * the source and the destination containers
> + */
> +static void manipulate_container(TestQListContainer *c)
> +{
> + TestQListElement *prev, *iter = QLIST_FIRST(&c->list);
> + TestQListElement *elem;
> +
> + elem = g_malloc(sizeof(TestQListElement));
> + elem->id = 0x12;
> + QLIST_INSERT_AFTER(iter, elem, next);
> +
> + elem = g_malloc(sizeof(TestQListElement));
> + elem->id = 0x13;
> + QLIST_INSERT_HEAD(&c->list, elem, next);
> +
> + while (iter) {
> + prev = iter;
> + iter = QLIST_NEXT(iter, next);
> + }
> +
> + elem = g_malloc(sizeof(TestQListElement));
> + elem->id = 0x14;
> + QLIST_INSERT_BEFORE(prev, elem, next);
> +
> + elem = g_malloc(sizeof(TestQListElement));
> + elem->id = 0x15;
> + QLIST_INSERT_AFTER(prev, elem, next);
> +
> + QLIST_REMOVE(prev, next);
> + g_free(prev);
> +}
> +
> +static void test_save_qlist(void)
> +{
> + TestQListContainer *container = alloc_container();
> +
> + save_vmstate(&vmstate_container, container);
> + compare_vmstate(qlist_dump, sizeof(qlist_dump));
> + free_container(container);
> +}
> +
> +static void test_load_qlist(void)
> +{
> + QEMUFile *fsave, *fload;
> + TestQListContainer *orig_container = alloc_container();
> + TestQListContainer *dest_container =
> g_malloc0(sizeof(TestQListContainer));
> + char eof;
> +
> + QLIST_INIT(&dest_container->list);
> +
> + fsave = open_test_file(true);
> + qemu_put_buffer(fsave, qlist_dump, sizeof(qlist_dump));
> + g_assert(!qemu_file_get_error(fsave));
> + qemu_fclose(fsave);
> +
> + fload = open_test_file(false);
> + vmstate_load_state(fload, &vmstate_container, dest_container, 1);
> + eof = qemu_get_byte(fload);
> + g_assert(!qemu_file_get_error(fload));
> + g_assert_cmpint(eof, ==, QEMU_VM_EOF);
> + manipulate_container(orig_container);
> + manipulate_container(dest_container);
> + compare_containers(orig_container, dest_container);
> + free_container(orig_container);
> + free_container(dest_container);
> +}
> +
> typedef struct TmpTestStruct {
> TestStruct *parent;
> int64_t diff;
> @@ -1353,6 +1521,8 @@ int main(int argc, char **argv)
> g_test_add_func("/vmstate/gtree/load/loaddomain",
> test_gtree_load_domain);
> g_test_add_func("/vmstate/gtree/save/saveiommu", test_gtree_save_iommu);
> g_test_add_func("/vmstate/gtree/load/loadiommu", test_gtree_load_iommu);
> + g_test_add_func("/vmstate/qlist/save/saveqlist", test_save_qlist);
> + g_test_add_func("/vmstate/qlist/load/loadqlist", test_load_qlist);
> g_test_add_func("/vmstate/tmp_struct", test_tmp_struct);
> g_test_run();
>
> --
> 2.20.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
