[Qemu-devel] [RFC PATCH] vmstate: Enable custom migration block name check

[Qemu-devel] [RFC PATCH] vmstate: Enable custom migration block name check

[Alexey Kardashevskiy]

This adds a callback to support custom names for migration blocks.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

RFC! not a real patch!

There was a problem a while ago how to migrate sPAPR TCE tables - they
needed unique id + instance_id and there 2 approaches for that:

1. Put them on a virtual made-up TCE bus, LIOBN (logical bus number) is
an unique ID and this would give TCE tables unique names like
liobn@80000000/spapr_iommu, instance id would always be 0.

vmstate_spapr_tce_table would be registered via DeviceClass::vmsd pointer.

2. Do not register vmsd via DeviceClass and use explicit call of
vmstate_register() using LIOBN as an instance id. This way TCE tables would
get "spapr_iommu" name and unique id == LIOBN.

Approach 2 is used by upstream.

Both 1 and 2 were suggested by maintainers :) However with 1 month delay
and I started using 1) in our internal build of "powerkvm".

In the current version of our internal "powerkvm" thing I used 2) as this
is what upstream uses.


The proposed patch is a part of a hack to allow migration
liobn@80000000/spapr_iommu + 0  to   spapr_iommu + 80000000.


Is this too horrible to be considered as a patch for upstream?
I am not going to push the second part of the hack anyway.

Is there any other way to implement the hack I want without toching these files?

Thanks!


---
 include/migration/vmstate.h | 1 +
 savevm.c                    | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 2759908..86afd9a 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -134,6 +134,7 @@ struct VMStateDescription {
     int (*pre_load)(void *opaque);
     int (*post_load)(void *opaque, int version_id);
     void (*pre_save)(void *opaque);
+    bool (*compat_name)(void *opaque, const char *idstr, int instance_id);
     VMStateField *fields;
     const VMStateSubsection *subsections;
 };
diff --git a/savevm.c b/savevm.c
index 22123be..60e7dd8 100644
--- a/savevm.c
+++ b/savevm.c
@@ -731,6 +731,12 @@ static SaveStateEntry *find_se(const char *idstr, int instance_id)
                  instance_id == se->alias_id))
                 return se;
         }
+
+        /* Migrating from a weird custom version? */
+        if (se->vmsd && se->vmsd->compat_name &&
+            se->vmsd->compat_name(se->opaque, idstr, instance_id)) {
+            return se;
+        }
     }
     return NULL;
 }
-- 
2.0.0

Re: [Qemu-devel] [RFC PATCH] vmstate: Enable custom migration block name check

[Alexander Graf]

On 25.08.14 12:22, Alexey Kardashevskiy wrote:
> This adds a callback to support custom names for migration blocks.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> RFC! not a real patch!
> 
> There was a problem a while ago how to migrate sPAPR TCE tables - they
> needed unique id + instance_id and there 2 approaches for that:
> 
> 1. Put them on a virtual made-up TCE bus, LIOBN (logical bus number) is
> an unique ID and this would give TCE tables unique names like
> liobn@80000000/spapr_iommu, instance id would always be 0.
> 
> vmstate_spapr_tce_table would be registered via DeviceClass::vmsd pointer.
> 
> 2. Do not register vmsd via DeviceClass and use explicit call of
> vmstate_register() using LIOBN as an instance id. This way TCE tables would
> get "spapr_iommu" name and unique id == LIOBN.
> 
> Approach 2 is used by upstream.
> 
> Both 1 and 2 were suggested by maintainers :) However with 1 month delay
> and I started using 1) in our internal build of "powerkvm".
> 
> In the current version of our internal "powerkvm" thing I used 2) as this
> is what upstream uses.
> 
> 
> The proposed patch is a part of a hack to allow migration
> liobn@80000000/spapr_iommu + 0  to   spapr_iommu + 80000000.
> 
> 
> Is this too horrible to be considered as a patch for upstream?

Is there any reason you can't keep this patch in your downstream fork
along with the user of it? :)


Alex

Re: [Qemu-devel] [RFC PATCH] vmstate: Enable custom migration block name check

[Alexey Kardashevskiy]

On 08/25/2014 08:23 PM, Alexander Graf wrote:
> 
> 
> On 25.08.14 12:22, Alexey Kardashevskiy wrote:
>> This adds a callback to support custom names for migration blocks.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> RFC! not a real patch!
>>
>> There was a problem a while ago how to migrate sPAPR TCE tables - they
>> needed unique id + instance_id and there 2 approaches for that:
>>
>> 1. Put them on a virtual made-up TCE bus, LIOBN (logical bus number) is
>> an unique ID and this would give TCE tables unique names like
>> liobn@80000000/spapr_iommu, instance id would always be 0.
>>
>> vmstate_spapr_tce_table would be registered via DeviceClass::vmsd pointer.
>>
>> 2. Do not register vmsd via DeviceClass and use explicit call of
>> vmstate_register() using LIOBN as an instance id. This way TCE tables would
>> get "spapr_iommu" name and unique id == LIOBN.
>>
>> Approach 2 is used by upstream.
>>
>> Both 1 and 2 were suggested by maintainers :) However with 1 month delay
>> and I started using 1) in our internal build of "powerkvm".
>>
>> In the current version of our internal "powerkvm" thing I used 2) as this
>> is what upstream uses.
>>
>>
>> The proposed patch is a part of a hack to allow migration
>> liobn@80000000/spapr_iommu + 0  to   spapr_iommu + 80000000.
>>
>>
>> Is this too horrible to be considered as a patch for upstream?
> 
> Is there any reason you can't keep this patch in your downstream fork
> along with the user of it? :)


I can and most likely will. But someone else could benefit from it sometime
later, dunno, there are already manymany callbacks, why not one more :)

But mostly - I actually want to know if what patch does can be done without
it. Enormous amount of callbacks and flags tell me that it is possible, I
am just not smart enough to see it :)



-- 
Alexey