Re: [Qemu-devel] [PATCH v2 000/124] VMState Simplification (Massive)

[Juan Quintela]

Peter Maydell <address@hidden> wrote:
> On 21 April 2014 18:25, Juan Quintela <address@hidden> wrote:
>> I can split the series at any point (they make sense even without the
>> rest).
>>
>> What about:
>>
>> 1,7,8: bug fixes/simplification
>>
>> 2-6: massive Unneeded version_minimum_id_old removal, but trivial per se
>>
>> 9-47: New testing framework and tests for integers and arrays of
>>       integers
>>
>> This three should be no controversial.
>>
>> 48-86: Removal of _V variants and ends with removal of version_id for
>>        fields
>>
>>        This shouldn't be controversial at all, but I agree that I can be
>>        biased here.
>>
>> 87-124: buffers, pointers and structs, in weird combinations.
>>
>>         This shouldn't be controversial either, but depends on the
>>         removal of version_id.  Why?  Because otherwise we should need
>>         (roughly) to double the number of tests, as we have the testing
>>         mechanism that is a super set of the version stuff.
>
> Yes, this sounds like a reasonable split.
>
>>> I'm not sure about merging the field versioning with the field test
>>> function stuff, but I'm not about to try to fish the relevant patches
>>> out of this enormous mess so I'll wait until they appear in a series
>>> of their own before I comment on them.
>>
>> Well, at least I now told you were the patches are O:-)
>>
>> If there is a reason why you don't want to remove it (or anybody else),
>> please let me know.
>
> Mostly just that I think that for vmstate definitions "this new field
> was added in version X" is natural and normal, whereas other
> test functions are odd and generally the exception. So a simple
> way to indicate minimum version for fields seems useful to retain.
> I don't mind if you want to unify the underlying implementation,
> but I would prefer to retain the _V macros for vmstate definitions
> to use (and it has the additional advantage of avoiding the need for
> touching lots of devices...)

If it makes you feel better,  We can get bock

#define VMSTATE_FOO_V(field, device, v) \
     VMSTATE_FOO_TEST(field, device, vmstate_ ## v ## _plus )

But I think it bring us back basically nothing.  Not so many devices use
versions, and from now on, we are really tring to use optional
subsections for the new stuff (or like arm, just not even trying
backward compatiblity).

If it has almost no users, why do we want yet another set of macros?
One of the complains that I get about vmstate is that there are too many
VMSTATE_FOO_X() macros, this change just remove 1/3 of the cases.

And anyways, a _V() should also require extra review as Paolo deszcribes
for _TEST.

Later, Juan.