[Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

This should prevent the destination guest from misbehaving when
the threads number is different in "-smp" command.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 target-ppc/cpu-qom.h | 2 +-
 target-ppc/machine.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 5eb56ea..59d0d4a 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -91,7 +91,7 @@ struct PowerPCCPU {
     /*< public >*/
 
     CPUPPCState env;
-    int cpu_dt_id;
+    uint32_t cpu_dt_id;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 834297e..92529ca 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -500,7 +500,7 @@ static const VMStateInfo vmstate_pvr = {
 
 const VMStateDescription vmstate_ppc_cpu = {
     .name = "cpu",
-    .version_id = 5,
+    .version_id = 6,
     .minimum_version_id = 5,
     .minimum_version_id_old = 4,
     .load_state_old = cpu_load_old,
@@ -537,6 +537,8 @@ const VMStateDescription vmstate_ppc_cpu = {
         VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
         VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
         VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
+        VMSTATE_UINT32_EQUAL(cpu_dt_id, PowerPCCPU),
+
         VMSTATE_END_OF_LIST()
     },
     .subsections = (VMStateSubsection []) {
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Andreas Färber]

Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
> This should prevent the destination guest from misbehaving when
> the threads number is different in "-smp" command.

Sorry, I don't understand. When migrating, surely -smp needs to be the
same on source and destination, so how can they differ?

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

On 03/27/2014 11:37 PM, Andreas Färber wrote:
> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>> This should prevent the destination guest from misbehaving when
>> the threads number is different in "-smp" command.
> 
> Sorry, I don't understand. When migrating, surely -smp needs to be the
> same on source and destination, so how can they differ?


The idea is that "-smp" does not migrate and if we run source and
destination guests with different numbers in -smp, we end up with weird
machine because interrupt server properties (gserver?) in the device tree
are different and no reconfiguration happens (no such mechanism as I know)
the whole XICS behaves different.

The patch just prevents migration from succeeding which seems better.

-- 
Alexey

Re: [Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Peter Maydell]

On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>> This should prevent the destination guest from misbehaving when
>>> the threads number is different in "-smp" command.
>>
>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>> same on source and destination, so how can they differ?
>
>
> The idea is that "-smp" does not migrate and if we run source and
> destination guests with different numbers in -smp, we end up with weird
> machine

Yes, so don't do that. As I understand it:
 (1) if you don't run QEMU with the exact same command line
     and config at both ends then migration won't work
 (2) we don't guarantee to detect and cleanly fail if you
     don't do (1)

It would probably be nice if we did detect config mismatches,
but that seems to me like a problem we should be addressing
more globally than just for one particular config item for
one particular target...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

On 03/27/2014 11:57 PM, Peter Maydell wrote:
> On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>>> This should prevent the destination guest from misbehaving when
>>>> the threads number is different in "-smp" command.
>>>
>>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>>> same on source and destination, so how can they differ?
>>
>>
>> The idea is that "-smp" does not migrate and if we run source and
>> destination guests with different numbers in -smp, we end up with weird
>> machine
> 
> Yes, so don't do that. As I understand it:
>  (1) if you don't run QEMU with the exact same command line
>      and config at both ends then migration won't work
>  (2) we don't guarantee to detect and cleanly fail if you
>      don't do (1)
> 
> It would probably be nice if we did detect config mismatches,

Yep, we do not send the device tree (as libvirt does). Pure command line
matching won't work.

> but that seems to me like a problem we should be addressing
> more globally than just for one particular config item for
> one particular target...

This is lot bigger that the issue I am trying to solve by this patch...
Still nack?


-- 
Alexey

Re: [Qemu-devel] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

On 03/28/2014 12:07 AM, Alexey Kardashevskiy wrote:
> On 03/27/2014 11:57 PM, Peter Maydell wrote:
>> On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>>>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>>>> This should prevent the destination guest from misbehaving when
>>>>> the threads number is different in "-smp" command.
>>>>
>>>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>>>> same on source and destination, so how can they differ?
>>>
>>>
>>> The idea is that "-smp" does not migrate and if we run source and
>>> destination guests with different numbers in -smp, we end up with weird
>>> machine
>>
>> Yes, so don't do that. As I understand it:
>>  (1) if you don't run QEMU with the exact same command line
>>      and config at both ends then migration won't work
>>  (2) we don't guarantee to detect and cleanly fail if you
>>      don't do (1)
>>
>> It would probably be nice if we did detect config mismatches,
> 
> Yep, we do not send the device tree (as libvirt does). Pure command line
> matching won't work.
> 
>> but that seems to me like a problem we should be addressing
>> more globally than just for one particular config item for
>> one particular target...


Ok. So. Let's assume I want to implement migration of "-smp" parameters.
What would be the correct way of doing this in terms of the current QOM
principles? Thanks.



> 
> This is lot bigger that the issue I am trying to solve by this patch...
> Still nack?





-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexander Graf]

On 08.04.14 03:26, Alexey Kardashevskiy wrote:
> On 03/28/2014 12:07 AM, Alexey Kardashevskiy wrote:
>> On 03/27/2014 11:57 PM, Peter Maydell wrote:
>>> On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>>>>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>>>>> This should prevent the destination guest from misbehaving when
>>>>>> the threads number is different in "-smp" command.
>>>>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>>>>> same on source and destination, so how can they differ?
>>>>
>>>> The idea is that "-smp" does not migrate and if we run source and
>>>> destination guests with different numbers in -smp, we end up with weird
>>>> machine
>>> Yes, so don't do that. As I understand it:
>>>   (1) if you don't run QEMU with the exact same command line
>>>       and config at both ends then migration won't work
>>>   (2) we don't guarantee to detect and cleanly fail if you
>>>       don't do (1)
>>>
>>> It would probably be nice if we did detect config mismatches,
>> Yep, we do not send the device tree (as libvirt does). Pure command line
>> matching won't work.
>>
>>> but that seems to me like a problem we should be addressing
>>> more globally than just for one particular config item for
>>> one particular target...
>
> Ok. So. Let's assume I want to implement migration of "-smp" parameters.
> What would be the correct way of doing this in terms of the current QOM
> principles? Thanks.

You don't. The migration protocol doesn't migrate configuration. If you 
want to start to transfer VM configuration (which I'd be all in for), do 
it properly and transfer _all_ configuration.


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

On 04/10/2014 10:10 PM, Alexander Graf wrote:
> 
> On 08.04.14 03:26, Alexey Kardashevskiy wrote:
>> On 03/28/2014 12:07 AM, Alexey Kardashevskiy wrote:
>>> On 03/27/2014 11:57 PM, Peter Maydell wrote:
>>>> On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>>>>>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>>>>>> This should prevent the destination guest from misbehaving when
>>>>>>> the threads number is different in "-smp" command.
>>>>>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>>>>>> same on source and destination, so how can they differ?
>>>>>
>>>>> The idea is that "-smp" does not migrate and if we run source and
>>>>> destination guests with different numbers in -smp, we end up with weird
>>>>> machine
>>>> Yes, so don't do that. As I understand it:
>>>>   (1) if you don't run QEMU with the exact same command line
>>>>       and config at both ends then migration won't work
>>>>   (2) we don't guarantee to detect and cleanly fail if you
>>>>       don't do (1)
>>>>
>>>> It would probably be nice if we did detect config mismatches,
>>> Yep, we do not send the device tree (as libvirt does). Pure command line
>>> matching won't work.
>>>
>>>> but that seems to me like a problem we should be addressing
>>>> more globally than just for one particular config item for
>>>> one particular target...
>>
>> Ok. So. Let's assume I want to implement migration of "-smp" parameters.
>> What would be the correct way of doing this in terms of the current QOM
>> principles? Thanks.
> 
> You don't. The migration protocol doesn't migrate configuration. If you
> want to start to transfer VM configuration (which I'd be all in for), do it
> properly and transfer _all_ configuration.


Then what is the purpose of many, many VMSTATE_.*_EQUAL?

And I do not want to send configuration by the proposed patch, I want to
make sure that the new guest is able to continue. Why exactly is this bad?


-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexander Graf]

On 10.04.2014, at 16:35, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 04/10/2014 10:10 PM, Alexander Graf wrote:
>> 
>> On 08.04.14 03:26, Alexey Kardashevskiy wrote:
>>> On 03/28/2014 12:07 AM, Alexey Kardashevskiy wrote:
>>>> On 03/27/2014 11:57 PM, Peter Maydell wrote:
>>>>> On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>>>>>>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>>>>>>> This should prevent the destination guest from misbehaving when
>>>>>>>> the threads number is different in "-smp" command.
>>>>>>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>>>>>>> same on source and destination, so how can they differ?
>>>>>> 
>>>>>> The idea is that "-smp" does not migrate and if we run source and
>>>>>> destination guests with different numbers in -smp, we end up with weird
>>>>>> machine
>>>>> Yes, so don't do that. As I understand it:
>>>>>  (1) if you don't run QEMU with the exact same command line
>>>>>      and config at both ends then migration won't work
>>>>>  (2) we don't guarantee to detect and cleanly fail if you
>>>>>      don't do (1)
>>>>> 
>>>>> It would probably be nice if we did detect config mismatches,
>>>> Yep, we do not send the device tree (as libvirt does). Pure command line
>>>> matching won't work.
>>>> 
>>>>> but that seems to me like a problem we should be addressing
>>>>> more globally than just for one particular config item for
>>>>> one particular target...
>>> 
>>> Ok. So. Let's assume I want to implement migration of "-smp" parameters.
>>> What would be the correct way of doing this in terms of the current QOM
>>> principles? Thanks.
>> 
>> You don't. The migration protocol doesn't migrate configuration. If you
>> want to start to transfer VM configuration (which I'd be all in for), do it
>> properly and transfer _all_ configuration.
> 
> 
> Then what is the purpose of many, many VMSTATE_.*_EQUAL?

Probably legacy from old vmstate layouts.

> And I do not want to send configuration by the proposed patch, I want to
> make sure that the new guest is able to continue. Why exactly is this bad?

It's not bad, but we should solve this properly, not one field at a time.


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

On 04/11/2014 12:41 AM, Alexander Graf wrote:
> 
> On 10.04.2014, at 16:35, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 04/10/2014 10:10 PM, Alexander Graf wrote:
>>>
>>> On 08.04.14 03:26, Alexey Kardashevskiy wrote:
>>>> On 03/28/2014 12:07 AM, Alexey Kardashevskiy wrote:
>>>>> On 03/27/2014 11:57 PM, Peter Maydell wrote:
>>>>>> On 27 March 2014 12:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>> On 03/27/2014 11:37 PM, Andreas Färber wrote:
>>>>>>>> Am 27.03.2014 03:41, schrieb Alexey Kardashevskiy:
>>>>>>>>> This should prevent the destination guest from misbehaving when
>>>>>>>>> the threads number is different in "-smp" command.
>>>>>>>> Sorry, I don't understand. When migrating, surely -smp needs to be the
>>>>>>>> same on source and destination, so how can they differ?
>>>>>>>
>>>>>>> The idea is that "-smp" does not migrate and if we run source and
>>>>>>> destination guests with different numbers in -smp, we end up with weird
>>>>>>> machine
>>>>>> Yes, so don't do that. As I understand it:
>>>>>>  (1) if you don't run QEMU with the exact same command line
>>>>>>      and config at both ends then migration won't work
>>>>>>  (2) we don't guarantee to detect and cleanly fail if you
>>>>>>      don't do (1)
>>>>>>
>>>>>> It would probably be nice if we did detect config mismatches,
>>>>> Yep, we do not send the device tree (as libvirt does). Pure command line
>>>>> matching won't work.
>>>>>
>>>>>> but that seems to me like a problem we should be addressing
>>>>>> more globally than just for one particular config item for
>>>>>> one particular target...
>>>>
>>>> Ok. So. Let's assume I want to implement migration of "-smp" parameters.
>>>> What would be the correct way of doing this in terms of the current QOM
>>>> principles? Thanks.
>>>
>>> You don't. The migration protocol doesn't migrate configuration. If you
>>> want to start to transfer VM configuration (which I'd be all in for), do it
>>> properly and transfer _all_ configuration.
>>
>>
>> Then what is the purpose of many, many VMSTATE_.*_EQUAL?
> 
> Probably legacy from old vmstate layouts.


So this should not be used from now on?


>> And I do not want to send configuration by the proposed patch, I want to
>> make sure that the new guest is able to continue. Why exactly is this bad?
> 
> It's not bad, but we should solve this properly, not one field at a time.





-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Peter Maydell]

On 10 April 2014 15:35, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> Then what is the purpose of many, many VMSTATE_.*_EQUAL?

Often it's backwards compatibility with a previous vmstate
or save/load function set which incorrectly sent data it didn't
need to.

> And I do not want to send configuration by the proposed patch, I want to
> make sure that the new guest is able to continue. Why exactly is this bad?

It's not bad, but as several people have now pointed out to you,
you're trying to fix a tiny tiny corner of the real, larger
problem, in a way which isn't going to generalise to actually
fixing the larger problem. So if we took your change then
(a) we still wouldn't be able to support detection of migration
between two systems with mismatched configuration, so it doesn't
really achieve anything
(b) if we ever did manage to fix that we'd have to remove your
change (because that bit of config checking would now be handled
via whatever generic mechanism we implemented), except we probably
couldn't remove it since that would break migration version
compatibility, so we'd end up with a wart we have to carry
around forever

thanks
-- PMM

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Add @cpu_dt_id into migration stream

[Alexey Kardashevskiy]

On 04/11/2014 12:49 AM, Peter Maydell wrote:
> On 10 April 2014 15:35, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> Then what is the purpose of many, many VMSTATE_.*_EQUAL?
> 
> Often it's backwards compatibility with a previous vmstate
> or save/load function set which incorrectly sent data it didn't
> need to.
> 
>> And I do not want to send configuration by the proposed patch, I want to
>> make sure that the new guest is able to continue. Why exactly is this bad?
> 
> It's not bad, but as several people have now pointed out to you,
> you're trying to fix a tiny tiny corner of the real, larger
> problem, in a way which isn't going to generalise to actually
> fixing the larger problem. So if we took your change then
> (a) we still wouldn't be able to support detection of migration
> between two systems with mismatched configuration, so it doesn't
> really achieve anything
> (b) if we ever did manage to fix that we'd have to remove your
> change (because that bit of config checking would now be handled
> via whatever generic mechanism we implemented), except we probably
> couldn't remove it since that would break migration version
> compatibility, so we'd end up with a wart we have to carry
> around forever

Ok, understood. Thanks.


ps. yeah, I do not often see the bigger picture, I know :(
-- 
Alexey