On 10/04/2011 01:24 PM, Laine Stump wrote:
- if (virDomainRevertToSnapshot(snapshot, flags)< 0) + result = virDomainRevertToSnapshot(snapshot, flags); + if (result< 0&& force&& + last_error->code == VIR_ERR_SNAPSHOT_REVERT_RISKY) { + flags |= VIR_DOMAIN_SNAPSHOT_REVERT_FORCE;Are you not adding the FORCE flag the first time to allow for better interoperability with older libvirtd?
Exactly. For example, libvirt 0.9.6 does not understand FORCE (and using it would error out with unrecognized flag bit), but just happens to revert to a snapshot created by 0.9.4 on the first call with flags == 0. Libvirt 0.9.7, in the same situation, will error out with the new error value when flags == 0, but with the new error, so that you know you can safely add FORCE. One other consideration is that 0.9.6 understands the new flag VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING, which 0.9.4 did not understand, so blindly looking for the 'unknown flag bit' error is not unique enough to be the indication that we could remove the --force bit and retry, because that still won't resolve on an 0.9.4 server. My choice of --force handling thus seems to be the one most likely to work for 0.9.4, 0.9.6, and 0.9.7, so that the user can blindly type 'virsh snapshot-revert dom old-snapshot --force' and attempt the revert regardless of server version.
Conversely, I still wanted 'virsh snapshot-revert' _without_ --force will work in as many situations as the server supports, and that if a server rejects the operation because force is required, the error message will be specific enough to mention that. This is a good thing, so that users don't have to blindly add '--force' and expose themselves to unnecessary risk; rather, they should only add --force after an explicit message says what force would solve, and deciding that the risk of forcing for that particular case is worth it.
That's the only question I had. It all looks fine - ACK.
I'll wait for the 2/2 ack before applying this, then. -- 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