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'. Thanks for your patience! Regards, Halil