* [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures @ 2024-04-30 1:23 Song Gao 2024-04-30 8:53 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 8+ messages in thread From: Song Gao @ 2024-04-30 1:23 UTC (permalink / raw) To: qemu-devel Cc: pbonzini, peter.maydell, richard.henderson, philmd, maobibo, zhaotianrui, lixianglai vmstate does not save kvm_state_conter, which can cause VM recovery from disk to fail. Signed-off-by: Song Gao <gaosong@loongson.cn> --- target/loongarch/machine.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c index c7029fb9b4..4cd1bf06ff 100644 --- a/target/loongarch/machine.c +++ b/target/loongarch/machine.c @@ -191,6 +191,8 @@ const VMStateDescription vmstate_loongarch_cpu = { VMSTATE_STRUCT_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX, 0, vmstate_tlb, LoongArchTLB), + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), + VMSTATE_END_OF_LIST() }, .subsections = (const VMStateDescription * const []) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-04-30 1:23 [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures Song Gao @ 2024-04-30 8:53 ` Philippe Mathieu-Daudé 2024-04-30 14:00 ` Fabiano Rosas 0 siblings, 1 reply; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2024-04-30 8:53 UTC (permalink / raw) To: Song Gao, qemu-devel, Tianrui Zhao Cc: pbonzini, peter.maydell, richard.henderson, maobibo, lixianglai, Fabiano Rosas, Peter Xu (Cc'ing migration maintainers) On 30/4/24 03:23, Song Gao wrote: > vmstate does not save kvm_state_conter, > which can cause VM recovery from disk to fail. Cc: qemu-stable@nongnu.org Fixes: d11681c94f ("target/loongarch: Implement kvm_arch_init_vcpu") > Signed-off-by: Song Gao <gaosong@loongson.cn> > --- > target/loongarch/machine.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c > index c7029fb9b4..4cd1bf06ff 100644 > --- a/target/loongarch/machine.c > +++ b/target/loongarch/machine.c > @@ -191,6 +191,8 @@ const VMStateDescription vmstate_loongarch_cpu = { > VMSTATE_STRUCT_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX, > 0, vmstate_tlb, LoongArchTLB), > > + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), > + > VMSTATE_END_OF_LIST() > }, > .subsections = (const VMStateDescription * const []) { The migration stream is versioned, so you should increase it, but this field is only relevant for KVM (it shouldn't be there in non-KVM builds). IMHO the correct migration way to fix that is (untested): -- >8 -- diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c index c7029fb9b4..08032c6d71 100644 --- a/target/loongarch/machine.c +++ b/target/loongarch/machine.c @@ -8,8 +8,27 @@ #include "qemu/osdep.h" #include "cpu.h" #include "migration/cpu.h" +#include "sysemu/kvm.h" #include "vec.h" +#ifdef CONFIG_KVM +static bool kvmcpu_needed(void *opaque) +{ + return kvm_enabled(); +} + +static const VMStateDescription vmstate_kvmtimer = { + .name = "cpu/kvmtimer", + .version_id = 1, + .minimum_version_id = 1, + .needed = kvmcpu_needed, + .fields = (const VMStateField[]) { + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), + VMSTATE_END_OF_LIST() + } +}; +#endif /* CONFIG_KVM */ + static const VMStateDescription vmstate_fpu_reg = { .name = "fpu_reg", .version_id = 1, @@ -194,6 +213,9 @@ const VMStateDescription vmstate_loongarch_cpu = { VMSTATE_END_OF_LIST() }, .subsections = (const VMStateDescription * const []) { +#ifdef CONFIG_KVM + &vmstate_kvmcpu, +#endif &vmstate_fpu, &vmstate_lsx, &vmstate_lasx, --- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-04-30 8:53 ` Philippe Mathieu-Daudé @ 2024-04-30 14:00 ` Fabiano Rosas 2024-05-01 15:45 ` Peter Xu 0 siblings, 1 reply; 8+ messages in thread From: Fabiano Rosas @ 2024-04-30 14:00 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Song Gao, qemu-devel, Tianrui Zhao Cc: pbonzini, peter.maydell, richard.henderson, maobibo, lixianglai, Peter Xu Philippe Mathieu-Daudé <philmd@linaro.org> writes: > (Cc'ing migration maintainers) > > On 30/4/24 03:23, Song Gao wrote: >> vmstate does not save kvm_state_conter, >> which can cause VM recovery from disk to fail. > > Cc: qemu-stable@nongnu.org > Fixes: d11681c94f ("target/loongarch: Implement kvm_arch_init_vcpu") > >> Signed-off-by: Song Gao <gaosong@loongson.cn> >> --- >> target/loongarch/machine.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c >> index c7029fb9b4..4cd1bf06ff 100644 >> --- a/target/loongarch/machine.c >> +++ b/target/loongarch/machine.c >> @@ -191,6 +191,8 @@ const VMStateDescription vmstate_loongarch_cpu = { >> VMSTATE_STRUCT_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX, >> 0, vmstate_tlb, LoongArchTLB), >> >> + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), >> + >> VMSTATE_END_OF_LIST() >> }, >> .subsections = (const VMStateDescription * const []) { > > The migration stream is versioned, so you should increase it, > but this field is only relevant for KVM (it shouldn't be there > in non-KVM builds). IMHO the correct migration way to fix that > is (untested): > > -- >8 -- > diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c > index c7029fb9b4..08032c6d71 100644 > --- a/target/loongarch/machine.c > +++ b/target/loongarch/machine.c > @@ -8,8 +8,27 @@ > #include "qemu/osdep.h" > #include "cpu.h" > #include "migration/cpu.h" > +#include "sysemu/kvm.h" > #include "vec.h" > > +#ifdef CONFIG_KVM > +static bool kvmcpu_needed(void *opaque) > +{ > + return kvm_enabled(); > +} > + > +static const VMStateDescription vmstate_kvmtimer = { > + .name = "cpu/kvmtimer", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = kvmcpu_needed, > + .fields = (const VMStateField[]) { > + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), > + VMSTATE_END_OF_LIST() > + } > +}; > +#endif /* CONFIG_KVM */ > + > static const VMStateDescription vmstate_fpu_reg = { > .name = "fpu_reg", > .version_id = 1, > @@ -194,6 +213,9 @@ const VMStateDescription vmstate_loongarch_cpu = { > VMSTATE_END_OF_LIST() > }, > .subsections = (const VMStateDescription * const []) { > +#ifdef CONFIG_KVM > + &vmstate_kvmcpu, > +#endif > &vmstate_fpu, > &vmstate_lsx, > &vmstate_lasx, > --- LGTM, I'd just leave only the .needed function under CONFIG_KVM instead of the whole subsection. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-04-30 14:00 ` Fabiano Rosas @ 2024-05-01 15:45 ` Peter Xu 2024-05-02 12:45 ` Fabiano Rosas 0 siblings, 1 reply; 8+ messages in thread From: Peter Xu @ 2024-05-01 15:45 UTC (permalink / raw) To: Fabiano Rosas Cc: Philippe Mathieu-Daudé, Song Gao, qemu-devel, Tianrui Zhao, pbonzini, peter.maydell, richard.henderson, maobibo, lixianglai On Tue, Apr 30, 2024 at 11:00:24AM -0300, Fabiano Rosas wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > > > (Cc'ing migration maintainers) > > > > On 30/4/24 03:23, Song Gao wrote: > >> vmstate does not save kvm_state_conter, > >> which can cause VM recovery from disk to fail. > > > > Cc: qemu-stable@nongnu.org > > Fixes: d11681c94f ("target/loongarch: Implement kvm_arch_init_vcpu") > > > >> Signed-off-by: Song Gao <gaosong@loongson.cn> > >> --- > >> target/loongarch/machine.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c > >> index c7029fb9b4..4cd1bf06ff 100644 > >> --- a/target/loongarch/machine.c > >> +++ b/target/loongarch/machine.c > >> @@ -191,6 +191,8 @@ const VMStateDescription vmstate_loongarch_cpu = { > >> VMSTATE_STRUCT_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX, > >> 0, vmstate_tlb, LoongArchTLB), > >> > >> + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), > >> + > >> VMSTATE_END_OF_LIST() > >> }, > >> .subsections = (const VMStateDescription * const []) { > > > > The migration stream is versioned, so you should increase it, > > but this field is only relevant for KVM (it shouldn't be there > > in non-KVM builds). IMHO the correct migration way to fix that > > is (untested): > > > > -- >8 -- > > diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c > > index c7029fb9b4..08032c6d71 100644 > > --- a/target/loongarch/machine.c > > +++ b/target/loongarch/machine.c > > @@ -8,8 +8,27 @@ > > #include "qemu/osdep.h" > > #include "cpu.h" > > #include "migration/cpu.h" > > +#include "sysemu/kvm.h" > > #include "vec.h" > > > > +#ifdef CONFIG_KVM > > +static bool kvmcpu_needed(void *opaque) > > +{ > > + return kvm_enabled(); > > +} > > + > > +static const VMStateDescription vmstate_kvmtimer = { > > + .name = "cpu/kvmtimer", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .needed = kvmcpu_needed, > > + .fields = (const VMStateField[]) { > > + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > +#endif /* CONFIG_KVM */ > > + > > static const VMStateDescription vmstate_fpu_reg = { > > .name = "fpu_reg", > > .version_id = 1, > > @@ -194,6 +213,9 @@ const VMStateDescription vmstate_loongarch_cpu = { > > VMSTATE_END_OF_LIST() > > }, > > .subsections = (const VMStateDescription * const []) { > > +#ifdef CONFIG_KVM > > + &vmstate_kvmcpu, > > +#endif > > &vmstate_fpu, > > &vmstate_lsx, > > &vmstate_lasx, > > --- > > LGTM, I'd just leave only the .needed function under CONFIG_KVM instead > of the whole subsection. But when !KVM it means there's no ".needed" and it'll still be migrated? IMHO it depends on whether loongarch is in the state already of trying to keep its ABI at all. I think we should still try to enjoy the time when that ABI is not required, then we can simply add whatever fields, and let things break with no big deal. Note that if with CONFIG_KVM it means it can break migration between kvm v.s. tcg when only one qemu enabled kvm when compile. Considering the patch is from the maintainer (which seems to say "breaking that is all fine!") I'd say the original patch looks good actually, as it allows kvm / tcg migrations too as a baseline. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-05-01 15:45 ` Peter Xu @ 2024-05-02 12:45 ` Fabiano Rosas 2024-05-07 8:12 ` gaosong 0 siblings, 1 reply; 8+ messages in thread From: Fabiano Rosas @ 2024-05-02 12:45 UTC (permalink / raw) To: Peter Xu Cc: Philippe Mathieu-Daudé, Song Gao, qemu-devel, Tianrui Zhao, pbonzini, peter.maydell, richard.henderson, maobibo, lixianglai Peter Xu <peterx@redhat.com> writes: > On Tue, Apr 30, 2024 at 11:00:24AM -0300, Fabiano Rosas wrote: >> Philippe Mathieu-Daudé <philmd@linaro.org> writes: >> >> > (Cc'ing migration maintainers) >> > >> > On 30/4/24 03:23, Song Gao wrote: >> >> vmstate does not save kvm_state_conter, >> >> which can cause VM recovery from disk to fail. >> > >> > Cc: qemu-stable@nongnu.org >> > Fixes: d11681c94f ("target/loongarch: Implement kvm_arch_init_vcpu") >> > >> >> Signed-off-by: Song Gao <gaosong@loongson.cn> >> >> --- >> >> target/loongarch/machine.c | 2 ++ >> >> 1 file changed, 2 insertions(+) >> >> >> >> diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c >> >> index c7029fb9b4..4cd1bf06ff 100644 >> >> --- a/target/loongarch/machine.c >> >> +++ b/target/loongarch/machine.c >> >> @@ -191,6 +191,8 @@ const VMStateDescription vmstate_loongarch_cpu = { >> >> VMSTATE_STRUCT_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX, >> >> 0, vmstate_tlb, LoongArchTLB), >> >> >> >> + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), >> >> + >> >> VMSTATE_END_OF_LIST() >> >> }, >> >> .subsections = (const VMStateDescription * const []) { >> > >> > The migration stream is versioned, so you should increase it, >> > but this field is only relevant for KVM (it shouldn't be there >> > in non-KVM builds). IMHO the correct migration way to fix that >> > is (untested): >> > >> > -- >8 -- >> > diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c >> > index c7029fb9b4..08032c6d71 100644 >> > --- a/target/loongarch/machine.c >> > +++ b/target/loongarch/machine.c >> > @@ -8,8 +8,27 @@ >> > #include "qemu/osdep.h" >> > #include "cpu.h" >> > #include "migration/cpu.h" >> > +#include "sysemu/kvm.h" >> > #include "vec.h" >> > >> > +#ifdef CONFIG_KVM >> > +static bool kvmcpu_needed(void *opaque) >> > +{ >> > + return kvm_enabled(); >> > +} >> > + >> > +static const VMStateDescription vmstate_kvmtimer = { >> > + .name = "cpu/kvmtimer", >> > + .version_id = 1, >> > + .minimum_version_id = 1, >> > + .needed = kvmcpu_needed, >> > + .fields = (const VMStateField[]) { >> > + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), >> > + VMSTATE_END_OF_LIST() >> > + } >> > +}; >> > +#endif /* CONFIG_KVM */ >> > + >> > static const VMStateDescription vmstate_fpu_reg = { >> > .name = "fpu_reg", >> > .version_id = 1, >> > @@ -194,6 +213,9 @@ const VMStateDescription vmstate_loongarch_cpu = { >> > VMSTATE_END_OF_LIST() >> > }, >> > .subsections = (const VMStateDescription * const []) { >> > +#ifdef CONFIG_KVM >> > + &vmstate_kvmcpu, >> > +#endif >> > &vmstate_fpu, >> > &vmstate_lsx, >> > &vmstate_lasx, >> > --- >> >> LGTM, I'd just leave only the .needed function under CONFIG_KVM instead >> of the whole subsection. > > But when !KVM it means there's no ".needed" and it'll still be migrated? I expressed myself poorly, I meant put the return from .needed under CONFIG_KVM. But that is not even necessary, kvm_enabled() is enough. > > IMHO it depends on whether loongarch is in the state already of trying to > keep its ABI at all. I think we should still try to enjoy the time when > that ABI is not required, then we can simply add whatever fields, and let > things break with no big deal. > > Note that if with CONFIG_KVM it means it can break migration between kvm > v.s. tcg when only one qemu enabled kvm when compile. Considering the > patch is from the maintainer (which seems to say "breaking that is all > fine!") I'd say the original patch looks good actually, as it allows kvm / > tcg migrations too as a baseline. I'm fine with this approach as well. > > Thanks, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-05-02 12:45 ` Fabiano Rosas @ 2024-05-07 8:12 ` gaosong 2024-05-07 15:47 ` Peter Xu 0 siblings, 1 reply; 8+ messages in thread From: gaosong @ 2024-05-07 8:12 UTC (permalink / raw) To: Fabiano Rosas, Peter Xu Cc: Philippe Mathieu-Daudé, qemu-devel, Tianrui Zhao, pbonzini, peter.maydell, richard.henderson, maobibo, lixianglai Thanks for the comments ! 在 2024/5/2 下午8:45, Fabiano Rosas 写道: > Peter Xu <peterx@redhat.com> writes: > >> On Tue, Apr 30, 2024 at 11:00:24AM -0300, Fabiano Rosas wrote: >>> Philippe Mathieu-Daudé <philmd@linaro.org> writes: >>> >>>> (Cc'ing migration maintainers) >>>> >>>> On 30/4/24 03:23, Song Gao wrote: >>>>> vmstate does not save kvm_state_conter, >>>>> which can cause VM recovery from disk to fail. >>>> Cc: qemu-stable@nongnu.org >>>> Fixes: d11681c94f ("target/loongarch: Implement kvm_arch_init_vcpu") >>>> >>>>> Signed-off-by: Song Gao <gaosong@loongson.cn> >>>>> --- >>>>> target/loongarch/machine.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c >>>>> index c7029fb9b4..4cd1bf06ff 100644 >>>>> --- a/target/loongarch/machine.c >>>>> +++ b/target/loongarch/machine.c >>>>> @@ -191,6 +191,8 @@ const VMStateDescription vmstate_loongarch_cpu = { >>>>> VMSTATE_STRUCT_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX, >>>>> 0, vmstate_tlb, LoongArchTLB), >>>>> >>>>> + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), >>>>> + >>>>> VMSTATE_END_OF_LIST() >>>>> }, >>>>> .subsections = (const VMStateDescription * const []) { >>>> The migration stream is versioned, so you should increase it, >>>> but this field is only relevant for KVM (it shouldn't be there >>>> in non-KVM builds). IMHO the correct migration way to fix that >>>> is (untested): >>>> >>>> -- >8 -- >>>> diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c >>>> index c7029fb9b4..08032c6d71 100644 >>>> --- a/target/loongarch/machine.c >>>> +++ b/target/loongarch/machine.c >>>> @@ -8,8 +8,27 @@ >>>> #include "qemu/osdep.h" >>>> #include "cpu.h" >>>> #include "migration/cpu.h" >>>> +#include "sysemu/kvm.h" >>>> #include "vec.h" >>>> >>>> +#ifdef CONFIG_KVM >>>> +static bool kvmcpu_needed(void *opaque) >>>> +{ >>>> + return kvm_enabled(); >>>> +} >>>> + >>>> +static const VMStateDescription vmstate_kvmtimer = { >>>> + .name = "cpu/kvmtimer", >>>> + .version_id = 1, >>>> + .minimum_version_id = 1, >>>> + .needed = kvmcpu_needed, >>>> + .fields = (const VMStateField[]) { >>>> + VMSTATE_UINT64(kvm_state_counter, LoongArchCPU), >>>> + VMSTATE_END_OF_LIST() >>>> + } >>>> +}; >>>> +#endif /* CONFIG_KVM */ >>>> + >>>> static const VMStateDescription vmstate_fpu_reg = { >>>> .name = "fpu_reg", >>>> .version_id = 1, >>>> @@ -194,6 +213,9 @@ const VMStateDescription vmstate_loongarch_cpu = { >>>> VMSTATE_END_OF_LIST() >>>> }, >>>> .subsections = (const VMStateDescription * const []) { >>>> +#ifdef CONFIG_KVM >>>> + &vmstate_kvmcpu, >>>> +#endif >>>> &vmstate_fpu, >>>> &vmstate_lsx, >>>> &vmstate_lasx, >>>> --- >>> LGTM, I'd just leave only the .needed function under CONFIG_KVM instead >>> of the whole subsection. >> But when !KVM it means there's no ".needed" and it'll still be migrated? > I expressed myself poorly, I meant put the return from .needed under > CONFIG_KVM. But that is not even necessary, kvm_enabled() is enough. > >> IMHO it depends on whether loongarch is in the state already of trying to >> keep its ABI at all. I think we should still try to enjoy the time when >> that ABI is not required, then we can simply add whatever fields, and let >> things break with no big deal. >> >> Note that if with CONFIG_KVM it means it can break migration between kvm >> v.s. tcg when only one qemu enabled kvm when compile. Just remove CONIFG_KVM would be OK? Thanks. Song Gao >> Considering the >> patch is from the maintainer (which seems to say "breaking that is all >> fine!") I'd say the original patch looks good actually, as it allows kvm / >> tcg migrations too as a baseline. > I'm fine with this approach as well. > >> Thanks, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-05-07 8:12 ` gaosong @ 2024-05-07 15:47 ` Peter Xu 2024-05-07 15:52 ` Peter Maydell 0 siblings, 1 reply; 8+ messages in thread From: Peter Xu @ 2024-05-07 15:47 UTC (permalink / raw) To: gaosong Cc: Fabiano Rosas, Philippe Mathieu-Daudé, qemu-devel, Tianrui Zhao, pbonzini, peter.maydell, richard.henderson, maobibo, lixianglai On Tue, May 07, 2024 at 04:12:34PM +0800, gaosong wrote: > Just remove CONIFG_KVM would be OK? You're the loongarch maintainer so I'd say your call. :) If you're not yet sure, IMHO we should go with the simplest, which is the original oneliner patch. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures 2024-05-07 15:47 ` Peter Xu @ 2024-05-07 15:52 ` Peter Maydell 0 siblings, 0 replies; 8+ messages in thread From: Peter Maydell @ 2024-05-07 15:52 UTC (permalink / raw) To: Peter Xu Cc: gaosong, Fabiano Rosas, Philippe Mathieu-Daudé, qemu-devel, Tianrui Zhao, pbonzini, richard.henderson, maobibo, lixianglai On Tue, 7 May 2024 at 16:47, Peter Xu <peterx@redhat.com> wrote: > > On Tue, May 07, 2024 at 04:12:34PM +0800, gaosong wrote: > > Just remove CONIFG_KVM would be OK? > > You're the loongarch maintainer so I'd say your call. :) > > If you're not yet sure, IMHO we should go with the simplest, which is the > original oneliner patch. The original patch needs to also bump the version numbers when it adds the new field. Even when we do not wish to maintain migration compatibility, bumping the version number means that users get a (more or less) helpful error message if they try an unsupported cross-version migration, rather than weird behaviour. thanks -- PMM ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-07 15:53 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-30 1:23 [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures Song Gao 2024-04-30 8:53 ` Philippe Mathieu-Daudé 2024-04-30 14:00 ` Fabiano Rosas 2024-05-01 15:45 ` Peter Xu 2024-05-02 12:45 ` Fabiano Rosas 2024-05-07 8:12 ` gaosong 2024-05-07 15:47 ` Peter Xu 2024-05-07 15:52 ` Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).