Re: [PATCH v6] migration: Support gtree migration
Eric Auger 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 Reviewed-by: Juan Quintela
Re: [PATCH v6] migration: Support gtree migration
Hi,
On 10/11/19 5:01 PM, Dr. David Alan Gilbert wrote:
> * 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
>
> I've applied:
> @@ -1070,8 +1070,8 @@ static gboolean match_domain_node(gpointer key,
> gpointer value, gpointer data)
> TestGTreeDomain *d1, *d2;
> struct match_node_data *d = (struct match_node_data *)data;
>
> -id1 = (uint64_t)key;
> -id2 = (uint64_t)d->key;
> +id1 = (uint64_t)(uintptr_t)key;
> +id2 = (uint64_t)(uintptr_t)d->key;
> d1 = (TestGTreeDomain *)value;
> d2 = (TestGTreeDomain *)d->value;
> assert(id1 == id2);
>
> to get the tests happy in 32bit.
Thank you for taking care of this.
Eric
>
>>
>> ---
>>
>> v4 -> v5:
>> - swap key/value in vmsd array
>> - fix g_free(key) in case of direct key
>> - return an error if the number of decoded nodes does not
>> match nnodes
>>
>> v3 -> v4:
>> - properly initialize capsule.vmdesc in put_gtree()
>> - use uintptr_t
>> - add trace points
>>
>> v2 -> v3:
>> - do not include glib.h anymore
>> - introduce VMSTATE_GTREE_DIRECT_KEY_V
>> - use pre_load to allocate the tree and remove data pointer
>> - dump the number of nnodes and add checks on get path
>>
>> v1 -> v2:
>> - fix compilation issues reported by robots
>> - fixed init of VMSD array
>> - direct key now dumped as a 32b
>> - removed useless cast from/to pointer
>> ---
>> include/migration/vmstate.h | 40
>> migration/trace-events | 5 +
>> migration/vmstate-types.c | 152 +
>> tests/test-vmstate.c| 421
>> 4 files changed, 618 insertions(+)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index 1fbfd099dd..b9ee563aa4 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -224,6 +224,7 @@ extern const VMStateInfo vmstate_info_unused_buffer;
>> extern const VMStateInfo vmstate_info_tmp;
>> extern const VMStateInfo vmstate_info_bitmap;
>> extern const VMStateInfo vmstate_info_qtailq;
>> +extern const VMStateInfo vmstate_info_gtree;
>>
>> #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
>> /*
>> @@ -754,6 +755,45 @@ extern const VMStateInfo vmstate_info_qtailq;
>> .start= offsetof(_type, _next), \
>> }
>>
>> +/*
>> + * For migrating a GTree whose key is a pointer to _key_type and the
>> + * value, a pointer to _val_type
>> + * The target tree must have been properly initialized
>> + * _vmsd: Start address of the 2 element array containing the data vmsd
>> + *and the key vmsd, in that order
>> + * _key_type: type of the key
>> + * _val_type: type of the value
>> + */
>> +#define VMSTATE_GTREE_V(_field, _state, _version, _vmsd,
>>\
>> +_key_type, _val_type)
>>\
>> +{
>>\
>> +.name = (stringify(_field)),
>>\
>> +.version_id = (_version),
>>\
>> +.vmsd = (_vmsd),
>>\
>> +.info = _info_gtree,
>>\
>> +.start= sizeof(_key_type),
>>\
>> +.size = sizeof(_val_type),
>>\
>> +.offset = offsetof(_state, _field),
>>\
>> +}
>> +
>> +/*
>> + * For migrating a GTree with direct key and the value a pointer
>> + * to _val_type
>> + * The target tree must have been properly initialized
>> + * _vmsd: data vmsd
>> + * _val_type: type of the value
>> + */
>> +#define VMSTATE_GTREE_DIRECT_KEY_V(_field, _state, _version, _vmsd,
>> _val_type) \
>> +{
>>\
>> +.name = (stringify(_field)),
>>\
>> +.version_id = (_version),
>>\
>> +.vmsd = (_vmsd),
>>\
>> +.info = _info_gtree,
>>\
>> +.start= 0,
Re: [PATCH v6] migration: Support gtree migration
* 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
I've applied:
@@ -1070,8 +1070,8 @@ static gboolean match_domain_node(gpointer key, gpointer
value, gpointer data)
TestGTreeDomain *d1, *d2;
struct match_node_data *d = (struct match_node_data *)data;
-id1 = (uint64_t)key;
-id2 = (uint64_t)d->key;
+id1 = (uint64_t)(uintptr_t)key;
+id2 = (uint64_t)(uintptr_t)d->key;
d1 = (TestGTreeDomain *)value;
d2 = (TestGTreeDomain *)d->value;
assert(id1 == id2);
to get the tests happy in 32bit.
>
> ---
>
> v4 -> v5:
> - swap key/value in vmsd array
> - fix g_free(key) in case of direct key
> - return an error if the number of decoded nodes does not
> match nnodes
>
> v3 -> v4:
> - properly initialize capsule.vmdesc in put_gtree()
> - use uintptr_t
> - add trace points
>
> v2 -> v3:
> - do not include glib.h anymore
> - introduce VMSTATE_GTREE_DIRECT_KEY_V
> - use pre_load to allocate the tree and remove data pointer
> - dump the number of nnodes and add checks on get path
>
> v1 -> v2:
> - fix compilation issues reported by robots
> - fixed init of VMSD array
> - direct key now dumped as a 32b
> - removed useless cast from/to pointer
> ---
> include/migration/vmstate.h | 40
> migration/trace-events | 5 +
> migration/vmstate-types.c | 152 +
> tests/test-vmstate.c| 421
> 4 files changed, 618 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 1fbfd099dd..b9ee563aa4 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -224,6 +224,7 @@ extern const VMStateInfo vmstate_info_unused_buffer;
> extern const VMStateInfo vmstate_info_tmp;
> extern const VMStateInfo vmstate_info_bitmap;
> extern const VMStateInfo vmstate_info_qtailq;
> +extern const VMStateInfo vmstate_info_gtree;
>
> #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
> /*
> @@ -754,6 +755,45 @@ extern const VMStateInfo vmstate_info_qtailq;
> .start= offsetof(_type, _next), \
> }
>
> +/*
> + * For migrating a GTree whose key is a pointer to _key_type and the
> + * value, a pointer to _val_type
> + * The target tree must have been properly initialized
> + * _vmsd: Start address of the 2 element array containing the data vmsd
> + *and the key vmsd, in that order
> + * _key_type: type of the key
> + * _val_type: type of the value
> + */
> +#define VMSTATE_GTREE_V(_field, _state, _version, _vmsd,
> \
> +_key_type, _val_type)
> \
> +{
> \
> +.name = (stringify(_field)),
> \
> +.version_id = (_version),
> \
> +.vmsd = (_vmsd),
> \
> +.info = _info_gtree,
> \
> +.start= sizeof(_key_type),
> \
> +.size = sizeof(_val_type),
> \
> +.offset = offsetof(_state, _field),
> \
> +}
> +
> +/*
> + * For migrating a GTree with direct key and the value a pointer
> + * to _val_type
> + * The target tree must have been properly initialized
> + * _vmsd: data vmsd
> + * _val_type: type of the value
> + */
> +#define VMSTATE_GTREE_DIRECT_KEY_V(_field, _state, _version, _vmsd,
> _val_type) \
> +{
> \
> +.name = (stringify(_field)),
> \
> +.version_id = (_version),
> \
> +.vmsd = (_vmsd),
> \
> +.info = _info_gtree,
> \
> +.start= 0,
> \
> +.size = sizeof(_val_type),
> \
> +.offset = offsetof(_state, _field),
> \
> +}
> +
>
Re: [PATCH v6] migration: Support gtree migration
* 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
Reviewed-by: Dr. David Alan Gilbert
>
> ---
>
> v4 -> v5:
> - swap key/value in vmsd array
> - fix g_free(key) in case of direct key
> - return an error if the number of decoded nodes does not
> match nnodes
>
> v3 -> v4:
> - properly initialize capsule.vmdesc in put_gtree()
> - use uintptr_t
> - add trace points
>
> v2 -> v3:
> - do not include glib.h anymore
> - introduce VMSTATE_GTREE_DIRECT_KEY_V
> - use pre_load to allocate the tree and remove data pointer
> - dump the number of nnodes and add checks on get path
>
> v1 -> v2:
> - fix compilation issues reported by robots
> - fixed init of VMSD array
> - direct key now dumped as a 32b
> - removed useless cast from/to pointer
> ---
> include/migration/vmstate.h | 40
> migration/trace-events | 5 +
> migration/vmstate-types.c | 152 +
> tests/test-vmstate.c| 421
> 4 files changed, 618 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 1fbfd099dd..b9ee563aa4 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -224,6 +224,7 @@ extern const VMStateInfo vmstate_info_unused_buffer;
> extern const VMStateInfo vmstate_info_tmp;
> extern const VMStateInfo vmstate_info_bitmap;
> extern const VMStateInfo vmstate_info_qtailq;
> +extern const VMStateInfo vmstate_info_gtree;
>
> #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
> /*
> @@ -754,6 +755,45 @@ extern const VMStateInfo vmstate_info_qtailq;
> .start= offsetof(_type, _next), \
> }
>
> +/*
> + * For migrating a GTree whose key is a pointer to _key_type and the
> + * value, a pointer to _val_type
> + * The target tree must have been properly initialized
> + * _vmsd: Start address of the 2 element array containing the data vmsd
> + *and the key vmsd, in that order
> + * _key_type: type of the key
> + * _val_type: type of the value
> + */
> +#define VMSTATE_GTREE_V(_field, _state, _version, _vmsd,
> \
> +_key_type, _val_type)
> \
> +{
> \
> +.name = (stringify(_field)),
> \
> +.version_id = (_version),
> \
> +.vmsd = (_vmsd),
> \
> +.info = _info_gtree,
> \
> +.start= sizeof(_key_type),
> \
> +.size = sizeof(_val_type),
> \
> +.offset = offsetof(_state, _field),
> \
> +}
> +
> +/*
> + * For migrating a GTree with direct key and the value a pointer
> + * to _val_type
> + * The target tree must have been properly initialized
> + * _vmsd: data vmsd
> + * _val_type: type of the value
> + */
> +#define VMSTATE_GTREE_DIRECT_KEY_V(_field, _state, _version, _vmsd,
> _val_type) \
> +{
> \
> +.name = (stringify(_field)),
> \
> +.version_id = (_version),
> \
> +.vmsd = (_vmsd),
> \
> +.info = _info_gtree,
> \
> +.start= 0,
> \
> +.size = sizeof(_val_type),
> \
> +.offset = offsetof(_state, _field),
> \
> +}
> +
> /* _f : field name
> _f_n : num of elements field_name
> _n : num of elements
> diff --git a/migration/trace-events b/migration/trace-events
> index 858d415d56..6dee7b5389 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -71,6 +71,11 @@ get_qtailq_end(const char *name, const char *reason, int
> val) "%s %s/%d"
> put_qtailq(const char *name, int version_id) "%s v%d"
> put_qtailq_end(const char *name,
