* Gerd Hoffmann ([email protected]) wrote: > On Di, 2016-07-12 at 16:02 +0200, Cornelia Huck wrote: > > On Tue, 12 Jul 2016 15:54:57 +0200 > > Gerd Hoffmann <[email protected]> wrote: > > > > > > @@ -1170,9 +1166,6 @@ static void virtio_gpu_device_realize(DeviceState > > > > *qdev, Error **errp) > > > > > > > > if (virtio_gpu_virgl_enabled(g->conf)) { > > > > vmstate_register(qdev, -1, &vmstate_virtio_gpu_unmigratable, > > > > g); > > > > - } else { > > > > - register_savevm(qdev, "virtio-gpu", -1, VIRTIO_GPU_VM_VERSION, > > > > - virtio_gpu_save, virtio_gpu_load, g); > > > > } > > > > } > > > > > > > > @@ -1220,6 +1213,9 @@ static void virtio_gpu_reset(VirtIODevice *vdev) > > > > #endif > > > > } > > > > > > > > +VMSTATE_VIRTIO_DEVICE(gpu, VIRTIO_GPU_VM_VERSION, virtio_gpu_load, > > > > + virtio_gpu_save); > > > > + > > > > static Property virtio_gpu_properties[] = { > > > > DEFINE_PROP_UINT32("max_outputs", VirtIOGPU, conf.max_outputs, 1), > > > > #ifdef CONFIG_VIRGL > > > > @@ -1245,6 +1241,7 @@ static void virtio_gpu_class_init(ObjectClass > > > > *klass, void *data) > > > > vdc->reset = virtio_gpu_reset; > > > > > > > > dc->props = virtio_gpu_properties; > > > > + dc->vmsd = &vmstate_virtio_gpu; > > > > } > > > > > > > > static const TypeInfo virtio_gpu_info = { > > > > > > This is confusing. I think for the virtio_gpu_virgl_enabled() case we > > > install *two* vmstates now ... > > > > I don't think that matters, as the unmigratable state already blocks, > > no? > > As long the core isn't confused if we register twice for the same device > it should work, yes ... > > > I think you should move up VMSTATE_VIRTIO_DEVICE, then simply replace > > > the register_savevm() call with a vmstate_register() call. > > > > It would make virtio-gpu look different from all other devices, though. > > Sure, because depending on device configuration you can migrate it or > not, which isn't the case for all other devices. > > I'd prefer to have the logic for that in one place as it makes more > clear how things are working: "if (!migrateable) register(block) else > register(normal);"
Hmm yes; I think the right fix here is to convert that into migrate_add_blocker / migrate_del_blocker since that's all you're currently doing. When you actually want to make it migratable add it to virtio_gpu_load/store (until they get borged into vmstate) I'll rework with migrate_add/del_blocker calls. Dave > > cheers, > Gerd -- Dr. David Alan Gilbert / [email protected] / Manchester, UK
