From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MnsXI-00072S-BR for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:18:56 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MnsXD-00071p-FL for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:18:55 -0400 Received: from [199.232.76.173] (port=36384 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MnsXD-00071m-AW for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:18:51 -0400 Received: from mx20.gnu.org ([199.232.41.8]:37762) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MnsXD-0000Wx-0Q for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:18:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MnsXB-0007Ls-6X for qemu-devel@nongnu.org; Wed, 16 Sep 2009 07:18:49 -0400 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8GBIlbD025511 for ; Wed, 16 Sep 2009 07:18:47 -0400 Date: Wed, 16 Sep 2009 14:18:45 +0300 From: Gleb Natapov Message-ID: <20090916111845.GJ23157@redhat.com> References: <20090916104620.GA4456@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: optional feature List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" On Wed, Sep 16, 2009 at 01:04:19PM +0200, Juan Quintela wrote: > "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. What is wrong about it? > > 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. -- Gleb.