[libvirt] [PATCHv2 16/16] save: support qemu modifying xml on domain save/restore

[Eric Blake]

On 07/21/2011 05:38 AM, Daniel P. Berrange wrote:
> On Tue, Jul 19, 2011 at 10:20:39PM -0600, 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.
>>
>> * src/qemu/qemu_driver.c (qemuDomainSaveInternal)
>> (qemuDomainSaveImageOpen): Add parameter.
>> (qemuDomainSaveFlags, qemuDomainManagedSave)
>>

>>   src/qemu/qemu_driver.c |   56 +++++++++++++++++++++++++++++++++--------------
>>   1 files changed, 39 insertions(+), 17 deletions(-)
>
>
> Daniel

Can I assume you merely forgot to list ACK here?

Meanwhile, as discussed on IRC, I need to send a v3 of this patch. 
Pre-patch, virDomainSave saves the live xml state, while 
virDomainRestore only parses the inactive xml state.  Which means that 
you've got some built-in padding for the later virDomainSaveImage API, 
but also means that a virDomainSaveImageGetXMLDesc followed by 
virDomainSaveImageDefineXML _changes_ the output file, even if xml is 
untouched.  Meanwhile, v2 of this patch makes it so that if you use the 
dxml argument, only the inactive xml state is saved, but if you omit the 
dxml argument, the active state is still saved.  You should get the same 
file whether you passed dxml as NULL or passed it as the live xml from 
virDomainGetXMLDesc.  So the conclusion is that the xml in the state 
file should always be written as the inactive version (since we already 
know that virDomainRestore only parses the inactive portions of the 
xml), rather than as the live version as is done now.

Next, realize that the current code leaves us the built-in padding of 
the difference in size between active and inactive state, so that 
virDomainSaveImageDefineXML can use longer xml, but still not run into 
length problems, because it is merely consuming that built-in padding. 
But after v2 of this patch, if you use the dxml argument, you no longer 
have that padding - while you still have padding due to the state file 
rounding up to the next 4k boundary, an xml that is already awfully 
close to 4k in length will be more likely to fail due to the length 
change on the state file redefine attempt.

Therefore, my proposal is to make virDomainSaveFlags always save the 
inactive xml to begin with, but to also provide a sane amount of padding 
(fixed width of 2000 bytes? 50% more length than the original XML? pad 
all the way out to the RPC maximum xml size?), at which point we no 
longer have to worry quite as much about replacing the xml running out 
of space on common use cases, as well as guaranteeing idempotent state 
files across the dumpxml/define sequence with no xml changes.

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