[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: optional feature
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: optional feature |
Date: |
Wed, 16 Sep 2009 14:52:24 +0300 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Wed, Sep 16, 2009 at 01:48:35PM +0200, Juan Quintela wrote:
> Gleb Natapov <address@hidden> wrote:
> > On Wed, Sep 16, 2009 at 01:04:19PM +0200, Juan Quintela wrote:
> >> "Michael S. Tsirkin" <address@hidden> 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?
>
> See below, we are changing the state to one table, and tables don't have
> neither if's or whiles (we have a limited for that just walks arrays).
Let's just bite the bullet and add support for if's? It's not like it's
hard to invent 'struct vmstate_condition' or some such.
> I don't really know how to handle virtio in a sane way. The _saner_ way
> that I thought was to split it into three devices as I sketched below,
> but I still don't understood virtio creation to see how to "fix" it.
> It is next on queue after cpu (BIGGG), and ide (was waiting for Gerd
> patches to be commited. After that, I will thought again on how to
> handle virtio.
>
>
> >>
> >> 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: <num_elems> + 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.
- [Qemu-devel] optional feature (was Re: The State of the SaveVM format), Michael S. Tsirkin, 2009/09/16
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16
- [Qemu-devel] Re: optional feature, Gleb Natapov, 2009/09/16
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16
- [Qemu-devel] Re: optional feature,
Michael S. Tsirkin <=
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16
- [Qemu-devel] Re: optional feature, Michael S. Tsirkin, 2009/09/16
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16
- [Qemu-devel] Re: optional feature, Michael S. Tsirkin, 2009/09/16
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16
- [Qemu-devel] Re: optional feature, Michael S. Tsirkin, 2009/09/16
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16
- [Qemu-devel] Re: optional feature, Michael S. Tsirkin, 2009/09/16
- [Qemu-devel] Re: optional feature, Gleb Natapov, 2009/09/16
- [Qemu-devel] Re: optional feature, Juan Quintela, 2009/09/16