Re: [Xen-devel] [PATCH V5 29/32] xl: use "libxl-json" format

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-13 at 22:54 +0100, Wei Liu wrote:
> Before this change, xl stores domain configuration in "xl" format, which
> is in fact a verbatim copy of user supplied domain config.
> 
> As now libxl is able to handle domain configuration with "libxl-json"
> format, use that wherever possible.
> 
> Tests done so far (xl.{new,old} denotes xl with{,out} "xl_json"
> support):
> 
> 1. xl.new create then xl.new save, hexdump saved file: domain config
>    saved in JSON format
> 2. xl.new create, xl.new save then xl.old restore: failed on
>    mandatory flag check
> 3. xl.new create, xl.new save then xl.new restore: succeeded
> 4. xl.old create, xl.old save then xl.new restore: succeeded
> 5. xl.new create then local migrate, receiving end xl.new: succeeded
> 6. xl.old create then local migrate, receiving end xl.new: succeeded

I think xl.old create, xl.new save, xl.new restore would be interesting
(it should work I think)

For cases like #4 and #6 if you subsequently save the domain does it use
xl or libxl-json? What I'm asking is does the oldness persist forever or
is it laundered out (which in turn feeds into how long old support needs
to stay in the tree...)

> @@ -1787,13 +1770,10 @@ static int handle_domain_death(uint32_t *r_domid,
>          break;
>  
>      case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
> -        reload_domain_config(*r_domid, config_data, config_len);
>          restart = 2;
>          break;
>  
>      case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
> -        reload_domain_config(*r_domid, config_data, config_len);

Why is it not equally necessary to reload the JSON config at this point
if it exists?

(commit message should explain this, and in general anywhere that the
need to reload/refresh the config differs, because my expectation was of
a more straight forward substitution of reload_domain_config for the new
function).

> -
>          restart = 1;
>          /* fall-through */
>      case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
[...] 
> -    parse_config_data(config_source, config_data, config_len, &d_config);
> +    if (config_in_json) {
> +        char *config_c = config_data;
> +        if (config_c[config_len-1] != '\0') {
> +            config_c = xrealloc(config_c, config_len + 1);
> +            config_c[config_len] = '\0';

I think this is somewhere that arranging via the protocol that things
must be null terminated might make sense.

> +    /* If we're doing migration, the domain name was appended with
> +     * "--incoming" a few lines above. So we need to remove that
> +     * suffix in the stored configuration.

It's not possible to save this before we do that appending?

Or should we not be updating the domain config as we do the renaming at
the end?

> +     */
> +    if (migrate_fd >= 0) {
> +        libxl_domain_config d;
> +        int xlen = strlen("--incoming");
> +        int orig_len;
> +
> +        ret = libxl_load_domain_configuration(ctx, domid, &d);
> +        if (ret) {
> +            perror("cannot load config data");
> +            ret = ERROR_FAIL;
> +            goto error_out;
> +        }
> +
> +        orig_len = strlen(d.c_info.name);
> +        d.c_info.name = xrealloc(d.c_info.name, orig_len - xlen);
> +        d.c_info.name[orig_len - xlen] = 0;

Since the new name is always shorter you could do this simply by
sticking a \0 in the middle without needing to realloc.

> @@ -3102,22 +3119,18 @@ static void list_domains_details(const libxl_dominfo 
> *info, int nb_domain)
>          s = yajl_gen_status_ok;
>  
>      for (i = 0; i < nb_domain; i++) {
> +        libxl_domain_config_init(&d_config);
>          /* no detailed info available on dom0 */
>          if (info[i].domid == 0)
>              continue;
> -        rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, &len);
> +        rc = libxl_load_domain_configuration(ctx, info[i].domid, &d_config);

If a domain was created with xl.old will we not now fail to list details
of it here?

Ian.


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