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