From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MnsJN-0002jn-7m for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:04:33 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MnsJI-0002fc-Bd for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:04:32 -0400 Received: from [199.232.76.173] (port=45564 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MnsJH-0002fI-V1 for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:04:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33994) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MnsJH-0003ji-8h for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:04:27 -0400 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8GB4Mt6031077 for ; Wed, 16 Sep 2009 07:04:22 -0400 From: Juan Quintela In-Reply-To: <20090916104620.GA4456@redhat.com> (Michael S. Tsirkin's message of "Wed, 16 Sep 2009 13:46:20 +0300") References: <20090916104620.GA4456@redhat.com> Date: Wed, 16 Sep 2009 13:04:19 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: [Qemu-devel] Re: optional feature List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, gleb@redhat.com "Michael S. Tsirkin" wrote: > On Wed, Sep 09, 2009 at 10:47:27AM +0200, Juan Quintela wrote: >> How do we deal with optional features? > > Here's an idea that Gleb suggested in a private > conversation: make optional features into > separate, non-user-visible devices. > > Thus we would have vmstate for virtio and additionally, if msix is > enabled, vmstate for msix. This solves the problem of the number of > devices becoming exponential with the number of features: we have device > per feature. > > I understand that RTC does something like this. And it is wrong :) I sent a patch to fix it properly, but we have the problem of backward compatibility with kvm. Forget msix for virtio, virtio has the problem already with pci. virtio_save() { if (vdev->binding->save_config) vdev->binding->save_config(vdev->binding_opaque, f); qemu_put_8s(f, &vdev->status); .... some other normal fields ... for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) { if (vdev->vq[i].vring.num == 0) break; Not a problem, we can precalculate i on pre_save() qemu_put_be32(f, vdev->vq[i].vring.num); qemu_put_be64(f, vdev->vq[i].pa); qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); This is sending a partial array of struct (the "i" 1st entries) No problem here. if (vdev->binding->save_queue) vdev->binding->save_queue(vdev->binding_opaque, i, f); Again, what to do with this one. } } Looking at what does virtio_pci_save_queue() static void virtio_pci_save_queue(void * opaque, int n, QEMUFile *f) { VirtIOPCIProxy *proxy = opaque; if (msix_present(&proxy->pci_dev)) qemu_put_be16(f, virtio_queue_vector(proxy->vdev, n)); } i.e. and now, an optional field. And no, I don't have either a clean design that will be backward compatible and clean. Clean design is easy: virtio virtio-pci (it does the equivalent of save_config() and then call virtio_save) virtio-pci-msix (it calls virtio-pci and then send a partial array of queues. (the save queue thing) Before you ask, partial arrays are sent: + array where num_elems == 0 is valid. But this is the "good" design if we started _now_, that is not the case, and I am trying to get something clean and bacward compatible. Later, Juan. PD. Optional fields are going to have to be in, arm cpus really need them if we want to maintain backward compatibility.