Re: [Qemu-devel] [PATCH RFC 8/8] virtio: add endian-ambivalent support to VirtIODevice

[Dr. David Alan Gilbert]

* Andreas F?rber (address@hidden) wrote:
> Am 19.05.2014 15:06, schrieb Greg Kurz:
> > On Mon, 19 May 2014 10:39:09 +0200
> > Greg Kurz <address@hidden> wrote:
> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >> index 7fbad29..6578854 100644
> >> --- a/hw/virtio/virtio.c
> >> +++ b/hw/virtio/virtio.c
> [...]
> >> @@ -839,10 +849,39 @@ typedef struct VirtIOSubsection {
> >>      int version_id;
> >>      void (*save)(VirtIODevice *vdev, QEMUFile *f);
> >>      int (*load)(VirtIODevice *vdev, QEMUFile *f);
> >> -    int (*needed)(VirtIODevice *vdev);
> >> +    bool (*needed)(VirtIODevice *vdev);
> >>  } VirtIOSubsection;
> >>
> >> +static void virtio_save_device_endian(VirtIODevice *vdev, QEMUFile *f)
> >> +{
> >> +    qemu_put_byte(f, vdev->device_endian);
> >> +}
> >> +
> >> +static int virtio_load_device_endian(VirtIODevice *vdev, QEMUFile *f)
> >> +{
> >> +    vdev->device_endian = qemu_get_byte(f);
> >> +    return 0;
> >> +}
> >> +
> >> +static bool virtio_device_endian_needed(VirtIODevice *vdev)
> >> +{
> >> +    /* No migration is supposed to occur while we are loading state.
> >> +     */
> >> +    assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
> >> +    if (target_words_bigendian()) {
> >> +        return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE;
> >> +    } else {
> >> +        return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
> >> +    }
> >> +}
> >> +
> >>  static const VirtIOSubsection virtio_subsection[] = {
> >> +    { .name = "virtio/device_endian",
> > 
> > Can anyone comment the subsection name ? Is there a chance the
> > VMState port would come up with the same ?
> > 
> >> +      .version_id = 1,
> >> +      .save = virtio_save_device_endian,
> >> +      .load = virtio_load_device_endian,
> >> +      .needed = virtio_device_endian_needed,
> >> +    },
> >>      { .name = NULL }
> >>  };
> >>
> 
> Different question: With converting VirtIO to VMState in mind, why are
> you not using a regular VMStateSubsection and loading/saving that as
> part of the old-style load/save functions? Is an API for that missing?

There are a handful of places that call into vmstate from a non-vmstate
routine but I don't think they're using plain subsections.

hw/pci/pci.c: pci_device_save/load
hw/scsi/spapr_vscsi.c: vscsi_save_request
hw/acpi/piix4.c: acpi_load_old

Dave
> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK