[Qemu-devel] vmstate: Useless post_save?

[Qemu-devel] vmstate: Useless post_save?

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

Hi all,

I wondered why we have the post_save callback in vmstate. Conceptually,
it made no sense to me. So I grep'ed for its users - and found exactly
one: tmp105. As suspected, only "strange" code was found:

static void tmp105_post_save(void *opaque)
{
    TMP105State *s = opaque;
    s->faults = tmp105_faultq[(s->config >> 3) & 3];            /* F */
}

First, s->config cannot be changed by saving the state. And, second,
s->faults is only written by this driver, never read.

Anyone any concerns dropping 'faults' from tmp105 and then dropping the
post_save handler from vmstate?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: vmstate: Useless post_save?

[Juan Quintela]

Jan Kiszka <jan.kiszka@web.de> wrote:
> Hi all,
>
> I wondered why we have the post_save callback in vmstate. Conceptually,
> it made no sense to me. So I grep'ed for its users - and found exactly
> one: tmp105. As suspected, only "strange" code was found:
>
> static void tmp105_post_save(void *opaque)
> {
>     TMP105State *s = opaque;
>     s->faults = tmp105_faultq[(s->config >> 3) & 3];            /* F */
> }
>
> First, s->config cannot be changed by saving the state. And, second,
> s->faults is only written by this driver, never read.
>
> Anyone any concerns dropping 'faults' from tmp105 and then dropping the
> post_save handler from vmstate?

About 'faults' dropping, I have to opinion at all.  It is done this way
because old code did it this way.

About post_save() it should be there only to "undo" things done in
"pre_save" to be able to continue in the source after a failed
migration.

As you have found, not much users around.  If you remove tmp105 user, I
vote to remove it.  We can "revert" the removal if need appears again.

Later, Juan.

[Qemu-devel] Re: vmstate: Useless post_save?

[andrzej zaborowski]

Hi,

On 8 May 2010 00:39, Jan Kiszka <jan.kiszka@web.de> wrote:
> I wondered why we have the post_save callback in vmstate. Conceptually,
> it made no sense to me. So I grep'ed for its users - and found exactly
> one: tmp105. As suspected, only "strange" code was found:
>
> static void tmp105_post_save(void *opaque)
> {
>    TMP105State *s = opaque;
>    s->faults = tmp105_faultq[(s->config >> 3) & 3];            /* F */
> }
>
> First, s->config cannot be changed by saving the state. And, second,
> s->faults is only written by this driver, never read.

I'm not sure why the post_save is there, it looks like it should be in
post_load rather.

The faults counter is an actual register somewhere in the hardware,
just not exposed through I2C bus.  However the counter is never
decremented because the temperature measurements are fake so there's
no measurement error and no point in delaying the interrupt until the
counter reaches zero.

Cheers