Patchwork vmstate: Add unmigratable flag

login
register
mail settings
Submitter Jan Kiszka
Date June 9, 2011, 4:44 p.m.
Message ID <4DF0F86E.8040307@siemens.com>
Download mbox | patch
Permalink /patch/99783/
State New
Headers show

Comments

Jan Kiszka - June 9, 2011, 4:44 p.m.
A first step towards getting rid of register_device_unmigratable
(ivshmem and lacking vmstate support in virtio are blocking this):

Allow to register an unmigratable vmstate via qdev, i.e. tag a device
declaratively.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/hw.h  |    1 +
 savevm.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)
Markus Armbruster - June 9, 2011, 6:27 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> A first step towards getting rid of register_device_unmigratable
> (ivshmem and lacking vmstate support in virtio are blocking this):
>
> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
> declaratively.

Declarative is nicer than imperative.  ACK.
Anthony Liguori - June 9, 2011, 8 p.m.
On 06/09/2011 11:44 AM, Jan Kiszka wrote:
> A first step towards getting rid of register_device_unmigratable
> (ivshmem and lacking vmstate support in virtio are blocking this):
>
> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
> declaratively.

I thought part of the problem with this was that for some devices (like 
ivshmem), whether it can be migrated was dynamic.  It depends on 
configuration, state, etc.

Regards,

Anthony Liguori

>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.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 56447a7..97c474e 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -333,6 +333,7 @@ struct VMStateDescription {
>       void (*pre_save)(void *opaque);
>       VMStateField *fields;
>       const VMStateSubsection *subsections;
> +    bool unmigratable;
>   };
>
>   extern const VMStateInfo vmstate_info_bool;
> diff --git a/savevm.c b/savevm.c
> index 939845c..98b2422 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->unmigratable;
>
>       if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
>           char *id = dev->parent_bus->info->get_dev_path(dev);
Jan Kiszka - June 9, 2011, 8:39 p.m.
On 2011-06-09 22:00, Anthony Liguori wrote:
> On 06/09/2011 11:44 AM, Jan Kiszka wrote:
>> A first step towards getting rid of register_device_unmigratable
>> (ivshmem and lacking vmstate support in virtio are blocking this):
>>
>> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
>> declaratively.
> 
> I thought part of the problem with this was that for some devices (like
> ivshmem), whether it can be migrated was dynamic.  It depends on
> configuration, state, etc.

That only applies to ivshmem (the other user is device assignment which
is unconditionally unmigratable). And the ivshmem issue could easily be
solved by defining two devices, ivshmem-peer (or just ivshmem) and
ivshmem-master, eliminating the need for the role property.

I don't think there will ever be a use case for a "transformer" device
that becomes unmigratable during runtime (would be a nightmare for
management apps anyway).

If breaking the user interface of ivshmem for this is OK, I'll post a patch.

Jan
Cam Macdonell - June 19, 2011, 8:46 p.m.
On Thu, Jun 9, 2011 at 2:39 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2011-06-09 22:00, Anthony Liguori wrote:
>> On 06/09/2011 11:44 AM, Jan Kiszka wrote:
>>> A first step towards getting rid of register_device_unmigratable
>>> (ivshmem and lacking vmstate support in virtio are blocking this):
>>>
>>> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
>>> declaratively.
>>
>> I thought part of the problem with this was that for some devices (like
>> ivshmem), whether it can be migrated was dynamic.  It depends on
>> configuration, state, etc.
>
> That only applies to ivshmem (the other user is device assignment which
> is unconditionally unmigratable). And the ivshmem issue could easily be
> solved by defining two devices, ivshmem-peer (or just ivshmem) and
> ivshmem-master, eliminating the need for the role property.
>
> I don't think there will ever be a use case for a "transformer" device
> that becomes unmigratable during runtime (would be a nightmare for
> management apps anyway).
>
> If breaking the user interface of ivshmem for this is OK, I'll post a patch.
>
> Jan
>
>

The migratability of ivshmem is not dynamic in that it doesn't change
at runtime, it's set when the device is created, either role=peer or
role=master is specified.  So iiuc, this could work with ivshmem.

Cam
Jan Kiszka - June 20, 2011, 9:05 a.m.
On 2011-06-19 22:46, Cam Macdonell wrote:
> On Thu, Jun 9, 2011 at 2:39 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2011-06-09 22:00, Anthony Liguori wrote:
>>> On 06/09/2011 11:44 AM, Jan Kiszka wrote:
>>>> A first step towards getting rid of register_device_unmigratable
>>>> (ivshmem and lacking vmstate support in virtio are blocking this):
>>>>
>>>> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
>>>> declaratively.
>>>
>>> I thought part of the problem with this was that for some devices (like
>>> ivshmem), whether it can be migrated was dynamic.  It depends on
>>> configuration, state, etc.
>>
>> That only applies to ivshmem (the other user is device assignment which
>> is unconditionally unmigratable). And the ivshmem issue could easily be
>> solved by defining two devices, ivshmem-peer (or just ivshmem) and
>> ivshmem-master, eliminating the need for the role property.
>>
>> I don't think there will ever be a use case for a "transformer" device
>> that becomes unmigratable during runtime (would be a nightmare for
>> management apps anyway).
>>
>> If breaking the user interface of ivshmem for this is OK, I'll post a patch.
>>
>> Jan
>>
>>
> 
> The migratability of ivshmem is not dynamic in that it doesn't change
> at runtime, it's set when the device is created, either role=peer or
> role=master is specified.  So iiuc, this could work with ivshmem.

