On Tue, Oct 21, 2025 at 04:43:52PM +0100, Peter Maydell wrote: > On Fri, 3 Oct 2025 at 16:39, Peter Xu <[email protected]> wrote: > > > > From: Arun Menon <[email protected]> > > > > This is an incremental step in converting vmstate loading > > code to report error via Error objects instead of directly > > printing it to console/monitor. > > It is ensured that vmstate_load_state() must report an error > > in errp, in case of failure. > > > > The errors are temporarily reported using error_report_err(). > > This is removed in the subsequent patches in this series, > > when we are actually able to propagate the error to the calling > > function using errp. Whereas, if we want the function to exit on > > error, then error_fatal is passed. > > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > > index de35902213..e61585aa61 100644 > > --- a/hw/display/virtio-gpu.c > > +++ b/hw/display/virtio-gpu.c > > @@ -1347,7 +1347,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, > > size_t size, > > } > > > > /* load & apply scanout state */ > > - vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1); > > + vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1, > > &error_fatal); > > This is in a migration VMState .get function -- shouldn't we > be passing failure up to the caller, rather than exiting > with error_fatal here ? > > The commit message says some of this is fixed in subsequent > patches, but as of today this is still the code in git. > The other callsites which pass error_fatal to vmstate_load_state() > also look wrong: > > hw/s390x/virtio-ccw.c: return vmstate_load_state(f, > &vmstate_virtio_ccw_dev, dev, 1, &error_fatal); > hw/virtio/virtio-mmio.c: return vmstate_load_state(f, > &vmstate_virtio_mmio, proxy, 1, &error_fatal); > hw/virtio/virtio-pci.c: return vmstate_load_state(f, > &vmstate_virtio_pci, proxy, 1, &error_fatal); > > as they are written to return an error value that they'll > never see because of the use of error_fatal here.
Indeed a fair question to ask. > > Do you have plans for further cleanup/extension of the > use of Error here that would let these functions pass > the Error back up the chain ? It would be non-trivial though as we'll need to change VMStateInfo.get() API and that'll be another lot of churns. Arun, should we pass NULL for above three occurances, so that we will still not brute force quit when error happens? Do you want to send a patch? -- Peter Xu
