On Wed, Mar 04, 2026 at 07:27:48PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 03.03.26 18:22, Peter Xu wrote:
> > On Sat, Feb 21, 2026 at 12:02:04AM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > Simplify vmstate_save_state() which is rather big, and simplify further
> > > refactoring.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> > 
> > Reviewed-by: Peter Xu <[email protected]>
> > 
> > One trivial thing..
> > 
> > > ---
> > >   migration/vmstate.c | 34 ++++++++++++++++++++++------------
> > >   1 file changed, 22 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/migration/vmstate.c b/migration/vmstate.c
> > > index b07bbdd366..810b131f18 100644
> > > --- a/migration/vmstate.c
> > > +++ b/migration/vmstate.c
> > > @@ -430,6 +430,26 @@ bool vmstate_section_needed(const VMStateDescription 
> > > *vmsd, void *opaque)
> > >       return true;
> > >   }
> > > +static bool vmstate_pre_save(const VMStateDescription *vmsd, void 
> > > *opaque,
> > > +                             Error **errp)
> > > +{
> > > +    ERRP_GUARD();
> > 
> > I can come up with no reason a caller should pass in NULL for errp..  I
> > don't know why we assert(errp) so rarely in qemu for similar cases, but
> > iiuc that's what we really want here..
> 
> assert(errp) will not handle error_fatal case correctly. ERRP_GUARD 
> guarantees,
> that in case of errp == &error_fatal, the error message will contain what we
> prepend by error_prepend().
> 
> So, current practice is add ERRP_GUARD in any function which want to use
> error_prepend(errp, ..).
> 
> May be, we may have some additional macro
> 
> #define ERRP_GOOD(); which will assert that errp is not in (NULL, 
> &error_fatal),
> 
> and then, use it here, but then we should check, that for the whole callers *
> caller either pass errp and has ERRP_GOOD() macro, or pass local error object.

Ohhhh, OK, thanks.  Let's leave that for later, keep my R-b.

-- 
Peter Xu


Reply via email to