So you are fine with breaking the interface? Everyone else as well? Then
I'll cook a patch to sort at least this out for 0.15.

Jan
Cam Macdonell - June 21, 2011, 8:25 p.m.
On Mon, Jun 20, 2011 at 3:05 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-06-19 22:46, Cam Macdonell wrote:
>> On Thu, Jun 9, 2011 at 2:39 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>> On 2011-06-09 22:00, Anthony Liguori wrote:
>>>> On 06/09/2011 11:44 AM, Jan Kiszka wrote:
>>>>> A first step towards getting rid of register_device_unmigratable
>>>>> (ivshmem and lacking vmstate support in virtio are blocking this):
>>>>>
>>>>> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
>>>>> declaratively.
>>>>
>>>> I thought part of the problem with this was that for some devices (like
>>>> ivshmem), whether it can be migrated was dynamic.  It depends on
>>>> configuration, state, etc.
>>>
>>> That only applies to ivshmem (the other user is device assignment which
>>> is unconditionally unmigratable). And the ivshmem issue could easily be
>>> solved by defining two devices, ivshmem-peer (or just ivshmem) and
>>> ivshmem-master, eliminating the need for the role property.
>>>
>>> I don't think there will ever be a use case for a "transformer" device
>>> that becomes unmigratable during runtime (would be a nightmare for
>>> management apps anyway).
>>>
>>> If breaking the user interface of ivshmem for this is OK, I'll post a patch.
>>>
>>> Jan
>>>
>>>
>>
>> The migratability of ivshmem is not dynamic in that it doesn't change
>> at runtime, it's set when the device is created, either role=peer or
>> role=master is specified.  So iiuc, this could work with ivshmem.
>
> So you are fine with breaking the interface? Everyone else as well? Then
> I'll cook a patch to sort at least this out for 0.15.
>

To be clear, this would break the interface in that a device cannot
specify whether it's migratable via a parameter?

> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
>
>
Jan Kiszka - June 21, 2011, 8:29 p.m.
On 2011-06-21 22:25, Cam Macdonell wrote:
> On Mon, Jun 20, 2011 at 3:05 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-06-19 22:46, Cam Macdonell wrote:
>>> On Thu, Jun 9, 2011 at 2:39 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>> On 2011-06-09 22:00, Anthony Liguori wrote:
>>>>> On 06/09/2011 11:44 AM, Jan Kiszka wrote:
>>>>>> A first step towards getting rid of register_device_unmigratable
>>>>>> (ivshmem and lacking vmstate support in virtio are blocking this):
>>>>>>
>>>>>> Allow to register an unmigratable vmstate via qdev, i.e. tag a device
>>>>>> declaratively.
>>>>>
>>>>> I thought part of the problem with this was that for some devices (like
>>>>> ivshmem), whether it can be migrated was dynamic.  It depends on
>>>>> configuration, state, etc.
>>>>
>>>> That only applies to ivshmem (the other user is device assignment which
>>>> is unconditionally unmigratable). And the ivshmem issue could easily be
>>>> solved by defining two devices, ivshmem-peer (or just ivshmem) and
>>>> ivshmem-master, eliminating the need for the role property.
>>>>
>>>> I don't think there will ever be a use case for a "transformer" device
>>>> that becomes unmigratable during runtime (would be a nightmare for
>>>> management apps anyway).
>>>>
>>>> If breaking the user interface of ivshmem for this is OK, I'll post a patch.
>>>>
>>>> Jan
>>>>
>>>>
>>>
>>> The migratability of ivshmem is not dynamic in that it doesn't change
>>> at runtime, it's set when the device is created, either role=peer or
>>> role=master is specified.  So iiuc, this could work with ivshmem.
>>
>> So you are fine with breaking the interface? Everyone else as well? Then
>> I'll cook a patch to sort at least this out for 0.15.
>>
> 
> To be clear, this would break the interface in that a device cannot
> specify whether it's migratable via a parameter?

It will break the interface how to set up a peer or master ivshmem, from
'-device ivshmem,role=X' to '-device ivshmem-X' (with the role property
removed).

Jan

Patch

diff --git a/hw/hw.h b/hw/hw.h
index 56447a7..97c474e 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -333,6 +333,7 @@  struct VMStateDescription {
     void (*pre_save)(void *opaque);
     VMStateField *fields;
     const VMStateSubsection *subsections;
+    bool unmigratable;
 };
 
 extern const VMStateInfo vmstate_info_bool;
diff --git a/savevm.c b/savevm.c
index 939845c..98b2422 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->unmigratable;
 
     if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
         char *id = dev->parent_bus->info->get_dev_path(dev);