From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41326) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZgvhG-0001TK-Fh for qemu-devel@nongnu.org; Tue, 29 Sep 2015 10:15:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZgvhB-00028i-5R for qemu-devel@nongnu.org; Tue, 29 Sep 2015 10:15:58 -0400 Received: from lhrrgout.huawei.com ([194.213.3.17]:4727) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZgvhA-00028U-Qw for qemu-devel@nongnu.org; Tue, 29 Sep 2015 10:15:53 -0400 References: <1443094669-4144-1-git-send-email-marcandre.lureau@redhat.com> <1443094669-4144-24-git-send-email-marcandre.lureau@redhat.com> <560A91F8.1080209@huawei.com> <2105557509.19691388.1443533997719.JavaMail.zimbra@redhat.com> From: Claudio Fontana Message-ID: <560A9D0F.60200@huawei.com> Date: Tue, 29 Sep 2015 16:15:43 +0200 MIME-Version: 1.0 In-Reply-To: <2105557509.19691388.1443533997719.JavaMail.zimbra@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 23/47] ivshmem: migrate with VMStateDescription List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: drjones@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, marcandre lureau , cam@cs.ualberta.ca On 29.09.2015 15:39, Marc-André Lureau wrote: > > > ----- Original Message ----- >> On 24.09.2015 13:37, marcandre.lureau@redhat.com wrote: >>> From: Marc-André Lureau >>> >>> load_state_old() is used to keep compatibility with version 0. >>> >>> Signed-off-by: Marc-André Lureau >>> --- >>> hw/misc/ivshmem.c | 141 >>> ++++++++++++++++++++++++++++++++++-------------------- >>> 1 file changed, 88 insertions(+), 53 deletions(-) >>> >>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c >>> index 8207a98..54f7093 100644 >>> --- a/hw/misc/ivshmem.c >>> +++ b/hw/misc/ivshmem.c >>> @@ -651,56 +651,6 @@ static int ivshmem_setup_msi(IVShmemState * s) >>> return 0; >>> } >>> >>> -static void ivshmem_save(QEMUFile* f, void *opaque) >>> -{ >>> - IVShmemState *proxy = opaque; >>> - PCIDevice *pci_dev = PCI_DEVICE(proxy); >>> - >>> - IVSHMEM_DPRINTF("ivshmem_save\n"); >>> - pci_device_save(pci_dev, f); >>> - >>> - if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) { >>> - msix_save(pci_dev, f); >>> - } else { >>> - qemu_put_be32(f, proxy->intrstatus); >>> - qemu_put_be32(f, proxy->intrmask); >>> - } >>> - >>> -} >>> - >>> -static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) >>> -{ >>> - IVSHMEM_DPRINTF("ivshmem_load\n"); >>> - >>> - IVShmemState *proxy = opaque; >>> - PCIDevice *pci_dev = PCI_DEVICE(proxy); >>> - int ret; >>> - >>> - if (version_id > 0) { >>> - return -EINVAL; >>> - } >>> - >>> - if (proxy->role_val == IVSHMEM_PEER) { >>> - error_report("'peer' devices are not migratable"); >>> - return -EINVAL; >>> - } >>> - >>> - ret = pci_device_load(pci_dev, f); >>> - if (ret) { >>> - return ret; >>> - } >>> - >>> - if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) { >>> - msix_load(pci_dev, f); >>> - ivshmem_use_msix(proxy); >>> - } else { >>> - proxy->intrstatus = qemu_get_be32(f); >>> - proxy->intrmask = qemu_get_be32(f); >>> - } >>> - >>> - return 0; >>> -} >>> - >>> static void ivshmem_write_config(PCIDevice *pci_dev, uint32_t address, >>> uint32_t val, int len) >>> { >>> @@ -726,8 +676,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error >>> **errp) >>> } >>> >>> fifo8_create(&s->incoming_fifo, sizeof(long)); >>> - register_savevm(DEVICE(dev), "ivshmem", 0, 0, ivshmem_save, >>> ivshmem_load, >>> - >>> dev); >>> + >>> /* IRQFD requires MSI */ >>> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && >>> !ivshmem_has_feature(s, IVSHMEM_MSI)) { >>> @@ -853,10 +802,95 @@ static void pci_ivshmem_exit(PCIDevice *dev) >>> >>> memory_region_del_subregion(&s->bar, &s->ivshmem); >>> vmstate_unregister_ram(&s->ivshmem, DEVICE(dev)); >>> - unregister_savevm(DEVICE(dev), "ivshmem", s); >>> fifo8_destroy(&s->incoming_fifo); >>> } >>> >>> +static bool test_msix(void *opaque, int version_id) >>> +{ >>> + IVShmemState *s = opaque; >>> + >>> + return ivshmem_has_feature(s, IVSHMEM_MSI); >>> +} >>> + >>> +static bool test_no_msix(void *opaque, int version_id) >>> +{ >>> + return !test_msix(opaque, version_id); >>> +} >>> + >>> +static int ivshmem_pre_load(void *opaque) >>> +{ >>> + IVShmemState *s = opaque; >>> + >>> + if (s->role_val == IVSHMEM_PEER) { >>> + error_report("'peer' devices are not migratable"); >>> + return -EINVAL; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int ivshmem_post_load(void *opaque, int version_id) >>> +{ >>> + IVShmemState *s = opaque; >>> + >>> + if (ivshmem_has_feature(s, IVSHMEM_MSI)) { >>> + ivshmem_use_msix(s); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int ivshmem_load_old(QEMUFile *f, void *opaque, int version_id) >>> +{ >>> + IVShmemState *s = opaque; >>> + PCIDevice *pdev = PCI_DEVICE(s); >>> + int ret; >>> + >>> + IVSHMEM_DPRINTF("ivshmem_load_old\n"); >>> + >>> + if (version_id != 0) { >>> + return -EINVAL; >>> + } >>> + >>> + if (s->role_val == IVSHMEM_PEER) { >>> + error_report("'peer' devices are not migratable"); >>> + return -EINVAL; >>> + } >>> + >>> + ret = pci_device_load(pdev, f); >>> + if (ret) { >>> + return ret; >>> + } >>> + >>> + if (ivshmem_has_feature(s, IVSHMEM_MSI)) { >>> + msix_load(pdev, f); >>> + ivshmem_use_msix(s); >>> + } else { >>> + s->intrstatus = qemu_get_be32(f); >>> + s->intrmask = qemu_get_be32(f); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static const VMStateDescription ivshmem_vmsd = { >>> + .name = "ivshmem", >>> + .version_id = 1, >>> + .minimum_version_id = 1, >>> + .pre_load = ivshmem_pre_load, >>> + .post_load = ivshmem_post_load, >>> + .fields = (VMStateField[]) { >>> + VMSTATE_PCI_DEVICE(parent_obj, IVShmemState), >>> + >>> + VMSTATE_MSIX_TEST(parent_obj, IVShmemState, test_msix), >>> + VMSTATE_UINT32_TEST(intrstatus, IVShmemState, test_no_msix), >>> + VMSTATE_UINT32_TEST(intrmask, IVShmemState, test_no_msix), >>> + >>> + VMSTATE_END_OF_LIST() >>> + }, >>> + .load_state_old = ivshmem_load_old >> >> Should you not have a minimum_version_id_old if you want to support >> load_state_old? >> Granted, it should be =0, so it probably works anyway.. > > yes, I can make this more explicit though, agree. > > is the rest of the patch getting your reviewed-by? I would not have even bothered probably with the load_old, but I have nothing against adding it. Reviewed-by: Claudio Fontana > >> >>> +}; >>> + >>> static Property ivshmem_properties[] = { >>> DEFINE_PROP_CHR("chardev", IVShmemState, server_chr), >>> DEFINE_PROP_STRING("size", IVShmemState, sizearg), >>> @@ -882,6 +916,7 @@ static void ivshmem_class_init(ObjectClass *klass, void >>> *data) >>> k->class_id = PCI_CLASS_MEMORY_RAM; >>> dc->reset = ivshmem_reset; >>> dc->props = ivshmem_properties; >>> + dc->vmsd = &ivshmem_vmsd; >>> set_bit(DEVICE_CATEGORY_MISC, dc->categories); >>> } >>> >>>