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@xxxxxxxxx> > --- > 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@xxxxxxxxxx> > + 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list