Re: [PATCH 5/5] conf: checkpoint: Don't require <domain> when redefining checkpoints

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

On 12/2/20 11:13 AM, Peter Krempa wrote:
> The domain definition stored with a checkpoint isn't used currently
> apart from matching disks when creating a new checkpoints.
>
> As some users of the incremental backup API want to provide backups in
> offline mode under their control (obviously while compying with our

s/compying/complying

> documentation on how the on-disk state should be handled) and then want
> to define the checkpoint for live use, supplying a <domain> sub-element
> is overly complex and not actually needed by the code.
>
> Relax the restriction when re-defining a checkpoint so that <domain> is
> not necessary and add (alibistic) documentation saying that future
> actions may not work if it's missing.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>

>   docs/formatcheckpoint.rst  | 12 +++++++++---
>   src/conf/checkpoint_conf.c |  4 ----
>   2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/docs/formatcheckpoint.rst b/docs/formatcheckpoint.rst
> index f159f2a7a3..ff3f1e8c00 100644
> --- a/docs/formatcheckpoint.rst
> +++ b/docs/formatcheckpoint.rst
> @@ -1,3 +1,5 @@
> +.. role:: since
> +
>   Checkpoint XML format
>   =====================
>
> @@ -103,12 +105,16 @@ The top-level ``domaincheckpoint`` element may contain the following elements:
>      A readonly representation of the inactive `domain
>      configuration <formatdomain.html>`__ at the time the checkpoint was created.
>      This element may be omitted for output brevity by supplying the
> -   ``VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN`` flag, but the resulting XML is no
> -   longer viable for use with the ``VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE`` flag
> -   of ``virDomainCheckpointCreateXML()``. The domain will have
> +   ``VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN`` flag. The domain will have
>      security-sensitive information omitted unless the flag
>      ``VIR_DOMAIN_CHECKPOINT_XML_SECURE`` is provided on a read-write connection.
>
> +   ``virDomainCheckpointCreateXML()`` requires that the ``<domain>`` is present
> +   when used with ``VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE``.
> +   :since:`Since 7.0.0` the ``<domain>`` element can be omitted when redefining
> +   a checkpoint, but hypervisors may not support certain operations if it's
> +   missing.
> +
>   Examples
>   --------
>
> diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
> index 2071494d52..2df5e41495 100644
> --- a/src/conf/checkpoint_conf.c
> +++ b/src/conf/checkpoint_conf.c
> @@ -165,10 +165,6 @@ virDomainCheckpointDefParse(xmlXPathContextPtr ctxt,
>                                                       domainParseFlags);
>               if (!def->parent.dom)
>                   return NULL;
> -        } else {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("missing domain in checkpoint redefine"));
> -            return NULL;
>           }
>       } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0) {
>           return NULL;