Re: [PATCHv3 2/3] save: let qemu driver manipulate save files

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

 



On 07/28/2011 12:47 PM, Laine Stump wrote:
On 07/22/2011 12:21 AM, Eric Blake wrote:
The goal here is that save-image-dumpxml fed back to save
image-define without changing the save file; anywhere that
this is not the case is probably a bug in domain_conf.c.

I'll do a v4 of patch 3/3 to plug those domain_conf.c holes, but in the meantime, this patch is good enough on its own even if the edit cycle is not idempotent yet.


+ if (edit&& STREQ(xml, xmlin)) {
+ VIR_FREE(xml);
+ if (VIR_CLOSE(fd)< 0) {
+ virReportSystemError(errno, _("cannot close file: %s"), path);
+ goto error;
+ }
+ return -2;
+ }


So "unchanged" is indicated with a < 0 return code, but that only
happens when edit == true, and only one of the callers does that (and
properly handles the special case).

Yes, because it cannot return a positive value for this case (the function returns an fd on normal success - so one way of looking at it is that no edits needed is a special case failure).

+
+ if (len> header.xml_len) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("new xml too large to fit in file"));
+ }

Did you forget a goto cleanup there?

+ if (VIR_EXPAND_N(xml, len, header.xml_len - len)< 0) {

because if len > header.xml_len, that's going to be a *very large*
number :-)

*very large* indeed - I'd love to have a 64-bit machine with that much RAM :-) I've added the missing goto.


ACK with the goto cleanup added in when the new length is too big to fit
(unless I've misunderstood the code).

You were right, so I made the change, and pushed.

--
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]