[libvirt] [PATCH 3/4] test_driver: implement virDomainSaveImageDefineXML

[Erik Skultety]

On Wed, May 29, 2019 at 02:22:58PM +0200, Ilias Stamatis wrote:
> Updates the existing image stored in @path, in case @dxml contains valid
> XML supported by the fake host.
>
> Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> ---
>  src/test/test_driver.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index ce32570b75..b0195ac63d 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2228,6 +2228,33 @@ testDomainRestore(virConnectPtr conn,
>      return testDomainRestoreFlags(conn, path, NULL, 0);
>  }
>
> +static int
> +testDomainSaveImageDefineXML(virConnectPtr conn,
> +                             const char *path,
> +                             const char *dxml,
> +                             unsigned int flags)
> +{
> +    int ret = -1;
> +    virDomainDefPtr def = NULL;
> +    testDriverPtr privconn = conn->privateData;
> +
> +    virCheckFlags(VIR_DOMAIN_SAVE_RUNNING |
> +                  VIR_DOMAIN_SAVE_PAUSED, -1);
> +

In essence I agree. I'm just wondering now that you extracted the common code
in the previous patches whether we should come one step closer to how the real
API behaves, IOW if the image given by @path doesn't exist, we naturally fail
to open it, the current approach within the test driver is that you parse
@dxml, open the image and write the new XML - if the image didn't previously
exist, it is automatically created. Again, depends on what we're trying to
achieve with the test driver, here I'd expect the app devs to follow a sequence
of API calls, so they would want to call Save first, so that SaveImageDefineXML
can be used subsequently.

So ^this is meant to be kind of a discussion starter, code-wise, I'm also okay
with what you have here, so:
Reviewed-by: Erik Skultety <eskultet at redhat.com>


> +    if ((def = virDomainDefParseString(dxml, privconn->caps, privconn->xmlopt, NULL,
> +                                       VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL)
> +        goto cleanup;
> +
> +    if (!testDomainSaveImageWrite(privconn, def, path))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    virDomainDefFree(def);
> +    return ret;
> +}
> +
>  static int testDomainCoreDumpWithFormat(virDomainPtr domain,
>                                          const char *to,
>                                          unsigned int dumpformat,
> @@ -7010,6 +7037,7 @@ static virHypervisorDriver testHypervisorDriver = {
>      .domainSaveFlags = testDomainSaveFlags, /* 0.9.4 */
>      .domainRestore = testDomainRestore, /* 0.3.2 */
>      .domainRestoreFlags = testDomainRestoreFlags, /* 0.9.4 */
> +    .domainSaveImageDefineXML = testDomainSaveImageDefineXML, /* 5.4.0 */

This would be 5.5.0

Erik