On (Tue) 20 Jan 2015 [14:48:02], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
>
> Mostly on the load side, so that when we get a complaint about
> a migration failure we can figure out what it didn't like.
Nice!
Just one note below.
> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> ---
> migration/vmstate.c | 22 +++++++++++++++++++---
> savevm.c | 10 +++++++---
> trace-events | 11 ++++++++++-
> 3 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 2c0b135..7b8dc3f 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -75,14 +75,19 @@ int vmstate_load_state(QEMUFile *f, const
> VMStateDescription *vmsd,
> VMStateField *field = vmsd->fields;
> int ret;
>
> + trace_vmstate_load_state(vmsd->name, version_id);
> if (version_id > vmsd->version_id) {
> + trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
> return -EINVAL;
> }
> if (version_id < vmsd->minimum_version_id) {
> if (vmsd->load_state_old &&
> version_id >= vmsd->minimum_version_id_old) {
> - return vmsd->load_state_old(f, opaque, version_id);
> + ret = vmsd->load_state_old(f, opaque, version_id);
> + trace_vmstate_load_state_end(vmsd->name, "old path", ret);
> + return ret;
> }
> + trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
> return -EINVAL;
> }
> if (vmsd->pre_load) {
> @@ -92,6 +97,7 @@ int vmstate_load_state(QEMUFile *f, const
> VMStateDescription *vmsd,
> }
> }
> while (field->name) {
> + trace_vmstate_load_state_field(vmsd->name, field->name);
> if ((field->field_exists &&
> field->field_exists(opaque, version_id)) ||
> (!field->field_exists &&
> @@ -134,9 +140,10 @@ int vmstate_load_state(QEMUFile *f, const
> VMStateDescription *vmsd,
> return ret;
> }
> if (vmsd->post_load) {
> - return vmsd->post_load(opaque, version_id);
> + ret = vmsd->post_load(opaque, version_id);
> }
> - return 0;
> + trace_vmstate_load_state_end(vmsd->name, "end", ret);
> + return ret;
> }
"return 0" becomes "return ret", and ret isn't assigned anywhere.
> void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
> @@ -193,6 +200,8 @@ static const VMStateDescription *
> static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription
> *vmsd,
> void *opaque)
> {
> + trace_vmstate_subsection_load(vmsd->name);
> +
> while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
> char idstr[256];
> int ret;
> @@ -202,20 +211,24 @@ static int vmstate_subsection_load(QEMUFile *f, const
> VMStateDescription *vmsd,
> len = qemu_peek_byte(f, 1);
> if (len < strlen(vmsd->name) + 1) {
> /* subsection name has be be "section_name/a" */
> + trace_vmstate_subsection_load_bad(vmsd->name, "(short)");
> return 0;
Nice how you use the () to differentiate from the idstr below..
> }
> size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
> if (size != len) {
> + trace_vmstate_subsection_load_bad(vmsd->name, "(peek fail)");
> return 0;
> }
> idstr[size] = 0;
>
> if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
> + trace_vmstate_subsection_load_bad(vmsd->name, idstr);
> /* it don't have a valid subsection name */
> return 0;
Amit
