Re: [Xen-devel] [PATCH v1 10/10] xl: use libxl_retrieve_domain_configuration and JSON format

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-07-10 at 15:32 +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.
> 
> Now libxl provides a new API to retrieve domain configuration, switch to
> that new API, store configuration in JSON format.
> 
> Tests done so far (xl.{new,old} denotes xl with{,out} "libxl-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
> 
> Note that "xl" config is still supported and handled when restarting a
> domain. "xl" config file takes precedence over "libxl-json" in that
> case, so that user who uses "config-update" to store new config file
> won't have regression. All other scenarios (migration, domain listing
> etc.) now use the new API.
> 
> Lastly, print out warning when users invoke "config-update" to
> discourage them from using this command.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  docs/man/xl.pod.1         |    3 ++
>  tools/libxl/xl_cmdimpl.c  |  121 
> ++++++++++++++++++++++++++++-----------------
>  tools/libxl/xl_cmdtable.c |    4 +-
>  3 files changed, 82 insertions(+), 46 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index 30bd4bf..6b5baf4 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -187,6 +187,9 @@ immediate effect but will be applied when the guest is 
> next
>  restarted. This command is useful to ensure that runtime modifications
>  made to the guest will be preserved when the guest is restarted.
>  
> +Libxl now has better capability to handle domain configuration, avoid
> +using this command when possible.

Perhaps say:

        Libxl now has improved capabilities to handle dynamic domain
        configuration changes and will preserve any changes made a
        runtime when necessary. Therefore it should not normally be
        necessary to use this command any more.
        
Ian.

> +
>  I<configfile> has to be an absolute path to a file.
>  
>  B<OPTIONS>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 68df548..3ee490e 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -107,6 +107,8 @@ static const char migrate_report[]=
>     *            from target to source
>     */
>  
> +#define XL_MANDATORY_FLAG_JSON (1U << 0) /* config data is in JSON format */
> +#define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON)
>  struct save_file_header {
>      char magic[32]; /* savefileheader_magic */
>      /* All uint32_ts are in domain's byte order. */
> @@ -1699,21 +1701,40 @@ skip_vfb:
>  }
>  
>  static void reload_domain_config(uint32_t domid,
> -                                 uint8_t **config_data, int *config_len)
> +                                 libxl_domain_config *d_config)
>  {
> +    int rc;
>      uint8_t *t_data;
>      int ret, t_len;
> +    libxl_domain_config d_config_new;
>  
> +    /* In case user has used "config-update" to store a new config
> +     * file.
> +     */
>      ret = libxl_userdata_retrieve(ctx, domid, "xl", &t_data, &t_len);
> -    if (ret) {
> -        LOG("failed to retrieve guest configuration (rc=%d). "
> -            "reusing old configuration", ret);
> +    if (ret && errno != ENOENT) {
> +        LOG("\"xl\" file found but failed to load\n");

Perhaps "Saved \"xl\" configuration found ..."
> +    }
> +    if (t_len > 0) {
> +        LOG("\"xl\" config file found, use it\n");

"using"

I wonder if at this point we should delete the saved config.

IOW do we expect xl update-config to be one shot or to persist. I think
the one shot is the only sensible answer and it doesn't conflict with
the previous implementation since in the old implementation the saved
config became the new stored config.

Ian.


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