From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48314) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7nkS-0005Co-27 for qemu-devel@nongnu.org; Wed, 14 Mar 2012 08:56:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7nkH-0006m3-HN for qemu-devel@nongnu.org; Wed, 14 Mar 2012 08:56:11 -0400 Received: from cantor2.suse.de ([195.135.220.15]:58566 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7nkH-0006lj-5K for qemu-devel@nongnu.org; Wed, 14 Mar 2012 08:56:01 -0400 Message-ID: <4F60955E.6060204@suse.de> Date: Wed, 14 Mar 2012 13:55:58 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1330936245-2570-1-git-send-email-i.mitsyanko@samsung.com> <1330936245-2570-6-git-send-email-i.mitsyanko@samsung.com> In-Reply-To: <1330936245-2570-6-git-send-email-i.mitsyanko@samsung.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2 5/5] vmstate: introduce get_bufsize entry in VMStateField List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mitsyanko Cc: peter.maydell@linaro.org, e.voevodin@samsung.com, quintela@redhat.com, qemu-devel@nongnu.org, kyungmin.park@samsung.com, d.solodkiy@samsung.com, m.kozlov@samsung.com 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 allocat= ed buffer is > already presented in specific device's state structure, but in such a f= orm that > can not be used with existing VMStateField interface. Currently, we eit= her can get size from > another variable in device's state as it is with VMSTATE_VBUFFER_* macr= os, or we can > also multiply value kept in a variable by a constant with VMSTATE_BUFFE= R_MULTIPLY > macro. If we need to perform any other action, we're forced to add addi= tional > variable with size information to device state structure with the only = intention > to use it in VMStateDescription. This approach is not very pretty. Addi= ng 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 perfo= rm a set of > operations to get size value. > With get_bufsize callback we can calculate size of dynamic array in whi= chever > 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 becau= se of > get_bufsize addition. Macros VMSTATE_VBUFFER* are modified to use new c= allback > instead of .size_offset. Macro VMSTATE_BUFFER_MULTIPLY and VMFlag VMS_M= ULTIPLY > are removed completely as they are now redundant. >=20 > Signed-off-by: Igor Mitsyanko 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 =3D 0x020, /* static sized buffer */ > VMS_ARRAY_OF_POINTER =3D 0x040, > VMS_VARRAY_UINT16 =3D 0x080, /* Array with size in uint16_t fi= eld */ > - VMS_VBUFFER =3D 0x100, /* Buffer with size in int32_t fi= eld */ > - VMS_MULTIPLY =3D 0x200, /* multiply "size" field by field= _size */ > + VMS_VBUFFER =3D 0x100, /* Buffer with dynamically calcul= ated size */ > VMS_VARRAY_UINT8 =3D 0x400, /* Array with size in uint8_t fie= ld*/ > VMS_VARRAY_UINT32 =3D 0x800, /* Array with size in uint32_t fi= eld*/ > }; > @@ -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; > =20 > typedef struct VMStateSubsection { > @@ -354,34 +353,11 @@ extern const VMStateInfo vmstate_info_unused_buff= er; > .offset =3D vmstate_offset_buffer(_state, _field) + _start, = \ > } > =20 > -#define VMSTATE_BUFFER_MULTIPLY(_field, _state, _version, _test, _star= t, _field_size, _multiply) { \ > +#define VMSTATE_VBUFFER(_field, _state, _version, _test, _start, _get_= bufsize) { \ > .name =3D (stringify(_field)), = \ > .version_id =3D (_version), = \ > .field_exists =3D (_test), = \ > - .size_offset =3D vmstate_offset_value(_state, _field_size, uint32= _t),\ > - .size =3D (_multiply), = \ > - .info =3D &vmstate_info_buffer, = \ > - .flags =3D VMS_VBUFFER|VMS_MULTIPLY, = \ > - .offset =3D offsetof(_state, _field), = \ > - .start =3D (_start), = \ > -} > - > -#define VMSTATE_VBUFFER(_field, _state, _version, _test, _start, _fiel= d_size) { \ > - .name =3D (stringify(_field)), = \ > - .version_id =3D (_version), = \ > - .field_exists =3D (_test), = \ > - .size_offset =3D vmstate_offset_value(_state, _field_size, int32_= t),\ > - .info =3D &vmstate_info_buffer, = \ > - .flags =3D VMS_VBUFFER|VMS_POINTER, = \ > - .offset =3D offsetof(_state, _field), = \ > - .start =3D (_start), = \ > -} > - > -#define VMSTATE_VBUFFER_UINT32(_field, _state, _version, _test, _start= , _field_size) { \ > - .name =3D (stringify(_field)), = \ > - .version_id =3D (_version), = \ > - .field_exists =3D (_test), = \ > - .size_offset =3D vmstate_offset_value(_state, _field_size, uint32= _t),\ > + .get_bufsize =3D (_get_bufsize), = \ > .info =3D &vmstate_info_buffer, = \ > .flags =3D VMS_VBUFFER|VMS_POINTER, = \ > .offset =3D offsetof(_state, _field), = \ > @@ -570,14 +546,11 @@ extern const VMStateInfo vmstate_info_unused_buff= er; > #define VMSTATE_BUFFER_START_MIDDLE(_f, _s, _start) \ > VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, _start, sizeof(typeof_field= (_s, _f))) > =20 > -#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) > =20 > -#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) > =20 > #define VMSTATE_BUFFER_TEST(_f, _s, _test) = \ > VMSTATE_STATIC_BUFFER(_f, _s, 0, _test, 0, sizeof(typeof_field(_s,= _f))) --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg