From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38988) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brrQR-0007Bn-RJ for qemu-devel@nongnu.org; Wed, 05 Oct 2016 15:00:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brrQM-0006qC-2O for qemu-devel@nongnu.org; Wed, 05 Oct 2016 15:00:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51010) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brrQL-0006ps-Pd for qemu-devel@nongnu.org; Wed, 05 Oct 2016 15:00:14 -0400 Date: Wed, 5 Oct 2016 20:00:07 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20161005190006.GF11921@work-vm> References: <20160930142003.53232-1-pasic@linux.vnet.ibm.com> <20160930142003.53232-2-pasic@linux.vnet.ibm.com> <2487d687-7fbe-43f4-b9c5-e03b26b2250d@redhat.com> <5b08c569-813b-d805-833a-210e9c8dc436@linux.vnet.ibm.com> <49f37536-0fa7-a38b-a3fa-7b3050001658@redhat.com> <57b04738-7e95-a8c5-d974-0386cbff1419@redhat.com> <20161005142902.GB11921@work-vm> <68ee3bbe-ef83-ce4b-4546-ea27fc2e2ea0@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <68ee3bbe-ef83-ce4b-4546-ea27fc2e2ea0@redhat.com> Subject: Re: [Qemu-devel] [PATCH 01/12] virtio: add VIRTIO_DEF_DEVICE_VMSD macro List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Halil Pasic , qemu-devel@nongnu.org, "Michael S . Tsirkin" , Gerd Hoffmann , Stefan Hajnoczi , Amit Shah , "Aneesh Kumar K . V" * Paolo Bonzini (pbonzini@redhat.com) wrote: > > > On 05/10/2016 16:29, Dr. David Alan Gilbert wrote: > > The virtio-input conversion especially is nice and simple. > > The only weird case is virtio-gpu, and well virtio-gpu is the one that's > > odd here (compared to the rest of virtio). > > Though virtio-gpu would be the one that could become nicer without the > macros. :) Yes, it would. > What I don't like about the macros is that they don't allow you to > extend the VMStateDescription. Hmm so would this work: add an 'extra_field' that we normally leave empty: #define VIRTIO_DEF_DEVICE_VMSD(devname, v, extra_field, ...) \ static const VMStateDescription vmstate_virtio_ ## devname = { \ .name = "virtio-" #devname , \ .minimum_version_id = v, \ .version_id = v, \ .fields = (VMStateField[]) { \ VMSTATE_VIRTIO_FIELD, \ extra_field \ VMSTATE_END_OF_LIST() \ }, \ __VA_ARGS__ \ }; so the normal case would gain messy commas: VIRTIO_DEF_DEVICE_VMSD(console,3, /* empty */) the case with pre/post would be: VIRTIO_DEF_DEVICE_VMSD(vhost_vsock, VHOST_VSOCK_SAVEVM_VERSION, /* empty */, .pre_save = vhost_vsock_pre_save, .post_load = vhost_vsock_post_load) ..... Define a macro for this field so it's passed as one entry: #define VMSTATE_VIRTIO_GPU_FIELD \ { \ .name = "virtio-gpu", \ .info = &(const VMStateInfo) { \ .name = "virtio-gpu", \ .get = virtio_gpu_load, \ .put = virtio_gpu_save, \ }, \ .flags = VMS_SINGLE, \ } /* device */, VIRTIO_DEF_DEVICE_VMSD(virtio-gpu, VIRTIO_GPU_VM_VERSION, VMSTATE_VIRTIO_GPU_FIELD) > However, if you agree with Halil your > opinion counts more. Well I think Halil's stuff moves it in the right direction; with everything in VIRTIO_DEF_DEVICE_VMSD it means we can move things out of virtio_load/save and into corresponding members in VIRTIO_DEF_DEVICE_VMSD's .fields array (before or after VMSTATE_VIRTIO_FIELD) without having to change any of the devices. Eventually virtio_load/save disappear. Dave > > Paolo > > > I'd say we take these macros (apart from anything minor) - I'd anticipate > > they'd evolve slowly as we move more and more chunks to VMState. -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK