There are places outside of migration.c which eventually leads to a
migration failure, but the failure reason is never updated. Hence
libvirt doesn't know why the migration failed when it queries for it.
Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
---
migration/savevm.c | 13 ++++++++++---
migration/vmstate.c | 13 ++++++++++---
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index e33788343a..39d4ecdd41 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1068,10 +1068,14 @@ void qemu_savevm_send_open_return_path(QEMUFile *f)
int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len)
{
uint32_t tmp;
+ MigrationState *ms = migrate_get_current();
+ Error *local_err = NULL;
if (len > MAX_VM_CMD_PACKAGED_SIZE) {
- error_report("%s: Unreasonably large packaged state: %zu",
+ error_setg(&local_err, "%s: Unreasonably large packaged state: %zu",
__func__, len);
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
return -1;
}
@@ -1456,6 +1460,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
int vmdesc_len;
SaveStateEntry *se;
int ret;
+ Error *local_err = NULL;
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->vmsd && se->vmsd->early_setup) {
@@ -1475,8 +1480,10 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
* bdrv_activate_all() on the other end won't fail. */
ret = bdrv_inactivate_all();
if (ret) {
- error_report("%s: bdrv_inactivate_all() failed (%d)",
- __func__, ret);
+ error_setg(&local_err, "%s: bdrv_inactivate_all() failed (%d)",
+ __func__, ret);
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
qemu_file_set_error(f, ret);
return ret;
}
diff --git a/migration/vmstate.c b/migration/vmstate.c
index af01d54b6f..3a5770b925 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -14,6 +14,7 @@
#include "migration.h"
#include "migration/vmstate.h"
#include "savevm.h"
+#include "qapi/error.h"
#include "qapi/qmp/json-writer.h"
#include "qemu-file.h"
#include "qemu/bitops.h"
@@ -323,6 +324,8 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
{
int ret = 0;
const VMStateField *field = vmsd->fields;
+ MigrationState *ms = migrate_get_current();
+ Error *local_err = NULL;
trace_vmstate_save_state_top(vmsd->name);
@@ -330,7 +333,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
ret = vmsd->pre_save(opaque);
trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
if (ret) {
- error_report("pre-save failed: %s", vmsd->name);
+ error_setg(&local_err, "pre-save failed: %s", vmsd->name);
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
return ret;
}
}
@@ -383,8 +388,10 @@ 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(&local_err, "Save of field %s/%s failed",
+ vmsd->name, field->name);
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
if (vmsd->post_save) {
vmsd->post_save(opaque);
}
--
2.22.3Tejus GK <tejus.gk@nutanix.com> wrote:
> There are places outside of migration.c which eventually leads to a
> migration failure, but the failure reason is never updated. Hence
> libvirt doesn't know why the migration failed when it queries for it.
>
> Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
If you have to respin:
> @@ -1456,6 +1460,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> int vmdesc_len;
> SaveStateEntry *se;
> int ret;
> + Error *local_err = NULL;
You can declare this:
> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> if (se->vmsd && se->vmsd->early_setup) {
> @@ -1475,8 +1480,10 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> * bdrv_activate_all() on the other end won't fail. */
> ret = bdrv_inactivate_all();
> if (ret) {
here
> - error_report("%s: bdrv_inactivate_all() failed (%d)",
> - __func__, ret);
On 18/05/23 5:22 pm, Juan Quintela wrote:
> Tejus GK <tejus.gk@nutanix.com> wrote:
>> There are places outside of migration.c which eventually leads to a
>> migration failure, but the failure reason is never updated. Hence
>> libvirt doesn't know why the migration failed when it queries for it.
>>
>> Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
Thank you for the reviews Juan, but I believe that this particular patch shouldn't be approved yet. I have mentioned it in the RFC cover letter that the changes in this patch, in the file vmstate.c, end up breaking the build for a unit-test, eventually breaking the entire build.
I was not sure how to implement the error reporting properly in such cases, and the aim of this patch was to receive advice on the same.
>
>
> If you have to respin:
>
>> @@ -1456,6 +1460,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> int vmdesc_len;
>> SaveStateEntry *se;
>> int ret;
>> + Error *local_err = NULL;
>
> You can declare this:
>
>> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> if (se->vmsd && se->vmsd->early_setup) {
>> @@ -1475,8 +1480,10 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> * bdrv_activate_all() on the other end won't fail. */
>> ret = bdrv_inactivate_all();
>> if (ret) {
>
> here
>
>> - error_report("%s: bdrv_inactivate_all() failed (%d)",
>> - __func__, ret);
>
© 2016 - 2026 Red Hat, Inc.