qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 12/13] virtio-gpu: Wrap in vmstate


From: Dr. David Alan Gilbert
Subject: Re: [Qemu-devel] [PATCH v2 12/13] virtio-gpu: Wrap in vmstate
Date: Thu, 14 Jul 2016 17:04:07 +0100
User-agent: Mutt/1.6.1 (2016-04-27)

* Gerd Hoffmann (address@hidden) wrote:
> On Di, 2016-07-12 at 16:02 +0200, Cornelia Huck wrote:
> > On Tue, 12 Jul 2016 15:54:57 +0200
> > Gerd Hoffmann <address@hidden> 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 / address@hidden / Manchester, UK



reply via email to

[Prev in Thread] Current Thread [Next in Thread]