On Wed, Mar 04, 2026 at 11:06:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> [add s390 channel maintainers]
> 
> On 04.03.26 22:20, Peter Xu wrote:
> > On Wed, Mar 04, 2026 at 10:15:01PM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > The only problem is that we don't have error_append() function)
> > 
> > I definitely wanted to type error_prepend() when replying... :(
> > 
> > I think it might be better when putting upfront / prepend, say, the
> > original error should look like this, which is weird to me:
> > 
> >    "XXX != YYY NAME"
> > 
> > (maybe even no space???)
> > 
> > With append (when doing that, we may also want to add a comma and some "0x"
> > prefix?), it can be:
> > 
> >    "NAME: XXX != YYY"
> > 
> 
> 
> Hmm. I'm now trying to find, where this .err_hint is set to not-NULL.. Don't 
> see...
> 
> O, I see them:
> 
> in hw/s390x/css.c

Ouch.. This is the 2nd time in exactly the same day I saw a feature
introduced into migration core with an use case only in s390's css.c file,
which only works for very limited use case but likely not usable for the
rest...  The other one is VMS_NULLPTR_MARKER..

https://lore.kernel.org/all/[email protected]/#t

> 
> 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,
>     .minimum_version_id = 1,
>     .post_load = subch_dev_post_load,
>     .pre_save = subch_dev_pre_save,
>     .fields = (const VMStateField[]) {
>         VMSTATE_UINT8_EQUAL(cssid, SubchDev, "Bug!"),   <<<<<<<<<
>         VMSTATE_UINT8_EQUAL(ssid, SubchDev, "Bug!"),    <<<<<<<<<
>         VMSTATE_UINT16(migrated_schid, SubchDev),
>         VMSTATE_UINT16_EQUAL(devno, SubchDev, err_hint_devno),   <<<<<<<<
> 
> 
> 
> "Bug" hints are obviously useless.
> 
> About err_hint_devno.. Doesn't seem a good reason to have this .err_hint
> field and the whole logic, including extra parameter to several vmstate
> macros through the whole code base. And at least mention about 2.7 looks
> not relevant today.
> 
> I'd replace definition of err_hint_devno by a comment, and drop the whole
> .err_hint feature.
> 
> What do you think?

Yes, I'd agree.

-- 
Peter Xu


Reply via email to