Am 07.04.2014 12:00, schrieb Juan Quintela:
> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
>> * Juan Quintela (quint...@redhat.com) wrote:
>>> Hi
>>>
>>> Look at the diffstat. Almost all the additions are at
>>> test-vmstate.c. That is the reason why it is called a simplification.
>>>
>>> What this series does:
>>> - peter removal of version_minimum_id_old field when not needed (Peter)
>>> - cleanup: based on the previous one, I removed all the unneeded
>>> the uses on the tree. This should make your compiles
>>> a couple of nanoseconds faster.
>>> - once there, fixed the indentation of the .fields line, to a canonical
>>> .fields = (VMStateField[])
>>> - mst simplifications for vmstate engine
>>>
>>> And now, the big cleanup.
>>> - Patches only do one thing, to make easy the review.
>>>
>>> - Added test for all VMSTATE_FOO() definitions
>>> (well, I am lying, VMSTATE_STRUCT* are still missing, will come soon)
>>> - We had two ways to make a field optional
>>> VMSTATE_INT64_V(field, state, version)
>>> and
>>> VMSTATE_INT64_TEST(field, state, test)
>>>
>>> We can do the version one with one test like:
>>>
>>> static inline bool vmstate_5_plus(void *opaque, int version_id)
>>> {
>>> return version_id >= 5;
>>> }
>>>
>>> and then change:
>>> VMSTATE_INT64_V(field, state, 5);
>>>
>>> into
>>> VMSTATE_INT64_TEST(field, state, vmstate_5_plus);
>>
>> I'm not sure if I like this; while I'm OK with the idea of changing the
>> implementation of VMSTATE_INT64_V to use that function trick internally,
>> it seems like we're discouraging providing easy to parse/record versionining
>> info out of the tree.
>
> That information is not exported Today. And if we want to export them,
> export v = 5 or test == vmstat_5_plus is exactly the same dificulty.
That's not quite true. My code can easily compare an integer field in
VMStateDescription, but for any .test function callback I am in the
blind: Without executing the test on a specific set of data, I cannot
reason whether incoming data will be compatible with the VMSD.
> The information is still there, what has changed is the removal of one
> mechanism.
>
> #define VMSTATE_UINT64_V(field, state, X) \
> VMSTATE_UINT64_TEST(field, state, vmstate_##X##_plus)
If these were a fixed set of global test functions, it could work, by
special-casing function pointer comparisons.
> The same that we could do for maintaining the macro, we can do to export
> the information to whatever format we want. I just wanted to remove one
> of the mechanism (the less powerful one).
On another matter, I still haven't heard from you on my QOM VMState
simplification series from last July:
https://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg01072.html
If we fix the NVEC issue I discovered in February and add qtests for the
remaining optional devices to assure there are no further dc->vmsd
overriding devices, would that approach be acceptable to you? Or do you
have a better suggestion?
Thanks,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
