On 06/30/2015 12:07 AM, Ian Campbell wrote:
On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote: ITYM "handling" in the subject. And perhaps "change" rather than "fix"?If the err is RECORD_NOT_PROCESSED, and it is an optional record, restore will still fail. There're two options to fix this, a, setting rc to 0 when it is an optional record. b, do the error handling in the caller. We choose b because: There will be another error type in COLO, which indicates a failover, that needs to be handled in restore(), so moving the error handling out to make the logic clearer...Otherwise, in process_record, RECORD_NOT_PROCESSED is handled, and in restore another error type returned from process_record is handled. Signed-off-by: Yang Hongyang <yan...@cn.fujitsu.com> CC: Ian Jackson <ian.jack...@eu.citrix.com> CC: Wei Liu <wei.l...@citrix.com> CC: Andrew Cooper <andrew.coop...@citrix.com> --- tools/libxc/xc_sr_restore.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index fd45775..d5645e0 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -569,19 +569,6 @@ static int process_record(struct xc_sr_context *ctx, struct xc_sr_record *rec) free(rec->data); rec->data = NULL; - if ( rc == RECORD_NOT_PROCESSED ) - { - if ( rec->type & REC_TYPE_OPTIONAL ) - DPRINTF("Ignoring optional record %#x (%s)", - rec->type, rec_type_to_str(rec->type)); - else - { - ERROR("Mandatory record %#x (%s) not handled", - rec->type, rec_type_to_str(rec->type)); - rc = -1; - } - } - return rc; } @@ -669,7 +656,20 @@ static int restore(struct xc_sr_context *ctx)There is a second caller of process_record in handle_checkpoint, so this change will result in a change of behaviour for that case, I think.
Ah, seems I missed the other caller, good catch, and I will fix it in the next version.
If that is intentional then it ought to be mentioned in the commit log, since otherwise it looks a lot like this change is supposed to result in no overall difference. Ian.else { rc = process_record(ctx, &rec); - if ( rc ) + if ( rc == RECORD_NOT_PROCESSED ) + { + if ( rec.type & REC_TYPE_OPTIONAL ) + DPRINTF("Ignoring optional record %#x (%s)", + rec.type, rec_type_to_str(rec.type)); + else + { + ERROR("Mandatory record %#x (%s) not handled", + rec.type, rec_type_to_str(rec.type)); + rc = -1; + goto err; + } + } + else if ( rc ) goto err; }.
-- Thanks, Yang. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
