Re: [Qemu-devel] [PATCH] i2c: factor out VMSD to parent class

[Juan Quintela <quintela@redhat.com>]

"Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com> wrote:
> Hi All. PMM raised a query on a recent series of mine (the SSI series) about
> handling VMSD for devices which define state at multiple levels of the QOM
> heirachy. Rather than complicate the discussion over in my series im trying to
> start the discussion with an existing subsystem - i2c. This patch is a first
> attempt at trying to get the VMSD for generic I2C state factored out of the
> individual devices and handled transparently by the super class (I2C_SLAVE).
>
> I have applied the change to only the one I2C device (max7310). If we were going
> to run with this, the change pattern would be applied to all I2C devices.
>
> This patch is not a merge proposal it is RFC only.
>
> Please review and let us know if this is flawed or not. What needs to be done to
> get this multi-level VMSD going?
>
> I will use whatever review I get to fix my SSI series as well as fix I2C.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

This series move data from one part to another (obvious), now the
questions:
- how do you know that one part of the data relates to the same device
  on the other side?  I don't know about i2c, I am hoping for an answer
  O;-)  This will make that the state of the device will be sent in two
  chunks, so we should make sure that the two chunks ends in the same
  device.

- This makes the change completely uncompatible.  What boards use
  i2c/ssi?  My understanding is that it is only used outside of
  x86(_64), if so, we can live without backward compatibility.  We
  should increase the version numbers.  Something like that on addition.

static const VMStateDescription vmstate_max7310 = {
    .name = "max7310",
-   .version_id = 0,
-   .minimum_version_id = 0,
-   .minimum_version_id_old = 0,
+   .version_id = 1,
+   .minimum_version_id = 1,
+   .minimum_version_id_old = 1,


   And that should make it.

- If you ask me, I would very much preffer something like PCI devices,
  where the 1st field of any specific device is the i2c part.  This
  would achieve two things:
   * all i2c devices would have the common fields at the beggining
   * we sent the data for one device in one go, so we will never had
     trouble making sure that both devices arrive at the same time, in
     the right order, etc.

- I guess there is same reasy why you want to split the device state,
  it could be on the other series where I haven't read it though.

Later, Juan.