hw/acpi/generic_event_device.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
which will lead to infinite recursion in dump_vmstate_vmsd.
Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
---
hw/acpi/generic_event_device.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 6df400e1ee16..4b6867300a55 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
.minimum_version_id = 1,
.needed = ghes_needed,
.fields = (VMStateField[]) {
- VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
- vmstate_ghes_state, AcpiGhesState),
+ VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
VMSTATE_END_OF_LIST()
}
};
--
2.26.2
On Mon, 19 Oct 2020 17:31:56 +0800
Peng Liang <liangpeng10@huawei.com> wrote:
> There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
> which will lead to infinite recursion in dump_vmstate_vmsd.
>
> Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> ---
> hw/acpi/generic_event_device.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 6df400e1ee16..4b6867300a55 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
> .minimum_version_id = 1,
> .needed = ghes_needed,
> .fields = (VMStateField[]) {
> - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> - vmstate_ghes_state, AcpiGhesState),
> + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
not sure its' ok handle it this way,
see how it is done with another structure:
static const VMStateDescription vmstate_ged_state = {
.name = "acpi-ged-state",
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
VMSTATE_UINT32(sel, GEDState),
VMSTATE_END_OF_LIST()
}
};
...
VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
i.e. it looks like we are missing structure definition for AcpiGhesState
CCing David,
to help with migration magic in case I'm wrong or missed something
> VMSTATE_END_OF_LIST()
> }
> };
* Igor Mammedov (imammedo@redhat.com) wrote:
> On Mon, 19 Oct 2020 17:31:56 +0800
> Peng Liang <liangpeng10@huawei.com> wrote:
>
> > There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
> > which will lead to infinite recursion in dump_vmstate_vmsd.
> >
> > Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
> > Reported-by: Euler Robot <euler.robot@huawei.com>
> > Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> > ---
> > hw/acpi/generic_event_device.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > index 6df400e1ee16..4b6867300a55 100644
> > --- a/hw/acpi/generic_event_device.c
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
> > .minimum_version_id = 1,
> > .needed = ghes_needed,
> > .fields = (VMStateField[]) {
> > - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > - vmstate_ghes_state, AcpiGhesState),
> > + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
>
> not sure its' ok handle it this way,
>
> see how it is done with another structure:
>
> static const VMStateDescription vmstate_ged_state = {
> .name = "acpi-ged-state",
> .version_id = 1,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> VMSTATE_UINT32(sel, GEDState),
> VMSTATE_END_OF_LIST()
> }
> };
>
> ...
>
> VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
>
> i.e. it looks like we are missing structure definition for AcpiGhesState
>
> CCing David,
> to help with migration magic in case I'm wrong or missed something
Yeh that's confusing :-)
Given a:
VMSTATE_STRUCT(a, B, 1, vmstate_c, C)
We're saying there's a field 'a' in type B, and field 'a'
should be of type C and be serialised using vmstate_c.
That also means that in any vmstate_c, we're expecting it
to be passed a type C generally.
Having said that; you don't need a struct - you can get away
with that VMSTATE_UINT64, there's two problems:
a) That assumes that your ghes always stays that simple.
b) If you wanted to store a Ghes from a number of different
parent structures then you're stuck because your vmstate_ghes_state
is bound to being a strict field of AcpiGedState.
So yes, it's neatest to do it using a VMSD for AcpiGhesState
And congratulations on finding a loop; I don't think we've ever had one
before :-)
Dave
> > VMSTATE_END_OF_LIST()
> > }
> > };
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, 23 Oct 2020 19:54:41 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Igor Mammedov (imammedo@redhat.com) wrote:
> > On Mon, 19 Oct 2020 17:31:56 +0800
> > Peng Liang <liangpeng10@huawei.com> wrote:
> >
> > > There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
> > > which will lead to infinite recursion in dump_vmstate_vmsd.
> > >
> > > Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
> > > Reported-by: Euler Robot <euler.robot@huawei.com>
> > > Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> > > ---
> > > hw/acpi/generic_event_device.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > > index 6df400e1ee16..4b6867300a55 100644
> > > --- a/hw/acpi/generic_event_device.c
> > > +++ b/hw/acpi/generic_event_device.c
> > > @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
> > > .minimum_version_id = 1,
> > > .needed = ghes_needed,
> > > .fields = (VMStateField[]) {
> > > - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > > - vmstate_ghes_state, AcpiGhesState),
> > > + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
> >
> > not sure its' ok handle it this way,
> >
> > see how it is done with another structure:
> >
> > static const VMStateDescription vmstate_ged_state = {
> > .name = "acpi-ged-state",
> > .version_id = 1,
> > .minimum_version_id = 1,
> > .fields = (VMStateField[]) {
> > VMSTATE_UINT32(sel, GEDState),
> > VMSTATE_END_OF_LIST()
> > }
> > };
> >
> > ...
> >
> > VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
> >
> > i.e. it looks like we are missing structure definition for AcpiGhesState
> >
> > CCing David,
> > to help with migration magic in case I'm wrong or missed something
>
> Yeh that's confusing :-)
>
> Given a:
>
> VMSTATE_STRUCT(a, B, 1, vmstate_c, C)
>
> We're saying there's a field 'a' in type B, and field 'a'
> should be of type C and be serialised using vmstate_c.
>
> That also means that in any vmstate_c, we're expecting it
> to be passed a type C generally.
>
> Having said that; you don't need a struct - you can get away
> with that VMSTATE_UINT64, there's two problems:
>
> a) That assumes that your ghes always stays that simple.
> b) If you wanted to store a Ghes from a number of different
> parent structures then you're stuck because your vmstate_ghes_state
> is bound to being a strict field of AcpiGedState.
>
> So yes, it's neatest to do it using a VMSD for AcpiGhesState
>
> And congratulations on finding a loop; I don't think we've ever had one
> before :-)
can we make compilation fail in case VMSTATE_STRUCT is used but
is not actually provided like it was in this case?
>
> Dave
>
> > > VMSTATE_END_OF_LIST()
> > > }
> > > };
> >
* Igor Mammedov (imammedo@redhat.com) wrote:
> On Fri, 23 Oct 2020 19:54:41 +0100
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
>
> > * Igor Mammedov (imammedo@redhat.com) wrote:
> > > On Mon, 19 Oct 2020 17:31:56 +0800
> > > Peng Liang <liangpeng10@huawei.com> wrote:
> > >
> > > > There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
> > > > which will lead to infinite recursion in dump_vmstate_vmsd.
> > > >
> > > > Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
> > > > Reported-by: Euler Robot <euler.robot@huawei.com>
> > > > Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> > > > ---
> > > > hw/acpi/generic_event_device.c | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > > > index 6df400e1ee16..4b6867300a55 100644
> > > > --- a/hw/acpi/generic_event_device.c
> > > > +++ b/hw/acpi/generic_event_device.c
> > > > @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
> > > > .minimum_version_id = 1,
> > > > .needed = ghes_needed,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > > > - vmstate_ghes_state, AcpiGhesState),
> > > > + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
> > >
> > > not sure its' ok handle it this way,
> > >
> > > see how it is done with another structure:
> > >
> > > static const VMStateDescription vmstate_ged_state = {
> > > .name = "acpi-ged-state",
> > > .version_id = 1,
> > > .minimum_version_id = 1,
> > > .fields = (VMStateField[]) {
> > > VMSTATE_UINT32(sel, GEDState),
> > > VMSTATE_END_OF_LIST()
> > > }
> > > };
> > >
> > > ...
> > >
> > > VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
> > >
> > > i.e. it looks like we are missing structure definition for AcpiGhesState
> > >
> > > CCing David,
> > > to help with migration magic in case I'm wrong or missed something
> >
> > Yeh that's confusing :-)
> >
> > Given a:
> >
> > VMSTATE_STRUCT(a, B, 1, vmstate_c, C)
> >
> > We're saying there's a field 'a' in type B, and field 'a'
> > should be of type C and be serialised using vmstate_c.
> >
> > That also means that in any vmstate_c, we're expecting it
> > to be passed a type C generally.
> >
> > Having said that; you don't need a struct - you can get away
> > with that VMSTATE_UINT64, there's two problems:
> >
> > a) That assumes that your ghes always stays that simple.
> > b) If you wanted to store a Ghes from a number of different
> > parent structures then you're stuck because your vmstate_ghes_state
> > is bound to being a strict field of AcpiGedState.
> >
> > So yes, it's neatest to do it using a VMSD for AcpiGhesState
> >
> > And congratulations on finding a loop; I don't think we've ever had one
> > before :-)
>
> can we make compilation fail in case VMSTATE_STRUCT is used but
> is not actually provided like it was in this case?
There's some type checking by the vmstate_offset_value macro; which I
think would have verified that the 'ghes_state' field in AcpiGedState
really is of type AcpiGhesState; but what we don't have is a type
associated with a given VMStateDescription to know what type of state
it's supposed to be called on.
Dave
> >
> > Dave
> >
> > > > VMSTATE_END_OF_LIST()
> > > > }
> > > > };
> > >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 10/24/2020 2:54 AM, Dr. David Alan Gilbert wrote:
> * Igor Mammedov (imammedo@redhat.com) wrote:
>> On Mon, 19 Oct 2020 17:31:56 +0800
>> Peng Liang <liangpeng10@huawei.com> wrote:
>>
>>> There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
>>> which will lead to infinite recursion in dump_vmstate_vmsd.
>>>
>>> Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
>>> ---
>>> hw/acpi/generic_event_device.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>>> index 6df400e1ee16..4b6867300a55 100644
>>> --- a/hw/acpi/generic_event_device.c
>>> +++ b/hw/acpi/generic_event_device.c
>>> @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
>>> .minimum_version_id = 1,
>>> .needed = ghes_needed,
>>> .fields = (VMStateField[]) {
>>> - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
>>> - vmstate_ghes_state, AcpiGhesState),
>>> + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
>>
>> not sure its' ok handle it this way,
>>
>> see how it is done with another structure:
>>
>> static const VMStateDescription vmstate_ged_state = {
>> .name = "acpi-ged-state",
>> .version_id = 1,
>> .minimum_version_id = 1,
>> .fields = (VMStateField[]) {
>> VMSTATE_UINT32(sel, GEDState),
>> VMSTATE_END_OF_LIST()
>> }
>> };
>>
>> ...
>>
>> VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
>>
>> i.e. it looks like we are missing structure definition for AcpiGhesState
>>
>> CCing David,
>> to help with migration magic in case I'm wrong or missed something
>
> Yeh that's confusing :-)
>
> Given a:
>
> VMSTATE_STRUCT(a, B, 1, vmstate_c, C)
>
> We're saying there's a field 'a' in type B, and field 'a'
> should be of type C and be serialised using vmstate_c.
>
> That also means that in any vmstate_c, we're expecting it
> to be passed a type C generally.
>
> Having said that; you don't need a struct - you can get away
> with that VMSTATE_UINT64, there's two problems:
>
> a) That assumes that your ghes always stays that simple.
> b) If you wanted to store a Ghes from a number of different
> parent structures then you're stuck because your vmstate_ghes_state
> is bound to being a strict field of AcpiGedState.
>
> So yes, it's neatest to do it using a VMSD for AcpiGhesState
>
> And congratulations on finding a loop; I don't think we've ever had one
> before :-)
>
> Dave
>
>>> VMSTATE_END_OF_LIST()
>>> }
>>> };
>>
Do you mean that we need another VMStateDescription to describe
AcpiGhesState instead of using VMSTATE_UINT64 directly? Maybe like this:
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 6df400e1ee16..5454be67d5f0 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -322,6 +322,16 @@ static const VMStateDescription vmstate_ged_state = {
}
};
+static const VMStateDescription vmstate_ghes = {
+ .name = "acpi-ghes",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
static bool ghes_needed(void *opaque)
{
AcpiGedState *s = opaque;
@@ -335,7 +345,7 @@ static const VMStateDescription vmstate_ghes_state = {
.needed = ghes_needed,
.fields = (VMStateField[]) {
VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
- vmstate_ghes_state, AcpiGhesState),
+ vmstate_ghes, AcpiGhesState),
VMSTATE_END_OF_LIST()
}
};
--
Thanks,
Peng
Is someone taking a fix for this in 5.2 - it's breaking vmstate
comparison.
Dave
* Peng Liang (liangpeng10@huawei.com) wrote:
> On 10/24/2020 2:54 AM, Dr. David Alan Gilbert wrote:
> > * Igor Mammedov (imammedo@redhat.com) wrote:
> >> On Mon, 19 Oct 2020 17:31:56 +0800
> >> Peng Liang <liangpeng10@huawei.com> wrote:
> >>
> >>> There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
> >>> which will lead to infinite recursion in dump_vmstate_vmsd.
> >>>
> >>> Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
> >>> Reported-by: Euler Robot <euler.robot@huawei.com>
> >>> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> >>> ---
> >>> hw/acpi/generic_event_device.c | 3 +--
> >>> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> >>> index 6df400e1ee16..4b6867300a55 100644
> >>> --- a/hw/acpi/generic_event_device.c
> >>> +++ b/hw/acpi/generic_event_device.c
> >>> @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
> >>> .minimum_version_id = 1,
> >>> .needed = ghes_needed,
> >>> .fields = (VMStateField[]) {
> >>> - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> >>> - vmstate_ghes_state, AcpiGhesState),
> >>> + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
> >>
> >> not sure its' ok handle it this way,
> >>
> >> see how it is done with another structure:
> >>
> >> static const VMStateDescription vmstate_ged_state = {
> >> .name = "acpi-ged-state",
> >> .version_id = 1,
> >> .minimum_version_id = 1,
> >> .fields = (VMStateField[]) {
> >> VMSTATE_UINT32(sel, GEDState),
> >> VMSTATE_END_OF_LIST()
> >> }
> >> };
> >>
> >> ...
> >>
> >> VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
> >>
> >> i.e. it looks like we are missing structure definition for AcpiGhesState
> >>
> >> CCing David,
> >> to help with migration magic in case I'm wrong or missed something
> >
> > Yeh that's confusing :-)
> >
> > Given a:
> >
> > VMSTATE_STRUCT(a, B, 1, vmstate_c, C)
> >
> > We're saying there's a field 'a' in type B, and field 'a'
> > should be of type C and be serialised using vmstate_c.
> >
> > That also means that in any vmstate_c, we're expecting it
> > to be passed a type C generally.
> >
> > Having said that; you don't need a struct - you can get away
> > with that VMSTATE_UINT64, there's two problems:
> >
> > a) That assumes that your ghes always stays that simple.
> > b) If you wanted to store a Ghes from a number of different
> > parent structures then you're stuck because your vmstate_ghes_state
> > is bound to being a strict field of AcpiGedState.
> >
> > So yes, it's neatest to do it using a VMSD for AcpiGhesState
> >
> > And congratulations on finding a loop; I don't think we've ever had one
> > before :-)
> >
> > Dave
> >
> >>> VMSTATE_END_OF_LIST()
> >>> }
> >>> };
> >>
>
> Do you mean that we need another VMStateDescription to describe
> AcpiGhesState instead of using VMSTATE_UINT64 directly? Maybe like this:
>
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 6df400e1ee16..5454be67d5f0 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -322,6 +322,16 @@ static const VMStateDescription vmstate_ged_state = {
> }
> };
>
> +static const VMStateDescription vmstate_ghes = {
> + .name = "acpi-ghes",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> + VMSTATE_END_OF_LIST()
> + },
> +};
> +
> static bool ghes_needed(void *opaque)
> {
> AcpiGedState *s = opaque;
> @@ -335,7 +345,7 @@ static const VMStateDescription vmstate_ghes_state = {
> .needed = ghes_needed,
> .fields = (VMStateField[]) {
> VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> - vmstate_ghes_state, AcpiGhesState),
> + vmstate_ghes, AcpiGhesState),
> VMSTATE_END_OF_LIST()
> }
> };
>
> --
> Thanks,
> Peng
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Wed, 11 Nov 2020 14:01:12 +0000
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> Is someone taking a fix for this in 5.2 - it's breaking vmstate
> comparison.
can you merge it via migration tree?
[...]
for fixed up version below
Acked-by: Igor Mammedov <imammedo@redhat.com>
> >
> > Do you mean that we need another VMStateDescription to describe
> > AcpiGhesState instead of using VMSTATE_UINT64 directly? Maybe like this:
> >
> > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > index 6df400e1ee16..5454be67d5f0 100644
> > --- a/hw/acpi/generic_event_device.c
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -322,6 +322,16 @@ static const VMStateDescription vmstate_ged_state = {
> > }
> > };
> >
> > +static const VMStateDescription vmstate_ghes = {
> > + .name = "acpi-ghes",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> > + VMSTATE_END_OF_LIST()
> > + },
> > +};
> > +
> > static bool ghes_needed(void *opaque)
> > {
> > AcpiGedState *s = opaque;
> > @@ -335,7 +345,7 @@ static const VMStateDescription vmstate_ghes_state = {
> > .needed = ghes_needed,
> > .fields = (VMStateField[]) {
> > VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > - vmstate_ghes_state, AcpiGhesState),
> > + vmstate_ghes, AcpiGhesState),
> > VMSTATE_END_OF_LIST()
> > }
> > };
> >
> > --
> > Thanks,
> > Peng
> >
* Igor Mammedov (imammedo@redhat.com) wrote:
> On Wed, 11 Nov 2020 14:01:12 +0000
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
>
> > Is someone taking a fix for this in 5.2 - it's breaking vmstate
> > comparison.
> can you merge it via migration tree?
I could; Peng: Could you give a sign-off for this version ?
Dave
> [...]
>
> for fixed up version below
> Acked-by: Igor Mammedov <imammedo@redhat.com>
>
> > >
> > > Do you mean that we need another VMStateDescription to describe
> > > AcpiGhesState instead of using VMSTATE_UINT64 directly? Maybe like this:
> > >
> > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > > index 6df400e1ee16..5454be67d5f0 100644
> > > --- a/hw/acpi/generic_event_device.c
> > > +++ b/hw/acpi/generic_event_device.c
> > > @@ -322,6 +322,16 @@ static const VMStateDescription vmstate_ged_state = {
> > > }
> > > };
> > >
> > > +static const VMStateDescription vmstate_ghes = {
> > > + .name = "acpi-ghes",
> > > + .version_id = 1,
> > > + .minimum_version_id = 1,
> > > + .fields = (VMStateField[]) {
> > > + VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
> > > + VMSTATE_END_OF_LIST()
> > > + },
> > > +};
> > > +
> > > static bool ghes_needed(void *opaque)
> > > {
> > > AcpiGedState *s = opaque;
> > > @@ -335,7 +345,7 @@ static const VMStateDescription vmstate_ghes_state = {
> > > .needed = ghes_needed,
> > > .fields = (VMStateField[]) {
> > > VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > > - vmstate_ghes_state, AcpiGhesState),
> > > + vmstate_ghes, AcpiGhesState),
> > > VMSTATE_END_OF_LIST()
> > > }
> > > };
> > >
> > > --
> > > Thanks,
> > > Peng
> > >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 11/12/2020 1:26 AM, Dr. David Alan Gilbert wrote:
> * Igor Mammedov (imammedo@redhat.com) wrote:
>> On Wed, 11 Nov 2020 14:01:12 +0000
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
>>
>>> Is someone taking a fix for this in 5.2 - it's breaking vmstate
>>> comparison.
>> can you merge it via migration tree?
>
> I could; Peng: Could you give a sign-off for this version ?
>
> Dave
OK, I'll send it as soon as possible.
Thanks,
Peng
>
>> [...]
>>
>> for fixed up version below
>> Acked-by: Igor Mammedov <imammedo@redhat.com>
>>
>>>>
>>>> Do you mean that we need another VMStateDescription to describe
>>>> AcpiGhesState instead of using VMSTATE_UINT64 directly? Maybe like this:
>>>>
>>>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>>>> index 6df400e1ee16..5454be67d5f0 100644
>>>> --- a/hw/acpi/generic_event_device.c
>>>> +++ b/hw/acpi/generic_event_device.c
>>>> @@ -322,6 +322,16 @@ static const VMStateDescription vmstate_ged_state = {
>>>> }
>>>> };
>>>>
>>>> +static const VMStateDescription vmstate_ghes = {
>>>> + .name = "acpi-ghes",
>>>> + .version_id = 1,
>>>> + .minimum_version_id = 1,
>>>> + .fields = (VMStateField[]) {
>>>> + VMSTATE_UINT64(ghes_addr_le, AcpiGhesState),
>>>> + VMSTATE_END_OF_LIST()
>>>> + },
>>>> +};
>>>> +
>>>> static bool ghes_needed(void *opaque)
>>>> {
>>>> AcpiGedState *s = opaque;
>>>> @@ -335,7 +345,7 @@ static const VMStateDescription vmstate_ghes_state = {
>>>> .needed = ghes_needed,
>>>> .fields = (VMStateField[]) {
>>>> VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
>>>> - vmstate_ghes_state, AcpiGhesState),
>>>> + vmstate_ghes, AcpiGhesState),
>>>> VMSTATE_END_OF_LIST()
>>>> }
>>>> };
>>>>
>>>> --
>>>> Thanks,
>>>> Peng
>>>>
>>
© 2016 - 2024 Red Hat, Inc.