* Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > > > On 05/08/2017 07:59 PM, Dr. David Alan Gilbert wrote: > >>>> const VMStateDescription vmstate_virtio_ccw_dev = { > >>>> .name = "s390_virtio_ccw_dev", > >>>> .version_id = 1, > >>>> @@ -67,6 +93,14 @@ const VMStateDescription vmstate_virtio_ccw_dev = { > >>>> VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), > >>>> VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), > >>>> VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), > >>>> + { > >>>> + /* > >>>> + * Ugly hack because VirtIODevice does not migrate itself. > >>>> + * This also makes legacy via vmstate_save_state possible. > >>>> + */ > >>>> + .name = "virtio/config_vector", > >>>> + .info = &vmstate_info_config_vector, > >>>> + }, > >>> I'm a bit confused - isn't just this bit the bit going > >>> through the vdev->load_config hook? > >>> > >> Exactly! If you examine the part underscored with ============== in > >> virtio_ccw_(load|save)_config (that is what is behind vdev->load_config) > >> you should see that we use vmstate_virtio_ccw_dev (that is this > >> VMStateDescription to implement these function. > >> > >> Actually virtio_ccw_(load|save)_config won't do anything after patch 9 > >> and for new machines since the VirtIOCcwDevice is going to migrate > >> itself via DeviceClass.vmsd like other "normal" devices, and we fall > >> back to the VirtIO specific callbacks only in "legacy mode". > >> > >> I hope this helps! > > Hmm, still confused! > > Why are you changing a Virtio device not to use the same migration > > oddities as all the other virtio devices? > > > > I was assuming we'd have to change the virtio core code to > > solve that for all virtio devices. > > > > You can ask difficult questions ;). First I'm not aware of any > ongoing effort to solve this for all virtio devices by changing > the core, and probably breaking compatibility for all devices > (in a sense I break migration compatibility for all virtio-ccw > devices). To be honest, I have considered that move unlikely, > but the possibility is one of my reasons for seeking an upstream > discussion and having You and Michael on cc.
Well I have been trying to improve it, but that code is particularly nasty; and I don't want to break compatibility. I had some ideas, for example I was thinking of changing the vdev->load_config to a VMState* and then calling a vmstate_load_state(f, vdc->config,... from virtio_load - but there's some challenging casting and hackery there. > > Why not use virtio oddities? Because they are oddities. I have > figured, it's a good idea to separate the migration of the > proxy form the rest: we have two QEMU Device objects and it > should be good practice, that these are migrating themselves via > DeviceClass.vmsd. That's what I get with this patch set, > for new machine versions (since we can not fix the past), and > with the notable difference of config_vector, because it is > defined as a common infrastructure (struct VirtIODevice) but > ain't migrated as a common virtio infrastructure. Have you got a bit of a description of your classes/structure - it's a little hard to get my head around. > Would you suggest to rather keep the oddities? Should I expect > to see a generic solution to the problem sometime soon? Not soon I fear; virtio_load is just too hairy. Dave > Halil > > > Dave > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK