Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration

[Dr. David Alan Gilbert]

* Halil Pasic (address@hidden) wrote:
> 
> 
> On 05/19/2017 07:28 PM, Dr. David Alan Gilbert wrote:
> >> To sum it up although I'm currently leaning towards abandoning my idea
> >> of two sections for two devices, I'm not comfortable with making the
> >> call myself. I'm hoping for some maintainer guidance (s390x, virtio
> >> and migration). 
> > <A couple of hours of reading, the CCW and PCI code, chatting with
> > dhildenb etc>
> > 
> > OK, so I think:
> >   a) First split the series into two separate series; one that
> > VMStatifies the existing stuff without breaking compatibility;
> > and one that adds the new stuff.  Lets get the first of those
> > going in while we think about the second.
> > 
> >     a.1) I'd do this with patches that convert one chunk into
> >      vmstate and remove the corresponding C code at the same time.
> > 
> >   b) While the way PCI devices are done is weird, I think it'll
> > be a lot simpler if you can stick to a structure that's similar
> > to them while diverging.  It's hard enough for those of us
> > who don't do Virtio every day to get our minds around virtio-pci
> > without it being different for virtio-ccw/css.
> > 
> >   c) To do (b) I suggest:
> >       c.1) you *don't* add a vmsd to your virtio_ccw_device
> > 
> >       c.2) but *do* add a VMSTATE_CCW_DEVICE to the start of any
> >          non-virtio devices you migrate (like each of the PCI devices
> >           have)
> > 
> >       c.3) You can add extra state for CSS to the ->save_extra_state
> >            handle on virtio devices or to the config
> > 
> >   d) vmstatifying the config is OK as well.
> > 
> > I should say I'm no virtio expert, so if any of that's truly
> > mad say so.
> > 
> > Dave
> > 
> 
> Agreed (a,b,c,d). Reorganizing my patch set according to a) is
> going to require some effort, but it should not be too bad. 
> About c.2): I don't think we have any migratable non virtio devices
> yet, but I'll keep it in mind.
> 
> I do not understand what you mean by c.3) and extra_sate. Maybe
> it will settle with time. My idea of extending VirtioCcwDevice
> is just adding subsections to it's VMStateDescription. The
> call vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL)
> in save_config should take care of compatibility. Maybe some
> staring at virtio-pci is going to help, but right now I can't tell
> what's the extra_state for, and how/why is it 'extra'.

Yes adding extra subsections into the 'config' is probably fine;
but there's also another hook that Jason added, see a6df8adf3,
it's an existing subsection at the end of the virtio state
that can be linked for transport specific data.

Dave

> Thanks for your patience!
> 
> Regards,
> Halil
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK