On 04/10/23 1:53 pm, Juan Quintela wrote:
Tejus GK <tejus...@nutanix.com> wrote:
On 03/10/23 6:14 pm, Juan Quintela wrote:
Tejus GK <tejus...@nutanix.com> wrote:
A few code paths exist in the source code,where a migration is
marked as failed via MIGRATION_STATUS_FAILED, but the failure happens
outside of migration.c
In such cases, an error_report() call is made, however the current
MigrationState is never updated with the error description, and hence
clients like libvirt never know the actual reason for the failure.
This patch covers such cases outside of migration.c and updates the
error description at the appropriate places.
Acked-by: Peter Xu <pet...@redhat.com>
Signed-off-by: Tejus GK <tejus...@nutanix.com>
Reviewed-by: Juan Quintela <quint...@redhat.com>
Queued.
Thanks, will be sending out a patch with the "Reviewed by" trailer added.
Send the other one. This one is already queued.
I think that the error_report() thing, you need to review more things
than the one in this patch.
Not sure what you mean here? The only other patch I have on the list
apart from this one is
https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg00280.html ,
which you marked as reviewed as well.
return ret;
}
}
@@ -389,8 +390,8 @@ int vmstate_save_state_v(QEMUFile *f, const
VMStateDescription *vmsd,
vmdesc_loop);
}
if (ret) {
- error_report("Save of field %s/%s failed",
- vmsd->name, field->name);
+ error_setg(errp, "Save of field %s/%s failed",
+ vmsd->name, field->name);
Same here.
You're right, I'm only setting it here and reporting it eventually in
savevm.c. The trivial solution for this would have been directly doing
a migrate_set_error() here, but that ended up breaking the build for
the unit test test-vmstate.c
What was the error? Because the problem could be on the test.
This is what I keep getting.
FAILED: tests/unit/test-vmstate
cc -m64 -mcx16 -o tests/unit/test-vmstate
tests/unit/test-vmstate.p/test-vmstate.c.o -Wl,--as-needed
-Wl,--no-undefined -pie -Wl,--whole-archive -Wl,--start-group
libevent-loop-base.a libqom.fa libio.fa libcrypto.fa libauthz.fa
-Wl,--no-whole-archive -fstack-protector-strong -Wl,-z,relro -Wl,-z,now
-Wl,--warn-common libqemuutil.a
subprojects/libvhost-user/libvhost-user-glib.a
subprojects/libvhost-user/libvhost-user.a libmigration.fa libqom.fa
libio.fa libcrypto.fa libauthz.fa /usr/lib64/libz.so
/usr/lib64/libgio-2.0.so /usr/lib64/libgobject-2.0.so
/usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -pthread -lm
/usr/lib64/libpixman-1.so -Wl,--end-group
libmigration.fa.p/migration_vmstate.c.o: In function `vmstate_save_state_v':
/rpmbuild/SOURCES/qemu/build/../migration/vmstate.c:333: undefined
reference to `migrate_get_current'
/rpmbuild/SOURCES/qemu/build/../migration/vmstate.c:344: undefined
reference to `migrate_set_error'
collect2: error: ld returned 1 exit status
I tried to figure out how the dependencies work out for unit test, but
found no luck with that.
if (vmsd->post_save) {
vmsd->post_save(opaque);
}
So, I am wondering if it could be better to just report the error in
a
single place for migration, and set it whenever we need it?
Yes, that would be very convenient, for all the errors to be reported
in lets say migration.c. Though that'd also require all the subsystems
under migration.c to properly propagate the errors.
Yeap, it requires auditing all the entry points, but will make easier to
know when we just need to set the error, the system will do the rest.
Thanks, Juan.
regards,
tejus
