Tejus GK <tejus...@nutanix.com> wrote: > 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.
Ouch, that again. I think that I know how to fix that. Will take a look. Later, Juan.
