On 07/22/2011 12:21 AM, Eric Blake wrote:
With this, it is possible to update the path to a disk backing
image on either the save or restore action, without having to
binary edit the XML embedded in the state file.
This also modifies virDomainSave to output a smaller xml (only
the inactive xml, which is all the more virDomainRestore parses),
while still guaranteeing padding for most typical abi-compatible
xml replacements, necessary so that the next patch for
virDomainSaveImageDefineXML will not cause unnecessary
modifications to the save image file.
* src/qemu/qemu_driver.c (qemuDomainSaveInternal): Add parameter,
only use inactive state, and guarantee padding.
(qemuDomainSaveImageOpen): Add parameter.
(qemuDomainSaveFlags, qemuDomainManagedSave)
(qemuDomainRestoreFlags, qemuDomainObjRestore): Update callers.
---
v3: change virDomainSave to always output minimal information,
but with fixed padding added, so that save file modification
will always be more likely to succeed,
"always be more likely". Heh.
Looking at this problem from the outside, it seems that if we wanted a
100% reliable solution, we would need to introduce the idea of a linked
header, which can be continued at the end of the file (of course that
wouldn't work if there are ever cases where the file is being read from
a pipe, and we can't seek, and it's entirely possible that 1024 is
always enough extra to ensure everything works).
and not generate quite
as many xml differences on round trips.
src/qemu/qemu_driver.c | 109 +++++++++++++++++++++++++++++------------------
1 files changed, 67 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1401967..2b1df6c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2193,7 +2193,7 @@ qemuCompressProgramName(int compress)
static int
qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
virDomainObjPtr vm, const char *path,
- int compressed, bool bypass_cache)
+ int compressed, bool bypass_cache, const char *xmlin)
{
char *xml = NULL;
struct qemud_save_header header;
@@ -2204,7 +2204,9 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
qemuDomainObjPrivatePtr priv;
struct stat sb;
bool is_reg = false;
+ size_t len;
unsigned long long offset;
+ unsigned long long pad;
int fd = -1;
uid_t uid = getuid();
gid_t gid = getgid();
@@ -2239,15 +2241,54 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
}
}
- /* Get XML for the domain */
- xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE);
+ /* Get XML for the domain. Restore needs only the inactive xml,
+ * including secure. We should get the same result whether xmlin
+ * is NULL or whether it was the live xml of the domain moments
+ * before. */
+ if (xmlin) {
+ virDomainDefPtr def = NULL;
+
+ if (!(def = virDomainDefParseString(driver->caps, xmlin,
+ QEMU_EXPECTED_VIRT_TYPES,
+ VIR_DOMAIN_XML_INACTIVE))) {
+ goto endjob;
+ }
+ if (!virDomainDefCheckABIStability(vm->def, def)) {
+ virDomainDefFree(def);
+ goto endjob;
+ }
+ xml = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE);
+ } else {
+ xml = virDomainDefFormat(vm->def, (VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_SECURE));
+ }
if (!xml) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("failed to get domain xml"));
goto endjob;
}
- header.xml_len = strlen(xml) + 1;
+ len = strlen(xml) + 1;
+ offset = sizeof(header) + len;
+
+ /* Due to way we append QEMU state on our header with dd,
+ * we need to ensure there's a 512 byte boundary. Unfortunately
+ * we don't have an explicit offset in the header, so we fake
+ * it by padding the XML string with NUL bytes. Additionally,
+ * we want to ensure that virDomainSaveImageDefineXML can supply
+ * slightly larger XML, so we add a miminum padding prior to
+ * rounding out to page boundaries.
+ */
+ pad = 1024;
+ pad += (QEMU_MONITOR_MIGRATE_TO_FILE_BS -
+ ((offset + pad) % QEMU_MONITOR_MIGRATE_TO_FILE_BS));
+ if (VIR_EXPAND_N(xml, len, pad)< 0) {
+ virReportOOMError();
+ goto endjob;
+ }
+ offset += pad;
+ header.xml_len = len;
+ /* Obtain the file handle. */
/* path might be a pre-existing block dev, in which case
* we need to skip the create step, and also avoid unlink
* in the failure case */
@@ -2268,29 +2309,6 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
}
}
- offset = sizeof(header) + header.xml_len;
-
- /* Due to way we append QEMU state on our header with dd,
- * we need to ensure there's a 512 byte boundary. Unfortunately
- * we don't have an explicit offset in the header, so we fake
- * it by padding the XML string with NULLs.
- */
- if (offset % QEMU_MONITOR_MIGRATE_TO_FILE_BS) {
- unsigned long long pad =
- QEMU_MONITOR_MIGRATE_TO_FILE_BS -
- (offset % QEMU_MONITOR_MIGRATE_TO_FILE_BS);
-
- if (VIR_REALLOC_N(xml, header.xml_len + pad)< 0) {
- virReportOOMError();
- goto endjob;
- }
- memset(xml + header.xml_len, 0, pad);
- offset += pad;
- header.xml_len += pad;
- }
-
- /* Obtain the file handle. */
-
/* First try creating the file as root */
if (bypass_cache) {
directFlag = virFileDirectFdFlag();
@@ -2461,11 +2479,6 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
virDomainObjPtr vm = NULL;
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE, -1);
- if (dxml) {
- qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("xml modification unsupported"));
- return -1;
- }
qemuDriverLock(driver);
@@ -2503,7 +2516,8 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
}
ret = qemuDomainSaveInternal(driver, dom, vm, path, compressed,
- (flags& VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0);
+ (flags& VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
+ dxml);
vm = NULL;
cleanup:
@@ -2567,7 +2581,8 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
compressed = QEMUD_SAVE_FORMAT_RAW;
ret = qemuDomainSaveInternal(driver, dom, vm, name, compressed,
- (flags& VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0);
+ (flags& VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
+ NULL);
vm = NULL;
cleanup:
@@ -3696,7 +3711,8 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
const char *path,
virDomainDefPtr *ret_def,
struct qemud_save_header *ret_header,
- bool bypass_cache, virFileDirectFdPtr *directFd)
+ bool bypass_cache, virFileDirectFdPtr *directFd,
+ const char *xmlin)
{
int fd;
struct qemud_save_header header;
@@ -3780,6 +3796,20 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver,
QEMU_EXPECTED_VIRT_TYPES,
VIR_DOMAIN_XML_INACTIVE)))
(I've never noticed before that the diff shows the old signature of a
function if its signature has been changed as a part of the current
patch. A bit confusing for the human, but I'm sure doing it the other
way would make it confusing for the machine :-)
goto error;
+ if (xmlin) {
+ virDomainDefPtr def2 = NULL;
+
+ if (!(def2 = virDomainDefParseString(driver->caps, xmlin,
+ QEMU_EXPECTED_VIRT_TYPES,
+ VIR_DOMAIN_XML_INACTIVE)))
+ goto error;
+ if (!virDomainDefCheckABIStability(def, def2)) {
+ virDomainDefFree(def2);
+ goto error;
+ }
+ virDomainDefFree(def);
+ def = def2;
+ }
VIR_FREE(xml);
@@ -3914,17 +3944,12 @@ qemuDomainRestoreFlags(virConnectPtr conn,
virFileDirectFdPtr directFd = NULL;
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE, -1);
- if (dxml) {
- qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("xml modification unsupported"));
- return -1;
- }
qemuDriverLock(driver);
fd = qemuDomainSaveImageOpen(driver, path,&def,&header,
(flags& VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
-&directFd);
+&directFd, dxml);
if (fd< 0)
goto cleanup;
@@ -3984,7 +4009,7 @@ qemuDomainObjRestore(virConnectPtr conn,
virFileDirectFdPtr directFd = NULL;
fd = qemuDomainSaveImageOpen(driver, path,&def,&header,
- bypass_cache,&directFd);
+ bypass_cache,&directFd, NULL);
if (fd< 0)
goto cleanup;
ACK.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list