Am 05.03.2012 09:30, schrieb Igor Mitsyanko:
> New get_bufsize field in VMStateField is supposed to help us easily add
> save/restore
> support of dynamically allocated buffers in device's states.
> There are some cases when information about size of dynamically allocated
> buffer is
> already presented in specific device's state structure, but in such a form
> that
> can not be used with existing VMStateField interface. Currently, we either
> can get size from
> another variable in device's state as it is with VMSTATE_VBUFFER_* macros, or
> we can
> also multiply value kept in a variable by a constant with
> VMSTATE_BUFFER_MULTIPLY
> macro. If we need to perform any other action, we're forced to add additional
> variable with size information to device state structure with the only
> intention
> to use it in VMStateDescription. This approach is not very pretty. Adding
> extra
> flags to VMStateFlags enum for every other possible operation with size field
> seems redundant, and still it would't cover cases when we need to perform a
> set of
> operations to get size value.
> With get_bufsize callback we can calculate size of dynamic array in whichever
> way we need. We don't need .size_offset field anymore, so we can remove it
> from
> VMState Field structure to compensate for extra memory consuption because of
> get_bufsize addition. Macros VMSTATE_VBUFFER* are modified to use new callback
> instead of .size_offset. Macro VMSTATE_BUFFER_MULTIPLY and VMFlag VMS_MULTIPLY
> are removed completely as they are now redundant.
>
> Signed-off-by: Igor Mitsyanko <i.mitsya...@samsung.com>
Apart from this commit message being an overwhelmingly huge block of
text ;) (maybe insert an empty line or two?) this had touched on the
overall discussion of whether to pursue a declarative or imperative
approach for VMState.
So, what about adding this as a new, optional mechanism and leaving
VBUFFER / BUFFER_MULTIPLY users untouched?
Andreas
[...]
> diff --git a/vmstate.h b/vmstate.h
> index 9d3c49c..a210e08 100644
> --- a/vmstate.h
> +++ b/vmstate.h
> @@ -73,8 +73,7 @@ enum VMStateFlags {
> VMS_BUFFER = 0x020, /* static sized buffer */
> VMS_ARRAY_OF_POINTER = 0x040,
> VMS_VARRAY_UINT16 = 0x080, /* Array with size in uint16_t field */
> - VMS_VBUFFER = 0x100, /* Buffer with size in int32_t field */
> - VMS_MULTIPLY = 0x200, /* multiply "size" field by field_size */
> + VMS_VBUFFER = 0x100, /* Buffer with dynamically calculated
> size */
> VMS_VARRAY_UINT8 = 0x400, /* Array with size in uint8_t field*/
> VMS_VARRAY_UINT32 = 0x800, /* Array with size in uint32_t field*/
> };
> @@ -86,12 +85,12 @@ typedef struct {
> size_t start;
> int num;
> size_t num_offset;
> - size_t size_offset;
> const VMStateInfo *info;
> enum VMStateFlags flags;
> const VMStateDescription *vmsd;
> int version_id;
> bool (*field_exists)(void *opaque, int version_id);
> + size_t (*get_bufsize)(void *opaque, int version_id);
> } VMStateField;
>
> typedef struct VMStateSubsection {
> @@ -354,34 +353,11 @@ extern const VMStateInfo vmstate_info_unused_buffer;
> .offset = vmstate_offset_buffer(_state, _field) + _start, \
> }
>
> -#define VMSTATE_BUFFER_MULTIPLY(_field, _state, _version, _test, _start,
> _field_size, _multiply) { \
> +#define VMSTATE_VBUFFER(_field, _state, _version, _test, _start,
> _get_bufsize) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> - .size = (_multiply), \
> - .info = &vmstate_info_buffer, \
> - .flags = VMS_VBUFFER|VMS_MULTIPLY, \
> - .offset = offsetof(_state, _field), \
> - .start = (_start), \
> -}
> -
> -#define VMSTATE_VBUFFER(_field, _state, _version, _test, _start,
> _field_size) { \
> - .name = (stringify(_field)), \
> - .version_id = (_version), \
> - .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
> - .info = &vmstate_info_buffer, \
> - .flags = VMS_VBUFFER|VMS_POINTER, \
> - .offset = offsetof(_state, _field), \
> - .start = (_start), \
> -}
> -
> -#define VMSTATE_VBUFFER_UINT32(_field, _state, _version, _test, _start,
> _field_size) { \
> - .name = (stringify(_field)), \
> - .version_id = (_version), \
> - .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .get_bufsize = (_get_bufsize), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -570,14 +546,11 @@ extern const VMStateInfo vmstate_info_unused_buffer;
> #define VMSTATE_BUFFER_START_MIDDLE(_f, _s, _start) \
> VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, _start, sizeof(typeof_field(_s,
> _f)))
>
> -#define VMSTATE_PARTIAL_VBUFFER(_f, _s, _size) \
> - VMSTATE_VBUFFER(_f, _s, 0, NULL, 0, _size)
> -
> -#define VMSTATE_PARTIAL_VBUFFER_UINT32(_f, _s, _size)
> \
> - VMSTATE_VBUFFER_UINT32(_f, _s, 0, NULL, 0, _size)
> +#define VMSTATE_PARTIAL_VBUFFER(_f, _s, _get_bufsize) \
> + VMSTATE_VBUFFER(_f, _s, 0, NULL, 0, _get_bufsize)
>
> -#define VMSTATE_SUB_VBUFFER(_f, _s, _start, _size) \
> - VMSTATE_VBUFFER(_f, _s, 0, NULL, _start, _size)
> +#define VMSTATE_SUB_VBUFFER(_f, _s, _start, _get_bufsize) \
> + VMSTATE_VBUFFER(_f, _s, 0, NULL, _start, _get_bufsize)
>
> #define VMSTATE_BUFFER_TEST(_f, _s, _test) \
> VMSTATE_STATIC_BUFFER(_f, _s, 0, _test, 0, sizeof(typeof_field(_s, _f)))
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
