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

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

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(-)


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 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]