[libvirt] [PATCH v2 0/4] test_driver: implement virDomainSaveImageGetXMLDesc and virDomainSaveImageDefineXML
Changed in v2: * in testDomainSaveImageDefineXML check first that a saved image already exists before parsing the new file * reordered the arguments of testDomainSaveImageWrite in order for its signature to be consistent with testDomainSaveImageOpen While implementing virDomainSaveImageGetXMLDesc and virDomainSaveImageDefineXML for the test driver, I realized that there exists already code for saving and loading test images which can be reused. However, it needed to be extracted from testDomainSaveFlags and testDomainRestoreFlags into separate functions. The new functions are inspired by the corresponding QEMU driver code where e.g. qemuDomainSaveImageOpen serves as a helper used by other functions. This series of patches initially extracts the code mentioned above into separate functions and then provides the test driver with implementations for virDomainSaveImageGetXMLDesc and virDomainSaveImageDefineXML which make use of the newly introduced functions. Ilias Stamatis (4): test_driver: extract image saving code into a separate function test_driver: extract image loading code into a separate function test_driver: implement virDomainSaveImageDefineXML test_driver: implement virDomainSaveImageGetXMLDesc src/test/test_driver.c | 293 ++++++++++++++++++++++++++++------------- 1 file changed, 205 insertions(+), 88 deletions(-) -- 2.21.0
Extracting the code logic for writing a test image to disk from testDomainSaveFlags into a separate function, allows us to reuse this code in other functions such as testDomainSaveImageDefineXML. Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 115 +++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 45 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1aa79ce898..3331ed5495 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1999,75 +1999,107 @@ testDomainGetTime(virDomainPtr dom ATTRIBUTE_UNUSED, #define TEST_SAVE_MAGIC "TestGuestMagic" -static int -testDomainSaveFlags(virDomainPtr domain, const char *path, - const char *dxml, unsigned int flags) + +/** + * testDomainSaveImageWrite: + * @driver: test driver data + * @def: domain definition whose XML will be stored in the image + * @path: path to the saved image + * + * Returns true on success, else false. + */ +static bool +testDomainSaveImageWrite(testDriverPtr driver, + const char *path, + virDomainDefPtr def) { - testDriverPtr privconn = domain->conn->privateData; - int fd = -1; int len; - virDomainObjPtr privdom; - virObjectEventPtr event = NULL; - int ret = -1; + int fd = -1; VIR_AUTOFREE(char *) xml = NULL; - virCheckFlags(0, -1); - if (dxml) { - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", - _("xml modification unsupported")); - return -1; - } - - - if (!(privdom = testDomObjFromDomain(domain))) - goto cleanup; - - if (virDomainObjCheckActive(privdom) < 0) - goto cleanup; - - xml = virDomainDefFormat(privdom->def, privconn->caps, - VIR_DOMAIN_DEF_FORMAT_SECURE); + xml = virDomainDefFormat(def, driver->caps, VIR_DOMAIN_DEF_FORMAT_SECURE); if (xml == NULL) { virReportSystemError(errno, _("saving domain '%s' failed to allocate space for metadata"), - domain->name); - goto cleanup; + def->name); + goto error; } if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { virReportSystemError(errno, _("saving domain '%s' to '%s': open failed"), - domain->name, path); - goto cleanup; + def->name, path); + goto error; } - len = strlen(xml); + if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) { virReportSystemError(errno, _("saving domain '%s' to '%s': write failed"), - domain->name, path); - goto cleanup; + def->name, path); + goto error; } + + len = strlen(xml); if (safewrite(fd, (char*)&len, sizeof(len)) < 0) { virReportSystemError(errno, _("saving domain '%s' to '%s': write failed"), - domain->name, path); - goto cleanup; + def->name, path); + goto error; } + if (safewrite(fd, xml, len) < 0) { virReportSystemError(errno, _("saving domain '%s' to '%s': write failed"), - domain->name, path); - goto cleanup; + def->name, path); + goto error; } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("saving domain '%s' to '%s': write failed"), - domain->name, path); - goto cleanup; + def->name, path); + goto error; + } + + return true; + + error: + /* Don't report failure in close or unlink, because + * in either case we're already in a failure scenario + * and have reported an earlier error */ + VIR_FORCE_CLOSE(fd); + unlink(path); + + return false; +} + + +static int +testDomainSaveFlags(virDomainPtr domain, const char *path, + const char *dxml, unsigned int flags) +{ + testDriverPtr privconn = domain->conn->privateData; + virDomainObjPtr privdom; + virObjectEventPtr event = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (dxml) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("xml modification unsupported")); + return -1; } - fd = -1; + + if (!(privdom = testDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainObjCheckActive(privdom) < 0) + goto cleanup; + + if (!testDomainSaveImageWrite(privconn, path, privdom->def)) + goto cleanup; testDomainShutdownState(domain, privdom, VIR_DOMAIN_SHUTOFF_SAVED); event = virDomainEventLifecycleNewFromObj(privdom, @@ -2079,13 +2111,6 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, ret = 0; cleanup: - /* Don't report failure in close or unlink, because - * in either case we're already in a failure scenario - * and have reported an earlier error */ - if (ret != 0) { - VIR_FORCE_CLOSE(fd); - unlink(path); - } virDomainObjEndAPI(&privdom); virObjectEventStateQueue(privconn->eventState, event); return ret; -- 2.21.0
Extracting the code logic for opening and parsing a test image from testDomainRestoreFlags into a separate function, allows us to reuse this code in other functions such as testDomainSaveImageGetXMLDesc. Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 116 ++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 43 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 3331ed5495..1b92cb43dd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2075,6 +2075,78 @@ testDomainSaveImageWrite(testDriverPtr driver, } +/** + * testDomainSaveImageOpen: + * @driver: test driver data + * @path: path of the saved image + * @ret_def: returns domain definition created from the XML stored in the image + * + * Returns the opened fd of the save image file and fills ret_def on success. + * Returns -1, on error. + */ +static int ATTRIBUTE_NONNULL(3) +testDomainSaveImageOpen(testDriverPtr driver, + const char *path, + virDomainDefPtr *ret_def) +{ + char magic[15]; + int fd = -1; + int len; + virDomainDefPtr def = NULL; + VIR_AUTOFREE(char *) xml = NULL; + + if ((fd = open(path, O_RDONLY)) < 0) { + virReportSystemError(errno, _("cannot read domain image '%s'"), path); + goto error; + } + + if (saferead(fd, magic, sizeof(magic)) != sizeof(magic)) { + virReportSystemError(errno, _("incomplete save header in '%s'"), path); + goto error; + } + + if (memcmp(magic, TEST_SAVE_MAGIC, sizeof(magic))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("mismatched header magic")); + goto error; + } + + if (saferead(fd, (char*)&len, sizeof(len)) != sizeof(len)) { + virReportSystemError(errno, + _("failed to read metadata length in '%s'"), + path); + goto error; + } + + if (len < 1 || len > 8192) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("length of metadata out of range")); + goto error; + } + + if (VIR_ALLOC_N(xml, len+1) < 0) + goto error; + + if (saferead(fd, xml, len) != len) { + virReportSystemError(errno, _("incomplete metadata in '%s'"), path); + goto error; + } + xml[len] = '\0'; + + if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) + goto error; + + VIR_STEAL_PTR(*ret_def, def); + return fd; + + error: + virDomainDefFree(def); + VIR_FORCE_CLOSE(fd); + return -1; +} + + static int testDomainSaveFlags(virDomainPtr domain, const char *path, const char *dxml, unsigned int flags) @@ -2130,14 +2202,11 @@ testDomainRestoreFlags(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - char magic[15]; int fd = -1; - int len; virDomainDefPtr def = NULL; virDomainObjPtr dom = NULL; virObjectEventPtr event = NULL; int ret = -1; - VIR_AUTOFREE(char *) xml = NULL; virCheckFlags(0, -1); if (dxml) { @@ -2146,46 +2215,7 @@ testDomainRestoreFlags(virConnectPtr conn, return -1; } - if ((fd = open(path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot read domain image '%s'"), - path); - goto cleanup; - } - if (saferead(fd, magic, sizeof(magic)) != sizeof(magic)) { - virReportSystemError(errno, - _("incomplete save header in '%s'"), - path); - goto cleanup; - } - if (memcmp(magic, TEST_SAVE_MAGIC, sizeof(magic))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("mismatched header magic")); - goto cleanup; - } - if (saferead(fd, (char*)&len, sizeof(len)) != sizeof(len)) { - virReportSystemError(errno, - _("failed to read metadata length in '%s'"), - path); - goto cleanup; - } - if (len < 1 || len > 8192) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("length of metadata out of range")); - goto cleanup; - } - if (VIR_ALLOC_N(xml, len+1) < 0) - goto cleanup; - if (saferead(fd, xml, len) != len) { - virReportSystemError(errno, - _("incomplete metadata in '%s'"), path); - goto cleanup; - } - xml[len] = '\0'; - - def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, - NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (!def) + if ((fd = testDomainSaveImageOpen(privconn, path, &def)) < 0) goto cleanup; if (testDomainGenerateIfnames(def) < 0) -- 2.21.0
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@gmail.com> --- src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1b92cb43dd..906c9d5365 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2255,6 +2255,42 @@ 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; + int fd = -1; + virDomainDefPtr def = NULL; + virDomainDefPtr newdef = NULL; + testDriverPtr privconn = conn->privateData; + + virCheckFlags(VIR_DOMAIN_SAVE_RUNNING | + VIR_DOMAIN_SAVE_PAUSED, -1); + + if ((fd = testDomainSaveImageOpen(privconn, path, &def)) < 0) + goto cleanup; + + if ((newdef = virDomainDefParseString(dxml, privconn->caps, privconn->xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + goto cleanup; + + if (!testDomainSaveImageWrite(privconn, path, newdef)) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + virDomainDefFree(def); + virDomainDefFree(newdef); + return ret; +} + + static int testDomainCoreDumpWithFormat(virDomainPtr domain, const char *to, unsigned int dumpformat, @@ -7077,6 +7113,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainSaveFlags = testDomainSaveFlags, /* 0.9.4 */ .domainRestore = testDomainRestore, /* 0.3.2 */ .domainRestoreFlags = testDomainRestoreFlags, /* 0.9.4 */ + .domainSaveImageDefineXML = testDomainSaveImageDefineXML, /* 5.5.0 */ .domainCoreDump = testDomainCoreDump, /* 0.3.2 */ .domainCoreDumpWithFormat = testDomainCoreDumpWithFormat, /* 1.2.3 */ .domainSetVcpus = testDomainSetVcpus, /* 0.1.4 */ -- 2.21.0
On Mon, Jun 10, 2019 at 11:05:00AM +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@gmail.com> --- src/test/test_driver.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1b92cb43dd..906c9d5365 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2255,6 +2255,42 @@ 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; + int fd = -1; + virDomainDefPtr def = NULL; + virDomainDefPtr newdef = NULL; + testDriverPtr privconn = conn->privateData; + + virCheckFlags(VIR_DOMAIN_SAVE_RUNNING | + VIR_DOMAIN_SAVE_PAUSED, -1); + + if ((fd = testDomainSaveImageOpen(privconn, path, &def)) < 0) + goto cleanup;
Since we're not going to use @fd anymore, I'd move VIR_FORCE_CLOSE(fd) here to be more explicit. I'll make the change before pushing: Reviewed-by: Erik Skultety <eskultet@redhat.com>
+ + if ((newdef = virDomainDefParseString(dxml, privconn->caps, privconn->xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + goto cleanup; + + if (!testDomainSaveImageWrite(privconn, path, newdef)) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + virDomainDefFree(def); + virDomainDefFree(newdef); + return ret; +} + + static int testDomainCoreDumpWithFormat(virDomainPtr domain, const char *to, unsigned int dumpformat, @@ -7077,6 +7113,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainSaveFlags = testDomainSaveFlags, /* 0.9.4 */ .domainRestore = testDomainRestore, /* 0.3.2 */ .domainRestoreFlags = testDomainRestoreFlags, /* 0.9.4 */ + .domainSaveImageDefineXML = testDomainSaveImageDefineXML, /* 5.5.0 */ .domainCoreDump = testDomainCoreDump, /* 0.3.2 */ .domainCoreDumpWithFormat = testDomainCoreDumpWithFormat, /* 1.2.3 */ .domainSetVcpus = testDomainSetVcpus, /* 0.1.4 */ -- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 906c9d5365..ebf6f84b58 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2291,6 +2291,30 @@ testDomainSaveImageDefineXML(virConnectPtr conn, } +static char * +testDomainSaveImageGetXMLDesc(virConnectPtr conn, + const char *path, + unsigned int flags) +{ + int fd = -1; + char *ret = NULL; + virDomainDefPtr def = NULL; + testDriverPtr privconn = conn->privateData; + + virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL); + + if ((fd = testDomainSaveImageOpen(privconn, path, &def)) < 0) + goto cleanup; + + ret = virDomainDefFormat(def, privconn->caps, VIR_DOMAIN_DEF_FORMAT_SECURE); + + cleanup: + virDomainDefFree(def); + VIR_FORCE_CLOSE(fd); + return ret; +} + + static int testDomainCoreDumpWithFormat(virDomainPtr domain, const char *to, unsigned int dumpformat, @@ -7114,6 +7138,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainRestore = testDomainRestore, /* 0.3.2 */ .domainRestoreFlags = testDomainRestoreFlags, /* 0.9.4 */ .domainSaveImageDefineXML = testDomainSaveImageDefineXML, /* 5.5.0 */ + .domainSaveImageGetXMLDesc = testDomainSaveImageGetXMLDesc, /* 5.5.0 */ .domainCoreDump = testDomainCoreDump, /* 0.3.2 */ .domainCoreDumpWithFormat = testDomainCoreDumpWithFormat, /* 1.2.3 */ .domainSetVcpus = testDomainSetVcpus, /* 0.1.4 */ -- 2.21.0
On Mon, Jun 10, 2019 at 11:04:57AM +0200, Ilias Stamatis wrote:
Changed in v2:
* in testDomainSaveImageDefineXML check first that a saved image already exists before parsing the new file * reordered the arguments of testDomainSaveImageWrite in order for its signature to be consistent with testDomainSaveImageOpen
While implementing virDomainSaveImageGetXMLDesc and virDomainSaveImageDefineXML for the test driver, I realized that there exists already code for saving and loading test images which can be reused. However, it needed to be extracted from testDomainSaveFlags and testDomainRestoreFlags into separate functions. The new functions are inspired by the corresponding QEMU driver code where e.g. qemuDomainSaveImageOpen serves as a helper used by other functions.
This series of patches initially extracts the code mentioned above into separate functions and then provides the test driver with implementations for virDomainSaveImageGetXMLDesc and virDomainSaveImageDefineXML which make use of the newly introduced functions.
Ilias Stamatis (4): test_driver: extract image saving code into a separate function test_driver: extract image loading code into a separate function test_driver: implement virDomainSaveImageDefineXML test_driver: implement virDomainSaveImageGetXMLDesc
src/test/test_driver.c | 293 ++++++++++++++++++++++++++++------------- 1 file changed, 205 insertions(+), 88 deletions(-)
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety -
Ilias Stamatis