* [RFC 1/2] migration/vmstate: Introduce vmstate_save_state_with_err
2023-05-26 11:50 [RFC 0/2] migration: Update error description outside migration.c Tejus GK
@ 2023-05-26 11:50 ` Tejus GK
2023-05-26 11:50 ` [RFC 2/2] migration: Update error description outside migration.c Tejus GK
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Tejus GK @ 2023-05-26 11:50 UTC (permalink / raw)
To: qemu-devel
Cc: quintela, peterx, leobras, berrange, aravind.retnakaran,
shivam.kumar1, Tejus GK
Currently, a few code paths exist in the function vmstate_save_state_v,
which ultimately leads to a migration failure. However, an update in the
current MigrationState for the error description is never done.
vmstate.c somehow doesn't seem to allow the use of migrate_set_error due
to some dependencies for unit tests. Hence, this patch introduces a new
function vmstate_save_state_with_err, which will eventually propagate
the error message to savevm.c where a migrate_set_error call can be
eventually done.
Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
---
include/migration/vmstate.h | 4 +++-
migration/savevm.c | 2 +-
migration/vmstate.c | 12 +++++++++---
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 084f5e784a..244d00ca74 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1194,9 +1194,11 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id);
int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc);
+int vmstate_save_state_with_err(QEMUFile *f, const VMStateDescription *vmsd,
+ void *opaque, JSONWriter *vmdesc, Error **errp);
int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc,
- int version_id);
+ int version_id, Error **errp);
bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
diff --git a/migration/savevm.c b/migration/savevm.c
index 03795ce8dc..ec7f66619f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1000,7 +1000,7 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
if (!se->vmsd) {
vmstate_save_old_style(f, se, vmdesc);
} else {
- ret = vmstate_save_state(f, se->vmsd, se->opaque, vmdesc);
+ ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc, &local_err);
if (ret) {
return ret;
}
diff --git a/migration/vmstate.c b/migration/vmstate.c
index af01d54b6f..35a9b67afc 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -315,11 +315,17 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque)
int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc_id)
{
- return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id);
+ return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id, NULL);
+}
+
+int vmstate_save_state_with_err(QEMUFile *f, const VMStateDescription *vmsd,
+ void *opaque, JSONWriter *vmdesc_id, Error **errp)
+{
+ return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id, errp);
}
int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, JSONWriter *vmdesc, int version_id)
+ void *opaque, JSONWriter *vmdesc, int version_id, Error **errp)
{
int ret = 0;
const VMStateField *field = vmsd->fields;
@@ -377,7 +383,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
} else if (field->flags & VMS_VSTRUCT) {
ret = vmstate_save_state_v(f, field->vmsd, curr_elem,
vmdesc_loop,
- field->struct_version_id);
+ field->struct_version_id, errp);
} else {
ret = field->info->put(f, curr_elem, size, field,
vmdesc_loop);
--
2.22.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC 2/2] migration: Update error description outside migration.c
2023-05-26 11:50 [RFC 0/2] migration: Update error description outside migration.c Tejus GK
2023-05-26 11:50 ` [RFC 1/2] migration/vmstate: Introduce vmstate_save_state_with_err Tejus GK
@ 2023-05-26 11:50 ` Tejus GK
2023-06-12 5:00 ` [RFC 0/2] " Tejus GK
2023-06-12 13:14 ` Peter Xu
3 siblings, 0 replies; 5+ messages in thread
From: Tejus GK @ 2023-05-26 11:50 UTC (permalink / raw)
To: qemu-devel
Cc: quintela, peterx, leobras, berrange, aravind.retnakaran,
shivam.kumar1, Tejus GK
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.
Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
---
migration/savevm.c | 17 ++++++++++++++---
migration/vmstate.c | 7 ++++---
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index ec7f66619f..783d42009c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -979,6 +979,8 @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se)
static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
{
int ret;
+ Error *local_err = NULL;
+ MigrationState *s = migrate_get_current();
if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
return 0;
@@ -1002,6 +1004,8 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
} else {
ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc, &local_err);
if (ret) {
+ migrate_set_error(s, local_err);
+ error_report_err(local_err);
return ret;
}
}
@@ -1068,10 +1072,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;
}
@@ -1475,8 +1483,11 @@ 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 *local_err = NULL;
+ 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 35a9b67afc..71dc21c273 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"
@@ -336,7 +337,7 @@ 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(errp, "pre-save failed: %s", vmsd->name);
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);
if (vmsd->post_save) {
vmsd->post_save(opaque);
}
--
2.22.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC 0/2] migration: Update error description outside migration.c
2023-05-26 11:50 [RFC 0/2] migration: Update error description outside migration.c Tejus GK
2023-05-26 11:50 ` [RFC 1/2] migration/vmstate: Introduce vmstate_save_state_with_err Tejus GK
2023-05-26 11:50 ` [RFC 2/2] migration: Update error description outside migration.c Tejus GK
@ 2023-06-12 5:00 ` Tejus GK
2023-06-12 13:14 ` Peter Xu
3 siblings, 0 replies; 5+ messages in thread
From: Tejus GK @ 2023-06-12 5:00 UTC (permalink / raw)
To: qemu-devel
Cc: quintela, peterx, leobras, berrange, aravind.retnakaran,
shivam.kumar1
On 26/05/23 5:20 pm, Tejus GK wrote:
> Hi everyone,
>
> This patchset aims to cover code paths in the source code where a
> migration is marked as failed via MIGRATION_STATUS_FAILED, however the
> failure exists outside of migration.c, and without a call for
> migrate_set_error at this place.
>
> This patchset has been split out from the patchset sent before which
> covered cases of such gaps in migration.c aswell.
>
> Previous patchset:
> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04463.html
>
> Regards,
> Tejus
>
> Tejus GK (2):
> migration/vmstate: Introduce vmstate_save_state_with_err
> migration: Update error description outside migration.c
>
> include/migration/vmstate.h | 4 +++-
> migration/savevm.c | 19 +++++++++++++++----
> migration/vmstate.c | 19 +++++++++++++------
> 3 files changed, 31 insertions(+), 11 deletions(-)
>
Hi everyone,
Apologies for the delayed ping. Can someone please review this patchset?
Regards,
Tejus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 0/2] migration: Update error description outside migration.c
2023-05-26 11:50 [RFC 0/2] migration: Update error description outside migration.c Tejus GK
` (2 preceding siblings ...)
2023-06-12 5:00 ` [RFC 0/2] " Tejus GK
@ 2023-06-12 13:14 ` Peter Xu
3 siblings, 0 replies; 5+ messages in thread
From: Peter Xu @ 2023-06-12 13:14 UTC (permalink / raw)
To: Tejus GK
Cc: qemu-devel, quintela, leobras, berrange, aravind.retnakaran,
shivam.kumar1
On Fri, May 26, 2023 at 11:50:01AM +0000, Tejus GK wrote:
> Hi everyone,
>
> This patchset aims to cover code paths in the source code where a
> migration is marked as failed via MIGRATION_STATUS_FAILED, however the
> failure exists outside of migration.c, and without a call for
> migrate_set_error at this place.
>
> This patchset has been split out from the patchset sent before which
> covered cases of such gaps in migration.c aswell.
>
> Previous patchset:
> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04463.html
Acked-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 5+ messages in thread