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

["Daniel P. Berrange" <berrange redhat com>]

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.
> ---
> 
> Tested via:
> 
> virsh save dom dom.save
> virsh save-image-dumpxml dom.save > dom.xml
> edit dom.xml
> virsh save-image-define dom.save dom.xml
> virsh save-image-edit dom.save
> virsh restore dom.save
> 
> and the edits in dom.xml showed up in my editor, as well as
> on the restore.
> 
>  src/qemu/qemu_driver.c |  111 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 100 insertions(+), 11 deletions(-)

> +static int
> +qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
> +                             const char *dxml, unsigned int flags)
> +{
> +    struct qemud_driver *driver = conn->privateData;
> +    int ret = -1;
> +    virDomainDefPtr def = NULL;
> +    int fd = -1;
> +    struct qemud_save_header header;
> +    char *xml = NULL;
> +    size_t len;
> +
> +    virCheckFlags(0, -1);
> +
> +    qemuDriverLock(driver);
> +
> +    fd = qemuDomainSaveImageOpen(driver, path, &def, &header, false, NULL,
> +                                 dxml, true);
> +
> +    if (fd < 0)
> +        goto cleanup;
> +
> +    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.

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.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|