[libvirt] [PATCH v2 0/4] test_driver: implement virDomainSaveImageGetXMLDesc and virDomainSaveImageDefineXML
4:04 a.m.
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
4:04 a.m.
New subject: [libvirt] [PATCH v2 1/4] test_driver: extract image saving code into a separate function
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(a)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
4:04 a.m.
New subject: [libvirt] [PATCH v2 2/4] test_driver: extract image loading code into a separate function
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(a)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
4:05 a.m.
New subject: [libvirt] [PATCH v2 3/4] test_driver: implement virDomainSaveImageDefineXML
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(a)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
10:31 a.m.
New subject: [libvirt] [PATCH v2 3/4] test_driver: implement virDomainSaveImageDefineXML
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(a)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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:05 a.m.
New subject: [libvirt] [PATCH v2 4/4] test_driver: implement virDomainSaveImageGetXMLDesc
Signed-off-by: Ilias Stamatis <stamatis.iliass(a)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
10:31 a.m.
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(a)redhat.com>
1956
days inactive
1957
days old
6 comments
2 participants
participants (2)
-
Erik Skultety -
Ilias Stamatis