On 2/24/2022 1:25 PM, Dr. David Alan Gilbert wrote: > * Steve Sistare (steven.sist...@oracle.com) wrote: >> Use qemu_file_open to simplify a few functions in savevm.c. >> No functional change. >> >> Signed-off-by: Steve Sistare <steven.sist...@oracle.com> > > So I think this is mostly OK, but a couple of minor tidyups below; > so with the tidies and the renames from the previous patch: > > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Cool, thanks. >> --- >> migration/savevm.c | 21 +++++++-------------- >> 1 file changed, 7 insertions(+), 14 deletions(-) >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 0bef031..c71d525 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2910,8 +2910,9 @@ bool save_snapshot(const char *name, bool overwrite, >> const char *vmstate, >> void qmp_xen_save_devices_state(const char *filename, bool has_live, bool >> live, >> Error **errp) >> { >> + const char *ioc_name = "migration-xen-save-state"; >> + int flags = O_WRONLY | O_CREAT | O_TRUNC; > > I don't see why to take these (or the matching ones in load) as separate > variables; just keep them as is, and be parameters. Will do. - Steve >> QEMUFile *f; >> - QIOChannelFile *ioc; >> int saved_vm_running; >> int ret; >> >> @@ -2925,14 +2926,10 @@ void qmp_xen_save_devices_state(const char >> *filename, bool has_live, bool live, >> vm_stop(RUN_STATE_SAVE_VM); >> global_state_store_running(); >> >> - ioc = qio_channel_file_new_path(filename, O_WRONLY | O_CREAT | O_TRUNC, >> - 0660, errp); >> - if (!ioc) { >> + f = qemu_file_open(filename, flags, 0660, ioc_name, errp); >> + if (!f) { >> goto the_end; >> } >> - qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state"); >> - f = qemu_fopen_channel_output(QIO_CHANNEL(ioc)); >> - object_unref(OBJECT(ioc)); >> ret = qemu_save_device_state(f); >> if (ret < 0 || qemu_fclose(f) < 0) { >> error_setg(errp, QERR_IO_ERROR); >> @@ -2960,8 +2957,8 @@ void qmp_xen_save_devices_state(const char *filename, >> bool has_live, bool live, >> >> void qmp_xen_load_devices_state(const char *filename, Error **errp) >> { >> + const char *ioc_name = "migration-xen-load-state"; >> QEMUFile *f; >> - QIOChannelFile *ioc; >> int ret; >> >> /* Guest must be paused before loading the device state; the RAM state >> @@ -2973,14 +2970,10 @@ void qmp_xen_load_devices_state(const char >> *filename, Error **errp) >> } >> vm_stop(RUN_STATE_RESTORE_VM); >> >> - ioc = qio_channel_file_new_path(filename, O_RDONLY | O_BINARY, 0, errp); >> - if (!ioc) { >> + f = qemu_file_open(filename, O_RDONLY | O_BINARY, 0, ioc_name, errp); >> + if (!f) { >> return; >> } >> - qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state"); >> - f = qemu_fopen_channel_input(QIO_CHANNEL(ioc)); >> - object_unref(OBJECT(ioc)); >> - >> ret = qemu_loadvm_state(f); >> qemu_fclose(f); >> if (ret < 0) { >> -- >> 1.8.3.1 >> >>