Eric Auger <eric.au...@redhat.com> wrote:
> Introduce support for GTree migration. A custom save/restore
> is implemented. Each item is made of a key and a data.
>
> If the key is a pointer to an object, 2 VMSDs are passed into
> the GTree VMStateField.
>
> When putting the items, the tree is traversed in sorted order by
> g_tree_foreach.
>
> On the get() path, gtrees must be allocated using the proper
> key compare, key destroy and value destroy. This must be handled
> beforehand, for example in a pre_load method.
>
> Tests are added to test save/dump of structs containing gtrees
> including the virtio-iommu domain/mappings scenario.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
Overal I like the patch. I think that I found a minor error.
> +static int put_gtree(QEMUFile *f, void *pv, size_t unused_size,
> + const VMStateField *field, QJSON *vmdesc)
> +{
> + bool direct_key = (!field->start);
> + const VMStateDescription *key_vmsd = direct_key ? NULL : &field->vmsd[0];
> + const VMStateDescription *val_vmsd = direct_key ? &field->vmsd[0] :
> + &field->vmsd[1];
> + struct put_gtree_data capsule = {f, key_vmsd, val_vmsd, 0};
Please, consider change the last line to:
struct put_gtree_data capsule = {
.f = f,
.key_vmsd = key_vmsd,
.val_vmsd = val_vmsd,
.ret = 0};
It makes much easier make changes on the future.
Once here, I think that you are missing on the middle a:
.vmdesc = vmdesc,
No? At least you use it on put_gtree_elem, and I don't see any place
where you assign it. When I did the change is when I noticed that it
was missing.
> + GTree **pval = pv;
> + GTree *tree = *pval;
> + int ret;
> +
> + qemu_put_be32(f, g_tree_nnodes(tree));
> + g_tree_foreach(tree, put_gtree_elem, (gpointer)&capsule);
> + qemu_put_byte(f, false);
> + ret = capsule.ret;
> + if (ret) {
> + error_report("%s : failed to save gtree (%d)", field->name, ret);
> + }
> + return ret;
> +}
As said before, with this changes you have my reviewed-by.
Later, Juan.
