[Qemu-devel] [PATCH 0/3] non-migratable devices

[Qemu-devel] [PATCH 0/3] non-migratable devices

[Gerd Hoffmann]

  Hi,

This patch series adds an easy way to tag devices as non-migratable
puts it into use for ahci and ehci.

Gerd Hoffmann (3):
  vmstate: add no_migrate flag to VMStateDescription
  ahci doesn't support migration
  ehci doesn't support migration

 hw/hw.h       |    1 +
 hw/ide/ich.c  |    6 ++++++
 hw/usb-ehci.c |    6 ++++++
 savevm.c      |    1 +
 4 files changed, 14 insertions(+), 0 deletions(-)

[Qemu-devel] [PATCH 1/3] vmstate: add no_migrate flag to VMStateDescription

[Gerd Hoffmann]

This allows to easily tag devices as non-migratable,
so any attempt to migrate a virtual machine with the
device in question active will make migration fail.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/hw.h  |    1 +
 savevm.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 9dd7096..1eb3486 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -324,6 +324,7 @@ typedef struct VMStateSubsection {
 
 struct VMStateDescription {
     const char *name;
+    int no_migrate;
     int version_id;
     int minimum_version_id;
     int minimum_version_id_old;
diff --git a/savevm.c b/savevm.c
index 8139bc7..fa2da3e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1234,6 +1234,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     se->opaque = opaque;
     se->vmsd = vmsd;
     se->alias_id = alias_id;
+    se->no_migrate = vmsd->no_migrate;
 
     if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
         char *id = dev->parent_bus->info->get_dev_path(dev);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 1/3] vmstate: add no_migrate flag to VMStateDescription

[Anthony Liguori]

On 07/08/2011 03:53 AM, Gerd Hoffmann wrote:
> This allows to easily tag devices as non-migratable,
> so any attempt to migrate a virtual machine with the
> device in question active will make migration fail.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>

register_device_unmigratable()?

Regards,

Anthony Liguori

> ---
>   hw/hw.h  |    1 +
>   savevm.c |    1 +
>   2 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/hw.h b/hw/hw.h
> index 9dd7096..1eb3486 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -324,6 +324,7 @@ typedef struct VMStateSubsection {
>
>   struct VMStateDescription {
>       const char *name;
> +    int no_migrate;
>       int version_id;
>       int minimum_version_id;
>       int minimum_version_id_old;
> diff --git a/savevm.c b/savevm.c
> index 8139bc7..fa2da3e 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1234,6 +1234,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>       se->opaque = opaque;
>       se->vmsd = vmsd;
>       se->alias_id = alias_id;
> +    se->no_migrate = vmsd->no_migrate;
>
>       if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
>           char *id = dev->parent_bus->info->get_dev_path(dev);

Re: [Qemu-devel] [PATCH 1/3] vmstate: add no_migrate flag to VMStateDescription

[Gerd Hoffmann]

On 07/08/11 16:26, Anthony Liguori wrote:
> On 07/08/2011 03:53 AM, Gerd Hoffmann wrote:
>> This allows to easily tag devices as non-migratable,
>> so any attempt to migrate a virtual machine with the
>> device in question active will make migration fail.
>>
>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
> register_device_unmigratable()?

That is just a more complicated way to do the same ;)
Wanna have a patch to zap it?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 1/3] vmstate: add no_migrate flag to VMStateDescription

[Gerd Hoffmann]

On 07/08/11 17:44, Gerd Hoffmann wrote:
> On 07/08/11 16:26, Anthony Liguori wrote:
>> register_device_unmigratable()?
>
> Wanna have a patch to zap it?

Ah, no, we can't, for ivshmem this isn't fixed but depends on the 
configuration, so a static flag in the VMStateDescription doesn't cut it.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 1/3] vmstate: add no_migrate flag to VMStateDescription

[Anthony Liguori]

On 07/08/2011 11:04 AM, Gerd Hoffmann wrote:
> On 07/08/11 17:44, Gerd Hoffmann wrote:
>> On 07/08/11 16:26, Anthony Liguori wrote:
>>> register_device_unmigratable()?
>>
>> Wanna have a patch to zap it?
>
> Ah, no, we can't, for ivshmem this isn't fixed but depends on the
> configuration, so a static flag in the VMStateDescription doesn't cut it.

Right, but can we have some continuity at least between the two interfaces?

At least make the vmstate flag 'unmigratable' or rename the function to 
qdev_set_no_migrate().

BTW, should this be a vmstate flag or a qdev flag?

Regards,

Anthony Liguori

>
> cheers,
> Gerd
>

Re: [Qemu-devel] [PATCH 1/3] vmstate: add no_migrate flag to VMStateDescription

[Gerd Hoffmann]

>>>> register_device_unmigratable()?

> Right, but can we have some continuity at least between the two interfaces?
>
> At least make the vmstate flag 'unmigratable' or rename the function to
> qdev_set_no_migrate().

Will rename the flag.

> BTW, should this be a vmstate flag or a qdev flag?

vmstate.  savevm.c doesn't look at the qdev tree.

cheers,
   Gerd

[Qemu-devel] [PATCH 2/3] ahci doesn't support migration

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/ich.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 054e073..0f26603 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -72,6 +72,11 @@
 #include <hw/ide/pci.h>
 #include <hw/ide/ahci.h>
 
+static const VMStateDescription vmstate_ahci = {
+    .name = "ahci",
+    .no_migrate = 1,
+};
+
 static int pci_ich9_ahci_init(PCIDevice *dev)
 {
     struct AHCIPCIState *d;
@@ -123,6 +128,7 @@ static PCIDeviceInfo ich_ahci_info[] = {
         .qdev.name    = "ich9-ahci",
         .qdev.alias   = "ahci",
         .qdev.size    = sizeof(AHCIPCIState),
+        .qdev.vmsd    = &vmstate_ahci,
         .init         = pci_ich9_ahci_init,
         .exit         = pci_ich9_uninit,
         .config_write = pci_ich9_write_config,
-- 
1.7.1

[Qemu-devel] [PATCH 3/3] ehci doesn't support migration

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb-ehci.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 91fb7de..a0449be 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -2136,9 +2136,15 @@ static USBBusOps ehci_bus_ops = {
     .device_destroy = ehci_device_destroy,
 };
 
+static const VMStateDescription vmstate_ehci = {
+    .name = "ehci",
+    .no_migrate = 1,
+};
+
 static PCIDeviceInfo ehci_info = {
     .qdev.name    = "usb-ehci",
     .qdev.size    = sizeof(EHCIState),
+    .qdev.vmsd    = &vmstate_ehci,
     .init         = usb_ehci_initfn,
     .vendor_id    = PCI_VENDOR_ID_INTEL,
     .device_id    = PCI_DEVICE_ID_INTEL_82801D,
-- 
1.7.1

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Paolo Bonzini]

On 07/08/2011 10:53 AM, Gerd Hoffmann wrote:
>    Hi,
>
> This patch series adds an easy way to tag devices as non-migratable
> puts it into use for ahci and ehci.
>
> Gerd Hoffmann (3):
>    vmstate: add no_migrate flag to VMStateDescription
>    ahci doesn't support migration
>    ehci doesn't support migration
>
>   hw/hw.h       |    1 +
>   hw/ide/ich.c  |    6 ++++++
>   hw/usb-ehci.c |    6 ++++++
>   savevm.c      |    1 +
>   4 files changed, 14 insertions(+), 0 deletions(-)
>
>
>

ACK series

For scsi-cd and other empty-but-generally-migratable devices we should 
probably add an empty vmstate, so that any further addition can be done 
as subsections.

Paolo

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Gerd Hoffmann]

   Hi,

> For scsi-cd and other empty-but-generally-migratable devices we should
> probably add an empty vmstate, so that any further addition can be done
> as subsections.

That will break migration to older versions which don't know about the 
new sections, even if they are empty ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Paolo Bonzini]

On 07/08/2011 11:59 AM, Gerd Hoffmann wrote:
>
>
>> For scsi-cd and other empty-but-generally-migratable devices we should
>> probably add an empty vmstate, so that any further addition can be done
>> as subsections.
>
> That will break migration to older versions which don't know about the
> new sections, even if they are empty ...

scsi-cd is new in 0.15, older versions have scsi-disk only.  That's why 
we should take the occasion to add the empty vmstate at least to it.

For other devices, it's better to make the breakage in a single version. 
  It also allows to make 0.15 the "flag day" where each device shall 
have a VMState or the entire VM will not be migratable.  If desired, in 
the future we can revert this behavior for pc-0.14 and earlier machines.

Paolo

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Gerd Hoffmann]

   Hi,

