[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 20/16] save: let qemu driver manipulate save files

On 07/21/2011 05:47 AM, Daniel P. Berrange wrote:
On Wed, Jul 20, 2011 at 02:54:26PM -0600, Eric Blake wrote:
* src/qemu/qemu_driver.c (qemuDomainSaveImageGetXMLDesc)
(qemuDomainSaveImageDefineXML): New functions.
(qemuDomainSaveImageOpen): Add parameter.
(qemuDomainRestoreFlags, qemuDomainObjRestore): Adjust clients.
(qemuDomainSaveInternal): Simplify array expansion.

+    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"));
+    }

This is what I was afraid of when I saw the API proposal. I fear that
this could make the use of the new API rather limited. I can easily
imagine 50%+ of end users wanting to the change the save image XML by
altering a disk path and getting a disk path which is longer.

We have one thing in our favor:

By default, virDomainSave stores the _active_ XML, rounded up to the next 4k boundary. But virDomainRestore only reads the _inactive_ XML. So my patch explicitly writes the inactive XML, which is typically much shorter than the active XML that was there originally (no <alias> tags, and so forth) - longer disk names are not a problem if they are still shorter than the length of the no-longer-present <alias> tags. To trigger this out-of-length error without breaking ABI, you have to try _really_ hard, such as making <description> much longer.

I don't think we should add an API like this unless we can come up
with a plan for addressing this problem which is generally going to

For in-place edits, I hold the driver lock for the entire time, so there is no chance that any other virDomainRestore can be called and see the state file in an inconsistent state.

But if we want to accommodate larger xml lengths, in spite of my above claim that we usually won't encounter them because of the difference in live vs. inactive shortening the length on average, then I think we have to:

mark the original file as in-use
drop driver lock
write the new longer header into a temp file
copy the old qemu data into the new file at the new offset
regain driver lock
rename the temp file over the original file

since the copy operation will be long-lived (no longer just 4k or 8k, but multiple megabytes, depending on how large the original save image was). Dropping the driver lock in the meantime means that another thread can call virDomainRestore using the same original file, which must fail because we are in the middle of altering it, hence the importance of being able to mark the original file in-use.

Thoughts on how to proceed?

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]