Re: [Xen-devel] [PATCH for-4.6 5/8] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jul 28, 2015 at 10:44:40PM +0100, Andrew Cooper wrote:
> The new EMULATOR_XENSTORE_DATA content is a sequence of NUL terminated
> key/value strings, with the key relative to the device model's xenstore
> tree.
> 
> A sample might look like (as decoded by verify-stream-v2):
> 
>     Emulator Xenstore Data (Qemu Upstream, idx 0)
>       'physmap/1f00000/start_addr' = 'f0000000'
>       'physmap/1f00000/size' = '800000'
>       'physmap/1f00000/name' = 'vga.vram'
> 
> This patch introduces libxl helpers to save and restore this new format.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_dom.c      |  141 
> ++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h |    4 ++
>  2 files changed, 145 insertions(+)
> 
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 5555fea..885eb5e 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1151,6 +1151,65 @@ int libxl__toolstack_restore(uint32_t domid, const 
> uint8_t *ptr,
>      return ret;
>  }
>  
> +int libxl__restore_emulator_xenstore_data(libxl__domain_create_state *dcs,
> +                                          const char *ptr, uint32_t size)
> +{
> +    STATE_AO_GC(dcs->ao);
> +    const char *key = ptr, *val;
> +    int rc;
> +
> +    const uint32_t domid = dcs->guest_domid;
> +    const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> +    const char *xs_root = libxl__device_model_xs_path(gc, dm_domid, domid, 
> "");
> +
> +    while ( size ) {

Coding style issues in this function.

> +        /* Sanitise key. */
> +        size_t keylen = strnlen(key, size);
> +
> +        if ( keylen == size ) {
> +            rc = ERROR_FAIL;
> +            LOG(ERROR, "Key in xenstore data not NUL terminated");
> +            goto out;
> +        }
> +        if ( keylen == 0 ) {
> +            rc = ERROR_FAIL;
> +            LOG(ERROR, "NULL key found in xenstore data");
> +            goto out;
> +        }
> +        if ( key[0] == '/' ) {
> +            rc = ERROR_FAIL;
> +            LOG(ERROR, "Key in xenstore data not relative");
> +            goto out;
> +        }
> +
> +        size -= (keylen + 1);
> +
> +        /* The val string starts after the NUL terminator of key. */
> +        val = key + keylen + 1;
> +
> +        /* Sanitise val. */
> +        size_t vallen = strnlen(val, size);
> +
> +        if ( vallen == size ) {
> +            rc = ERROR_FAIL;
> +            LOG(ERROR, "Val in xenstore data not NUL terminated");
> +            goto out;
> +        }
> +
> +        size -= (vallen + 1);
> +
> +        libxl__xs_write(gc, 0, GCSPRINTF("%s/%s", xs_root, key), "%s", val);
> +
> +        /* Move key forwards over the key and value pair just written. */
> +        key = key + keylen + vallen + 2;
> +    }
> +
> +    rc = 0;
> +
> + out:
> +    return rc;
> +}
> +
>  /*==================== Domain suspend (save) ====================*/
>  
>  static void stream_done(libxl__egc *egc,
> @@ -1487,6 +1546,88 @@ int libxl__toolstack_save(uint32_t domid, uint8_t 
> **buf,
>      return ret;
>  }
>  
> +int libxl__save_emulator_xenstore_data(libxl__domain_suspend_state *dss,
> +                                       uint8_t **callee_buf,
> +                                       uint32_t *callee_len)
> +{
> +    STATE_AO_GC(dss->ao);
> +    const char *xs_path;
> +    char **entries, *ptr, *buf = NULL;
> +    unsigned int nr_entries, rel_start, i, len = 0;
> +    int rc;
> +
> +    const uint32_t domid = dss->domid;
> +    const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
> +
> +    xs_path = libxl__device_model_xs_path(gc, dm_domid, domid, "/physmap");
> +    if (!xs_path) { rc = 0; goto out; }
> +
> +    /* &foo[rel_start] is the xenstore path starting at the 'p' of physmap */
> +    rel_start = strlen(xs_path) - 7;
> +
> +    entries = libxl__xs_directory(gc, 0, xs_path, &nr_entries);
> +    if (!entries || nr_entries == 0) { rc = 0; goto out; }
> +
> +    for (i = 0; i < nr_entries; ++i) {
> +        const char *start_addr, *start_addr_val,
> +            *size, *size_val, *name, *name_val;
> +
> +        start_addr = libxl__device_model_xs_path(
> +            gc, dm_domid, domid, "/physmap/%s/start_addr", entries[i]);
> +        size = libxl__device_model_xs_path(
> +            gc, dm_domid, domid, "/physmap/%s/size", entries[i]);
> +        name = libxl__device_model_xs_path(
> +            gc, dm_domid, domid, "/physmap/%s/name", entries[i]);
> +
> +        if (!start_addr || !size || !name) { rc = ERROR_FAIL; goto out; }
> +
> +        start_addr_val = libxl__xs_read(gc, 0, start_addr);
> +        size_val = libxl__xs_read(gc, 0, size);
> +        name_val = libxl__xs_read(gc, 0, name);

XBT_NULL in libxl__xs_read please.

With those issues fixed:

Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

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