Re: [Qemu-devel] [PATCH v7 02/13] ssi: Implemented CS behaviour

[Peter Crosthwaite]

On Mon, Sep 24, 2012 at 7:29 PM, Peter Maydell <address@hidden> wrote:
> On 24 September 2012 10:18, Peter A. G. Crosthwaite
> <address@hidden> wrote:
>> @@ -296,10 +297,13 @@ static void ssd0323_save(QEMUFile *f, void *opaque)
>>      qemu_put_be32(f, s->remap);
>>      qemu_put_be32(f, s->mode);
>>      qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>> +
>> +    qemu_put_be32(f, ss->cs);
>>  }
>>
>>  static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
>>  {
>> +    SSISlave *ss = SSI_SLAVE(opaque);
>>      ssd0323_state *s = (ssd0323_state *)opaque;
>>      int i;
>>
>> @@ -321,6 +325,8 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int 
>> version_id)
>>      s->mode = qemu_get_be32(f);
>>      qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>>
>> +    ss->cs = qemu_get_be32(f);
>> +
>>      return 0;
>>  }
>
> This looks odd. The cs field isn't part of the ssd0323_state,
> so it shouldn't be ssd0303_save/load's job to save and restore
> it. Contrast the way the vmstate-based devices don't directly
> save/restore cs but defer to VMSTATE_SSI_SLAVE.
>

I took a "when in rome" philosophy here, and did it the way the device
model already does VMSD. The rest of the device state is encapsulated
using this mechanism. Im guessing this is an old way of doing VMSD
with the whole register_savevm() thing. Is it possible to mix and
match this old file-style VMSD with the newer struct approach? I cant
see an easy way without completely rewriting the VMSD implementation
of this device model.

Regards,
Peter

> -- PMM