On 07/22/2011 12:21 AM, Eric Blake wrote:
The goal here is that save-image-dumpxml fed back to save
image-define without changing the save file; anywhere that
this is not the case is probably a bug in domain_conf.c.
* src/qemu/qemu_driver.c (qemuDomainSaveImageGetXMLDesc)
(qemuDomainSaveImageDefineXML): New functions.
(qemuDomainSaveImageOpen): Add parameter.
(qemuDomainRestoreFlags, qemuDomainObjRestore): Adjust clients.
---
v3: short cut exit with special value if no edits need to be made
src/qemu/qemu_driver.c | 120 ++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 112 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b1df6c..f1a4e8a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3706,29 +3706,32 @@ cleanup:
return ret;
}
-static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
+/* Return -1 on failure, -2 if edit was specified but xmlin does not
+ * represent any changes, and opened fd on all other success. */
+static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
qemuDomainSaveImageOpen(struct qemud_driver *driver,
const char *path,
virDomainDefPtr *ret_def,
struct qemud_save_header *ret_header,
bool bypass_cache, virFileDirectFdPtr *directFd,
- const char *xmlin)
+ const char *xmlin, bool edit)
{
int fd;
struct qemud_save_header header;
char *xml = NULL;
virDomainDefPtr def = NULL;
- int directFlag = 0;
+ int oflags = edit ? O_RDWR : O_RDONLY;
if (bypass_cache) {
- directFlag = virFileDirectFdFlag();
+ int directFlag = virFileDirectFdFlag();
if (directFlag< 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("bypass cache unsupported by this system"));
goto error;
}
+ oflags |= directFlag;
}
- if ((fd = virFileOpenAs(path, O_RDONLY | directFlag, 0,
+ if ((fd = virFileOpenAs(path, oflags, 0,
getuid(), getgid(), 0))< 0) {
if ((fd != -EACCES&& fd != -EPERM) ||
driver->user == getuid()) {
@@ -3739,7 +3742,7 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
/* Opening as root failed, but qemu runs as a different user
* that might have better luck. */
- if ((fd = virFileOpenAs(path, O_RDONLY | directFlag, 0,
+ if ((fd = virFileOpenAs(path, oflags, 0,
driver->user, driver->group,
VIR_FILE_OPEN_AS_UID))< 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
@@ -3791,6 +3794,15 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
goto error;
}
+ if (edit&& STREQ(xml, xmlin)) {
+ VIR_FREE(xml);
+ if (VIR_CLOSE(fd)< 0) {
+ virReportSystemError(errno, _("cannot close file: %s"), path);
+ goto error;
+ }
+ return -2;
+ }
So "unchanged" is indicated with a < 0 return code, but that only
happens when edit == true, and only one of the callers does that (and
properly handles the special case).
+
/* Create a domain from this XML */
if (!(def = virDomainDefParseString(driver->caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
@@ -3949,7 +3961,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
fd = qemuDomainSaveImageOpen(driver, path,&def,&header,
(flags& VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
-&directFd, dxml);
+&directFd, dxml, false);
if (fd< 0)
goto cleanup;
@@ -3995,6 +4007,96 @@ qemuDomainRestore(virConnectPtr conn,
return qemuDomainRestoreFlags(conn, path, NULL, 0);
}
+static char *
+qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = conn->privateData;
+ char *ret = NULL;
+ virDomainDefPtr def = NULL;
+ int fd = -1;
+ struct qemud_save_header header;
+
+ /* We only take subset of virDomainDefFormat flags. */
+ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
+
+ qemuDriverLock(driver);
+
+ fd = qemuDomainSaveImageOpen(driver, path,&def,&header, false, NULL,
+ NULL, false);
+
+ if (fd< 0)
+ goto cleanup;
+
+ ret = qemuDomainDefFormatXML(driver, def, flags);
+
+cleanup:
+ virDomainDefFree(def);
+ VIR_FORCE_CLOSE(fd);
+ qemuDriverUnlock(driver);
+ return ret;
+}
+
+static int
+qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
+ const char *dxml, unsigned int flags)
+{
+ struct qemud_driver *driver = conn->privateData;
+ int ret = -1;
+ virDomainDefPtr def = NULL;
+ int fd = -1;
+ struct qemud_save_header header;
+ char *xml = NULL;
+ size_t len;
+
+ virCheckFlags(0, -1);
+
+ qemuDriverLock(driver);
+
+ fd = qemuDomainSaveImageOpen(driver, path,&def,&header, false, NULL,
+ dxml, true);
+
+ if (fd< 0) {
+ /* Check for special case of no change needed. */
+ if (fd == -2)
+ ret = 0;
i.e. right here.
+ goto cleanup;
+ }
+
+ xml = qemuDomainDefFormatXML(driver, def, VIR_DOMAIN_XML_SECURE);
+ if (!xml)
+ goto cleanup;
+ len = strlen(xml) + 1;
+
+ if (len> header.xml_len) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("new xml too large to fit in file"));
+ }
Did you forget a goto cleanup there?
+ if (VIR_EXPAND_N(xml, len, header.xml_len - len)< 0) {
because if len > header.xml_len, that's going to be a *very large*
number :-)
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (lseek(fd, sizeof(header), SEEK_SET) != sizeof(header)) {
+ virReportSystemError(errno, _("cannot seek in '%s'"), path);
+ goto cleanup;
+ }
+ if (safewrite(fd, xml, len) != len ||
+ VIR_CLOSE(fd)< 0) {
+ virReportSystemError(errno, _("failed to write xml to '%s'"), path);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ virDomainDefFree(def);
+ VIR_FORCE_CLOSE(fd);
+ VIR_FREE(xml);
+ qemuDriverUnlock(driver);
+ return ret;
+}
+
static int
qemuDomainObjRestore(virConnectPtr conn,
struct qemud_driver *driver,
@@ -4009,7 +4111,7 @@ qemuDomainObjRestore(virConnectPtr conn,
virFileDirectFdPtr directFd = NULL;
fd = qemuDomainSaveImageOpen(driver, path,&def,&header,
- bypass_cache,&directFd, NULL);
+ bypass_cache,&directFd, NULL, false);
if (fd< 0)
goto cleanup;
@@ -9070,6 +9172,8 @@ static virDriver qemuDriver = {
.domainSaveFlags = qemuDomainSaveFlags, /* 0.9.4 */
.domainRestore = qemuDomainRestore, /* 0.2.0 */
.domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */
+ .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */
+ .domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */
.domainCoreDump = qemudDomainCoreDump, /* 0.7.0 */
.domainScreenshot = qemuDomainScreenshot, /* 0.9.2 */
.domainSetVcpus = qemuDomainSetVcpus, /* 0.4.4 */
ACK with the goto cleanup added in when the new length is too big to fit
(unless I've misunderstood the code).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list