From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMzK5-0002u5-Ta for qemu-devel@nongnu.org; Tue, 12 Jul 2016 11:10:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bMzK2-0001MN-7A for qemu-devel@nongnu.org; Tue, 12 Jul 2016 11:10:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMzK2-0001MI-0w for qemu-devel@nongnu.org; Tue, 12 Jul 2016 11:10:06 -0400 Message-ID: <1468336197.30552.16.camel@redhat.com> From: Gerd Hoffmann Date: Tue, 12 Jul 2016 17:09:57 +0200 In-Reply-To: <20160712160236.57c4a2b4.cornelia.huck@de.ibm.com> References: <1468325965-22818-1-git-send-email-dgilbert@redhat.com> <1468325965-22818-13-git-send-email-dgilbert@redhat.com> <1468331697.30552.7.camel@redhat.com> <20160712160236.57c4a2b4.cornelia.huck@de.ibm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2 12/13] virtio-gpu: Wrap in vmstate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: "Dr. David Alan Gilbert (git)" , amit.shah@redhat.com, cornelia.huck@del.ibm.com, quintela@redhat.com, qemu-devel@nongnu.org, mst@redhat.com On Di, 2016-07-12 at 16:02 +0200, Cornelia Huck wrote: > On Tue, 12 Jul 2016 15:54:57 +0200 > Gerd Hoffmann wrote: >=20 > > > @@ -1170,9 +1166,6 @@ static void virtio_gpu_device_realize(DeviceSta= te *qdev, Error **errp) > > > =20 > > > 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_VERSIO= N, > > > - virtio_gpu_save, virtio_gpu_load, g); > > > } > > > } > > > =20 > > > @@ -1220,6 +1213,9 @@ static void virtio_gpu_reset(VirtIODevice *vdev= ) > > > #endif > > > } > > > =20 > > > +VMSTATE_VIRTIO_DEVICE(gpu, VIRTIO_GPU_VM_VERSION, virtio_gpu_load, > > > + virtio_gpu_save); > > > + > > > static Property virtio_gpu_properties[] =3D { > > > 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 =3D virtio_gpu_reset; > > > =20 > > > dc->props =3D virtio_gpu_properties; > > > + dc->vmsd =3D &vmstate_virtio_gpu; > > > } > > > =20 > > > static const TypeInfo virtio_gpu_info =3D { > >=20 > > This is confusing. I think for the virtio_gpu_virgl_enabled() case we > > install *two* vmstates now ... >=20 > 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. >=20 > 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);" cheers, Gerd