> scsi-cd is new in 0.15, older versions have scsi-disk only. That's why
> we should take the occasion to add the empty vmstate at least to it.

Ok, good opportunity in that specific case.

> For other devices, it's better to make the breakage in a single version.
> It also allows to make 0.15 the "flag day" where each device shall have
> a VMState or the entire VM will not be migratable.

Not that easy given that we didn't fully migrate to vmstate yet, 
otherwise we could simply fail migration in case we find any device with 
qdev->vmsd == NULL.

> If desired, in the
> future we can revert this behavior for pc-0.14 and earlier machines.

And I'm still looking for a sane way to handle *this*.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Paolo Bonzini]

On 07/08/2011 01:47 PM, Gerd Hoffmann wrote:
>
>
>> scsi-cd is new in 0.15, older versions have scsi-disk only. That's why
>> we should take the occasion to add the empty vmstate at least to it.
>
> Ok, good opportunity in that specific case.
>
>> For other devices, it's better to make the breakage in a single version.
>> It also allows to make 0.15 the "flag day" where each device shall have
>> a VMState or the entire VM will not be migratable.
>
> Not that easy given that we didn't fully migrate to vmstate yet,
> otherwise we could simply fail migration in case we find any device with
> qdev->vmsd == NULL.

You can always add

#define VMSD_NONE  ((const VMStateDescription *) 1)

