[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState

[Juan Quintela <quintela@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> 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.