hw/s390x/css.c | 6 ++++- include/migration/vmstate.h | 55 ++++++++++++++++++++++++++++++++++++++++----- migration/vmstate-types.c | 36 ++++++++++++++++++++++++----- 3 files changed, 85 insertions(+), 12 deletions(-)
The goal is to be able to specify a hint for the case a vmstate equal assertion fails. Was previously discussed here: https://patchwork.kernel.org/patch/9720029/ The third patch is an usage example. It's on top of this https://www.mail-archive.com/qemu-devel@nongnu.org/msg454941.html patch and mimics the behavior of the patch reference above. I have faked the subject of this third patch so patchew does not complain about that not applying on top of master (let's see if that works). Halil Pasic (3): vmstate: error hint for failed equal checks vmstate: error hint for failed equal checks part 2 s390x/css: add hint for devno missmatch hw/s390x/css.c | 6 ++++- include/migration/vmstate.h | 55 ++++++++++++++++++++++++++++++++++++++++----- migration/vmstate-types.c | 36 ++++++++++++++++++++++++----- 3 files changed, 85 insertions(+), 12 deletions(-) -- 2.11.2
This one has to be fixed up to 's390x: vmstatify config migration for
virtio-ccw' provided we want to achieve the same as 's390x/css: catch
section mismatch on load' does.
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
This is on tom of 's390x: vmstatify config migration for virtio-ccw'
which ain't on top of 's390x/css: catch section mismatch on load' but on
top of master. I kind of have a circular dependency here. This is why
the series is RFC.
Wanted to provide an usage example. Faked 'Re: ' so patchew does not
try to apply this on top of current master.
---
hw/s390x/css.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 348129e1b2..de277d6a3d 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -134,6 +134,10 @@ static const VMStateDescription vmstate_sense_id = {
static int subch_dev_post_load(void *opaque, int version_id);
static void subch_dev_pre_save(void *opaque);
+const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
+ " Likely reason: some sequences of plug and unplug can break"
+ " migration for machine versions prior to 2.7 (known design flaw).";
+
const VMStateDescription vmstate_subch_dev = {
.name = "s390_subch_dev",
.version_id = 1,
@@ -144,7 +148,7 @@ const VMStateDescription vmstate_subch_dev = {
VMSTATE_UINT8_EQUAL(cssid, SubchDev),
VMSTATE_UINT8_EQUAL(ssid, SubchDev),
VMSTATE_UINT16(migrated_schid, SubchDev),
- VMSTATE_UINT16(devno, SubchDev),
+ VMSTATE_UINT16_EQUAL_HINT(devno, SubchDev, err_hint_devno),
VMSTATE_BOOL(thinint_active, SubchDev),
VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB),
VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32),
--
2.11.2
* Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> This one has to be fixed up to 's390x: vmstatify config migration for
> virtio-ccw' provided we want to achieve the same as 's390x/css: catch
> section mismatch on load' does.
>
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
>
> This is on tom of 's390x: vmstatify config migration for virtio-ccw'
p
> which ain't on top of 's390x/css: catch section mismatch on load' but on
> top of master. I kind of have a circular dependency here. This is why
> the series is RFC.
>
> Wanted to provide an usage example. Faked 'Re: ' so patchew does not
> try to apply this on top of current master.
> ---
> hw/s390x/css.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 348129e1b2..de277d6a3d 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -134,6 +134,10 @@ static const VMStateDescription vmstate_sense_id = {
> static int subch_dev_post_load(void *opaque, int version_id);
> static void subch_dev_pre_save(void *opaque);
>
> +const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
> + " Likely reason: some sequences of plug and unplug can break"
> + " migration for machine versions prior to 2.7 (known design flaw).";
> +
That's ok, but I suggest:
* 'bug' rather than 'design flaw' - it sounds a bit less scary to
endusers.
Other than that,
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Dave
> const VMStateDescription vmstate_subch_dev = {
> .name = "s390_subch_dev",
> .version_id = 1,
> @@ -144,7 +148,7 @@ const VMStateDescription vmstate_subch_dev = {
> VMSTATE_UINT8_EQUAL(cssid, SubchDev),
> VMSTATE_UINT8_EQUAL(ssid, SubchDev),
> VMSTATE_UINT16(migrated_schid, SubchDev),
> - VMSTATE_UINT16(devno, SubchDev),
> + VMSTATE_UINT16_EQUAL_HINT(devno, SubchDev, err_hint_devno),
> VMSTATE_BOOL(thinint_active, SubchDev),
> VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB),
> VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32),
> --
> 2.11.2
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 06/07/2017 01:22 PM, Dr. David Alan Gilbert wrote:
> * Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
>> This one has to be fixed up to 's390x: vmstatify config migration for
>> virtio-ccw' provided we want to achieve the same as 's390x/css: catch
>> section mismatch on load' does.
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
>>
>> This is on tom of 's390x: vmstatify config migration for virtio-ccw'
> p
>> which ain't on top of 's390x/css: catch section mismatch on load' but on
>> top of master. I kind of have a circular dependency here. This is why
>> the series is RFC.
>>
>> Wanted to provide an usage example. Faked 'Re: ' so patchew does not
>> try to apply this on top of current master.
>> ---
>> hw/s390x/css.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 348129e1b2..de277d6a3d 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -134,6 +134,10 @@ static const VMStateDescription vmstate_sense_id = {
>> static int subch_dev_post_load(void *opaque, int version_id);
>> static void subch_dev_pre_save(void *opaque);
>>
>> +const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
>> + " Likely reason: some sequences of plug and unplug can break"
>> + " migration for machine versions prior to 2.7 (known design flaw).";
>> +
>
> That's ok, but I suggest:
> * 'bug' rather than 'design flaw' - it sounds a bit less scary to
> endusers.
>
I do not think it can be changed now. Christian as already sent out a pull
request for the patch this series is re-doing with vmstate. That patch has the
same error message. I have considered bug but decided against because
the 'bug' can't be fixed. I think it's pretty hard to hit this with normal
usage (and that's probably the reason why it went undetected until recently),
so hope not too may endusers are going to get scared by developer honesty :).
> Other than that,
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Great thanks for the review!
Halil
* Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
>
>
> On 06/07/2017 01:22 PM, Dr. David Alan Gilbert wrote:
> > * Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> >> This one has to be fixed up to 's390x: vmstatify config migration for
> >> virtio-ccw' provided we want to achieve the same as 's390x/css: catch
> >> section mismatch on load' does.
> >>
> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> >> ---
> >>
> >> This is on tom of 's390x: vmstatify config migration for virtio-ccw'
> > p
> >> which ain't on top of 's390x/css: catch section mismatch on load' but on
> >> top of master. I kind of have a circular dependency here. This is why
> >> the series is RFC.
> >>
> >> Wanted to provide an usage example. Faked 'Re: ' so patchew does not
> >> try to apply this on top of current master.
> >> ---
> >> hw/s390x/css.c | 6 +++++-
> >> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> >> index 348129e1b2..de277d6a3d 100644
> >> --- a/hw/s390x/css.c
> >> +++ b/hw/s390x/css.c
> >> @@ -134,6 +134,10 @@ static const VMStateDescription vmstate_sense_id = {
> >> static int subch_dev_post_load(void *opaque, int version_id);
> >> static void subch_dev_pre_save(void *opaque);
> >>
> >> +const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
> >> + " Likely reason: some sequences of plug and unplug can break"
> >> + " migration for machine versions prior to 2.7 (known design flaw).";
> >> +
> >
> > That's ok, but I suggest:
> > * 'bug' rather than 'design flaw' - it sounds a bit less scary to
> > endusers.
> >
>
> I do not think it can be changed now. Christian as already sent out a pull
> request for the patch this series is re-doing with vmstate. That patch has the
> same error message. I have considered bug but decided against because
> the 'bug' can't be fixed. I think it's pretty hard to hit this with normal
> usage (and that's probably the reason why it went undetected until recently),
> so hope not too may endusers are going to get scared by developer honesty :).
No problem; it's your device anyway :-)
> > Other than that,
> >
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Great thanks for the review!
Dave
> Halil
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
>> This one has to be fixed up to 's390x: vmstatify config migration for
>> virtio-ccw' provided we want to achieve the same as 's390x/css: catch
>> section mismatch on load' does.
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
>>
>> This is on tom of 's390x: vmstatify config migration for virtio-ccw'
> p
>> which ain't on top of 's390x/css: catch section mismatch on load' but on
>> top of master. I kind of have a circular dependency here. This is why
>> the series is RFC.
>>
>> Wanted to provide an usage example. Faked 'Re: ' so patchew does not
>> try to apply this on top of current master.
>> ---
>> hw/s390x/css.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 348129e1b2..de277d6a3d 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -134,6 +134,10 @@ static const VMStateDescription vmstate_sense_id = {
>> static int subch_dev_post_load(void *opaque, int version_id);
>> static void subch_dev_pre_save(void *opaque);
>>
>> +const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
>> + " Likely reason: some sequences of plug and unplug can break"
>> + " migration for machine versions prior to 2.7 (known design flaw).";
>> +
>
> That's ok, but I suggest:
> * 'bug' rather than 'design flaw' - it sounds a bit less scary to
> endusers.
If we are being politically correct:
(known limitation)?
O:-)
Reviewed-by: Juan Quintela <quintela@redhat.com>
Later, Juan.
© 2016 - 2025 Red Hat, Inc.