On Tue, Apr 05, 2011 at 08:55:01AM -0600, Eric Blake wrote: > On 04/05/2011 07:20 AM, Jiri Denemark wrote: > > On Tue, Apr 05, 2011 at 14:47:22 +0800, Osier Yang wrote: > >> Both "qemuDomainStartWithFlags" and "qemuAutostartDomain" try to > >> restore the domain from managedsave'ed image if it exists (by > >> invoking "qemuDomainObjRestore"), but it unlinks the image even > >> if restoring fails, which causes data loss. > >> > >> However, I'm not sure if it's the very correct way to fix it, > >> if restoring fails, and we didn't remove the image, it will > >> trys to restore from the image again next time, if that's > >> not the user expected (e.g. the user made quite many changes > >> on the guest), then it's a new problem. > > > > I think this patch is risky. You should either remove the state on error > > (which is the current state) or fail domain start if managed state is present > > but resuming from it fails. If you do something in the middle (your patch) you > > will certainly end up corrupting domain's disks. > > What's more, I think we should consider removing the saved-state file on > success for 'virsh restore file' - once a state has been restored, the > guest is running and has likely modified its disks, which means that the > saved (memory) state is no longer consistent with the new disk state, > and a second restore of the saved file is asking for a different type of > data corruption. > > That is, I think: > > virsh save dom file > virsh restore file > > should leave file intact if and only if the restore failed, and: The problem there is that you are changing the command behaviour. The user may snapshot the disk separately and use this to implement a simplified domain snapshot. Doing the remove may avoid troubles for those not knowing what they are doing, but also break something for those who know what they are doing. In general I would tend to not change the established operation behaviour, especially on suddenly removing files owned by the user without asking. For managed save that's completely different due to the fact the file is owned by libvir, and removing is fine. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list