Re: [Xen-devel] [BUGFIX][PATCH 3/4] hvm_save_one: return correct data.

[Don Slutz <dslutz@xxxxxxxxxxx>]

On 12/13/13 09:20, Jan Beulich wrote:
> > > > On 12.12.13 at  01:56, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
>> From: Don Slutz <dslutz@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> But this needs to be cleaned up coding style wise and ...
>
>> --- 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) { +            struct
>> hvm_save_descriptor *desc +                   = (struct
>> hvm_save_descriptor *)&ctxt.data[off];
>
> .. this could be const, and the cast could simply be (void *), ...
>
>> +            if (instance == desc->instance) { +                rv
>> = 0; +                if ( copy_to_guest(handle, +
>> ctxt.data +                                   + off
>
> ... this doesn't need to be on a separate line, and ...
>
>> +                                   + sizeof (struct
>> hvm_save_descriptor), +
>> hvm_sr_handlers[typecode].size +
>> - sizeof (struct hvm_save_descriptor)) )
>
> ... both these sizeof()s would now better be sizeof(*desc).
>
> Jan
>
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@xxxxxxxxxxx>
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@xxxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 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;
--
1.7.11.7

   -Don Slutz

Attachment: 0003-hvm_save_one-return-correct-data.patch
Description: Text Data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel