* Amit Shah (amit.s...@redhat.com) wrote:
> 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.
Oops, yes, I'll reroll it.
Dave
>
> > 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
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
