Re: [PATCH 1/4] migration/savevm: respect qemu_fclose() error code in save_snapshot()
* Denis V. Lunev (d...@openvz.org) wrote: > qemu_fclose() could return error, f.e. if bdrv_co_flush() will return > the error. > > This validation will become more important once we will start waiting of > asynchronous IO operations, started from bdrv_write_vmstate(), which are > coming soon. > > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > CC: Max Reitz > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Juan Quintela > CC: "Dr. David Alan Gilbert" > CC: Vladimir Sementsov-Ogievskiy > CC: Denis Plotnikov We check the return value in very few other places; I think in the migration case we do flushes and assume that if the flushes work we were OK; then most of the closes happen on error paths or after points we think we're done. Reviewed-by: Dr. David Alan Gilbert > --- > migration/savevm.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index c00a6807d9..0ff5bb40ed 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2628,7 +2628,7 @@ int save_snapshot(const char *name, Error **errp) > { > BlockDriverState *bs, *bs1; > QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1; > -int ret = -1; > +int ret = -1, ret2; > QEMUFile *f; > int saved_vm_running; > uint64_t vm_state_size; > @@ -2712,10 +2712,14 @@ int save_snapshot(const char *name, Error **errp) > } > ret = qemu_savevm_state(f, errp); > vm_state_size = qemu_ftell(f); > -qemu_fclose(f); > +ret2 = qemu_fclose(f); > if (ret < 0) { > goto the_end; > } > +if (ret2 < 0) { > +ret = ret2; > +goto the_end; > +} > > /* The bdrv_all_create_snapshot() call that follows acquires the > AioContext > * for itself. BDRV_POLL_WHILE() does not support nested locking because > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH 1/4] migration/savevm: respect qemu_fclose() error code in save_snapshot()
11.06.2020 20:11, Denis V. Lunev wrote: qemu_fclose() could return error, f.e. if bdrv_co_flush() will return the error. This validation will become more important once we will start waiting of asynchronous IO operations, started from bdrv_write_vmstate(), which are coming soon. Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Max Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Juan Quintela CC: "Dr. David Alan Gilbert" CC: Vladimir Sementsov-Ogievskiy CC: Denis Plotnikov Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
[PATCH 1/4] migration/savevm: respect qemu_fclose() error code in save_snapshot()
qemu_fclose() could return error, f.e. if bdrv_co_flush() will return the error. This validation will become more important once we will start waiting of asynchronous IO operations, started from bdrv_write_vmstate(), which are coming soon. Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Max Reitz CC: Stefan Hajnoczi CC: Fam Zheng CC: Juan Quintela CC: "Dr. David Alan Gilbert" CC: Vladimir Sementsov-Ogievskiy CC: Denis Plotnikov --- migration/savevm.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/migration/savevm.c b/migration/savevm.c index c00a6807d9..0ff5bb40ed 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2628,7 +2628,7 @@ int save_snapshot(const char *name, Error **errp) { BlockDriverState *bs, *bs1; QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1; -int ret = -1; +int ret = -1, ret2; QEMUFile *f; int saved_vm_running; uint64_t vm_state_size; @@ -2712,10 +2712,14 @@ int save_snapshot(const char *name, Error **errp) } ret = qemu_savevm_state(f, errp); vm_state_size = qemu_ftell(f); -qemu_fclose(f); +ret2 = qemu_fclose(f); if (ret < 0) { goto the_end; } +if (ret2 < 0) { +ret = ret2; +goto the_end; +} /* The bdrv_all_create_snapshot() call that follows acquires the AioContext * for itself. BDRV_POLL_WHILE() does not support nested locking because -- 2.17.1