Re: [Xen-devel] [PATCH 2/2] libxl: save/restore qemu's physmap

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-01-20 at 11:18 +0000, Stefano Stabellini wrote:
> Read Qemu's physmap from xenstore and save it using toolstack_save.
> Restore Qemu's physmap using toolstack_restore.

Shall we have a version field now so we don't dig ourselves a hole we
can't get out of?

> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl_dom.c |  132 
> ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 131 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index fd2b051..3d60a35 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -347,6 +347,62 @@ out:
>      return rc;
>  }
>  
> +static int libxl__toolstack_restore(uint32_t domid, uint8_t *buf,
> +        uint32_t size, void *data)
> +{
> +    libxl__gc *gc = (libxl__gc *) data;
> +    int i, ret;
> +    uint8_t *ptr = buf;
> +    uint32_t namelen = 0;
> +    char *name = NULL;
> +    uint32_t count = 0;
> +    uint64_t phys_offset_v = 0, start_addr_v = 0, size_v = 0;
> +
> +    if (size < sizeof(count))
> +        return -1;
> +
> +    memcpy(&count, ptr, sizeof(count));
> +    ptr += sizeof(count);
> + 
> +    if (size <
> +            sizeof(count) + count * (sizeof(uint64_t) * 3 + 
> sizeof(uint32_t)))
> +        return -1;
> +
> +    for (i = 0; i < count; i++) {
> +        memcpy(&namelen, ptr, sizeof(namelen));
> +        ptr += sizeof(namelen);
> +        if (namelen > 0) {
> +            name = (char *)ptr;
> +            ptr += namelen;
> +        }
> +        memcpy(&phys_offset_v, ptr, sizeof(uint64_t));
> +        ptr += sizeof(uint64_t);
> +        memcpy(&start_addr_v, ptr, sizeof(uint64_t));
> +        ptr += sizeof(uint64_t);
> +        memcpy(&size_v, ptr, sizeof(uint64_t));
> +        ptr += sizeof(uint64_t);

Why not define a struct libxl__physmap_info for these three? You could
even do the trick with the "char name[]" at the end and incorporate
namelen too if you wanted.

The maximum size of the allocation is
        sizeof(count) + count * (sizeof(uint64_t) * 3 + sizeof(uint32_t)))
here but in the save it is allocating:
        sizeof(count) + count * (sizeof(val) * 3 + sizeof(namelen));
these work out the same but it would be more obviously correct if it
were "count * sizeof(struct)"

Actually, hang on where is the space for name itself allocated? I see,
you are reallocing. If you have to do that anyway you may as well start
off with just sizeof(count) and realloc namelen + sizeof(struct) at each
stage.

> +
> +        ret = libxl__xs_write(gc, 0, libxl__sprintf(gc,
> +                    
> "/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
> +                    domid, phys_offset_v), "%"PRIx64, start_addr_v);
> +        if (ret)
> +            return -1;
> +        ret = libxl__xs_write(gc, 0, libxl__sprintf(gc,
> +                    "/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
> +                    domid, phys_offset_v), "%"PRIx64, size_v);
> +        if (ret)
> +            return -1;
> +        if (namelen > 0) {
> +            ret = libxl__xs_write(gc, 0, libxl__sprintf(gc,
> +                        
> "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
> +                        domid, phys_offset_v), "%s", name);
> +            if (ret)
> +                return -1;
> +        }
> +    }
> +    return 0;
> +}
> +
>  int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
>                                   libxl_domain_build_info *info,
>                                   libxl__domain_build_state *state,



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel