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

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3247 bytes --]

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 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


[-- Attachment #1.2: Type: text/html, Size: 7419 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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