>> If desired, in the
>> future we can revert this behavior for pc-0.14 and earlier machines.
>
> And I'm still looking for a sane way to handle *this*.

One possibility is to add device name remapping to machine types, like 
"ide is actually ide-0.14 when using pc-0.14" and put a different 
VMState in ide-0.14.

Paolo

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Gerd Hoffmann]

   Hi,

>>> If desired, in the
>>> future we can revert this behavior for pc-0.14 and earlier machines.
>>
>> And I'm still looking for a sane way to handle *this*.
>
> One possibility is to add device name remapping to machine types, like
> "ide is actually ide-0.14 when using pc-0.14" and put a different
> VMState in ide-0.14.

That isn't very different from subsections.

Problem is that the old qemu version doesn't have the section at all, 
not that it looks somehow different.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Paolo Bonzini]

On 07/08/2011 02:47 PM, Gerd Hoffmann wrote:
>>
>> One possibility is to add device name remapping to machine types, like
>> "ide is actually ide-0.14 when using pc-0.14" and put a different
>> VMState in ide-0.14.
>
> That isn't very different from subsections.
>
> Problem is that the old qemu version doesn't have the section at all,
> not that it looks somehow different.

No, I meant a different qdev name, i.e. mapping the same device name to 
a different DeviceInfo.

Paolo

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Gerd Hoffmann]

On 07/08/11 14:59, Paolo Bonzini wrote:
> On 07/08/2011 02:47 PM, Gerd Hoffmann wrote:
>>>
>>> One possibility is to add device name remapping to machine types, like
>>> "ide is actually ide-0.14 when using pc-0.14" and put a different
>>> VMState in ide-0.14.
>>
>> That isn't very different from subsections.
>>
>> Problem is that the old qemu version doesn't have the section at all,
>> not that it looks somehow different.
>
> No, I meant a different qdev name, i.e. mapping the same device name to
> a different DeviceInfo.

That is an idea.  This could also replace compat properties (by giving 
ide-0.14 different default values for the properties) and thereby move 
the compat info from hw/pc.c to hw/ide/xxx.c.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] non-migratable devices

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> For scsi-cd and other empty-but-generally-migratable devices we should
>> probably add an empty vmstate, so that any further addition can be done
>> as subsections.
>
> That will break migration to older versions which don't know about the
> new sections, even if they are empty ...

Send them only when running a sufficiently recent -M?

[Qemu-devel] [PATCH 4/3] vmstate: complain about devices without vmstate

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/hw.h   |    2 ++
 hw/qdev.c |    7 ++++++-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 1eb3486..6d6c493 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -336,6 +336,8 @@ struct VMStateDescription {
     const VMStateSubsection *subsections;
 };
 
+#define VMSD_NONE ((const VMStateDescription*)1)
+
 extern const VMStateInfo vmstate_info_bool;
 
 extern const VMStateInfo vmstate_info_int8;
diff --git a/hw/qdev.c b/hw/qdev.c
index 292b52f..fafbbae 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -283,7 +283,12 @@ int qdev_init(DeviceState *dev)
         qdev_free(dev);
         return rc;
     }
-    if (dev->info->vmsd) {
+    if (dev->info->vmsd == NULL) {
+        /* TODO: fixup qemu source code, then make this an assert() */
+        error_report("WARNING: device %s has no vmstate\n", dev->info->name);
+    } else if (dev->info->vmsd == VMSD_NONE) {
+        /* device doesn't need vmstate */;
+    } else {
         vmstate_register_with_alias_id(dev, -1, dev->info->vmsd, dev,
                                        dev->instance_id_alias,
                                        dev->alias_required_for_version);
-- 
1.7.1