True, but the inverse is the case here; .save writes 'n' 'size' blocks. Form the loop above:On 15/12/2013 00:29, Don Slutz wrote:
I think I have corrected all coding errors (please check again). And done all requested changes. I did add the reviewed by (not sure if I should since this changes a large part of the patch, but they are all what Jan said).
I have unit tested it and it appears to work the same as the previous version (as expected).
Here is the new version, also attached.
From e0e8f5246ba492b153884cea93bfe753f1b0782e Mon Sep 17 00:00:00 2001
From: Don Slutz <dslutz@verizon.com>
Date: Tue, 12 Nov 2013 08:22:53 -0500
Subject: [PATCH v2 3/4] hvm_save_one: return correct data.
It is possible that hvm_sr_handlers[typecode].save does not use all
the provided room. In that case, using:
instance * hvm_sr_handlers[typecode].size
does not select the correct instance. Add code to search for the
correct instance.
Signed-off-by: Don Slutz <dslutz@verizon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
but this fairs no better at selecting the correct subset in the case that less data than hvm_sr_handlers[typecode].size is written by hvm_sr_handlers[typecode].save.
It always increments by 'size' bytes, and will only copy the data back if the bytes under desc->instance happen to match the instance we are looking for.The only time it does not find one is for an offline vcpu. Try out the unit test code in patch #1 on an unchanged xen. It should not display anything. Then offline a cpu in a domU (echo 0 > /sys/devices/system/cpu/cpu1/online). And with 3 vcpus, it will report an error.
The only solution I can see is that for the per-vcpu records, the save functions get refactored to take an instance ID, and only save their specific instance.
I shall have a go at this and see how invasive it is.
~Andrew
---
xen/common/hvm/save.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c
index de76ada..6aaea6f 100644
--- a/xen/common/hvm/save.c
+++ b/xen/common/hvm/save.c
@@ -112,13 +112,27 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance,
d->domain_id, typecode);
rv = -EFAULT;
}
- else if ( copy_to_guest(handle,
- ctxt.data
- + (instance * hvm_sr_handlers[typecode].size)
- + sizeof (struct hvm_save_descriptor),
- hvm_sr_handlers[typecode].size
- - sizeof (struct hvm_save_descriptor)) )
- rv = -EFAULT;
+ else
+ {
+ uint32_t off;
+
+ rv = -EBADSLT;
+ for ( off = 0; off < ctxt.cur; off += hvm_sr_handlers[typecode].size )
+ {
+ const struct hvm_save_descriptor *desc = (void *)&ctxt.data[off];
+
+ if ( instance == desc->instance )
+ {
+ rv = 0;
+ if ( copy_to_guest(handle,
+ ctxt.data + off + sizeof(*desc),
+ hvm_sr_handlers[typecode].size
+ - sizeof(*desc)) )
+ rv = -EFAULT;
+ break;
+ }
+ }
+ }
xfree(ctxt.data);
return rv;
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel