From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MeQU0-0004ev-ID for qemu-devel@nongnu.org; Fri, 21 Aug 2009 05:32:28 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MeQTt-0004Zm-Jl for qemu-devel@nongnu.org; Fri, 21 Aug 2009 05:32:25 -0400 Received: from [199.232.76.173] (port=53360 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MeQTt-0004Za-7G for qemu-devel@nongnu.org; Fri, 21 Aug 2009 05:32:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31789) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MeQTs-00040M-Ms for qemu-devel@nongnu.org; Fri, 21 Aug 2009 05:32:21 -0400 Received: from int-mx06.intmail.prod.int.phx2.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.19]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n7L9WJRh004862 for ; Fri, 21 Aug 2009 05:32:19 -0400 From: Juan Quintela In-Reply-To: <4A8E657D.4040604@redhat.com> (Gerd Hoffmann's message of "Fri, 21 Aug 2009 11:14:37 +0200") References: <20ca5615c8cdc456296698133e3b0dbd5a1f4de7.1250788880.git.quintela@redhat.com> <4A8E6049.6020609@redhat.com> <4A8E657D.4040604@redhat.com> Date: Fri, 21 Aug 2009 11:30:05 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: [Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org Gerd Hoffmann wrote: > On 08/21/09 11:01, Juan Quintela wrote: >> I agree that the INT32_EQUAL is a bad idea. But that is how >> the current code is done. > > Consider changing the code then ;) > >>> Beside that we'll have to think about how to handle versioning of >>> structs. Do we want vmstate_pci_device (and all others) have its own >>> version? Probably makes sense. Handling this should go into the >>> generic code then and not be hacked into each structure using >>> VMSTATE_INT32_LE(). >> >> Proper solution here is to use subsections. >> Each time that you call VMSTATE_PCI_DEVICE() it sends a subsection >> there. Then we get the versioning by free. This is how I am thinking >> it is the right way to do it. > > Yep, something like this. > >> virtio stuff: the more than I think about it, the easier way is to just >> get rid of the whole mess and do something that is sensible. > > You are talking about VirtIOPCIProxy I guess? > Yea, that is messy ... I am talking about virtio_load() actually. How do you translate this to a table? - If at the beggining (and that load_config() reads the config from the savevm - and another if in the middle of a for int virtio_load(VirtIODevice *vdev, QEMUFile *f) { int num, i, ret; if (vdev->binding->load_config) { ret = vdev->binding->load_config(vdev->binding_opaque, f); if (ret) return ret; } qemu_get_8s(f, &vdev->status); qemu_get_8s(f, &vdev->isr); qemu_get_be16s(f, &vdev->queue_sel); qemu_get_be32s(f, &vdev->features); vdev->config_len = qemu_get_be32(f); qemu_get_buffer(f, vdev->config, vdev->config_len); num = qemu_get_be32(f); for (i = 0; i < num; i++) { vdev->vq[i].vring.num = qemu_get_be32(f); vdev->vq[i].pa = qemu_get_be64(f); qemu_get_be16s(f, &vdev->vq[i].last_avail_idx); if (vdev->vq[i].pa) { virtqueue_init(&vdev->vq[i]); } if (vdev->binding->load_queue) { ret = vdev->binding->load_queue(vdev->binding_opaque, i, f); if (ret) return ret; } } virtio_notify_vector(vdev, VIRTIO_NO_VECTOR); return 0; } Can we have something saner like: int virtio_load(VirtIODevice *vdev, QEMUFile *f) { int num, i, ret; qemu_get_be32s(f, &vdev->config_load_size); qemu_get_buffer(f, &vdev->config_load_value, &vdev->config_load_size); qemu_get_8s(f, &vdev->status); qemu_get_8s(f, &vdev->isr); qemu_get_be16s(f, &vdev->queue_sel); qemu_get_be32s(f, &vdev->features); vdev->config_len = qemu_get_be32(f); qemu_get_buffer(f, vdev->config, vdev->config_len); num = qemu_get_be32(f); for (i = 0; i < num; i++) { vdev->vq[i].vring.num = qemu_get_be32(f); vdev->vq[i].pa = qemu_get_be64(f); qemu_get_be16s(f, &vdev->vq[i].last_avail_idx); qemu_get_be32s(f, &vdev->queue_size[i]); qemu_get_buffer(f, &vdev->queue_value, &vdev->queue_size[i]); } /* load of state has ended, time to start configuring things */ if (vdev->binding->load_config) { ret = vdev->binding->load_config(vdev->binding_opaque, f); if (ret) return ret; } for (i = 0; i < num; i++) { if (vdev->vq[i].pa) { virtqueue_init(&vdev->vq[i]); if (vdev->binding->load_queue) { ret = vdev->binding->load_queue(vdev->binding_opaque, i, f); if (ret) return ret; } } } virtio_notify_vector(vdev, VIRTIO_NO_VECTOR); return 0; } I.e. we just reserved the space to load the stuff (0 is ok), and after loading, We initialize things at virtio maintainers pace. Obviously the load_queue/config stuff has to be changed to work from a local variable instead of the savevm stream. Later, Juan.