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

[Eric Blake]

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
> work.

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org