On Thu, Apr 07, 2011 at 10:31:52AM +0800, Osier Yang wrote:
> 1) 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. (This problem exists
> for "virsh managedsave dom; virsh start dom").
>
> The fix for is to unlink the managed state file only if restoring
> succeeded.
>
> 2) For "virsh save dom; virsh restore dom;", it can cause data
> corruption if one reuse the saved state file for restoring. Add
> doc to tell user about it.
>
> 3) In "qemuDomainObjStart", if "managed_save" is NULL, we shouldn't
> fallback to start the domain, skipping it to cleanup as a incidental
> fix. Discovered by Eric.
>
> ---
> src/qemu/qemu_driver.c | 12 +++++++-----
> tools/virsh.pod | 6 +++++-
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 48fe266..a84780b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3423,18 +3423,20 @@ static int qemudDomainObjStart(virConnectPtr conn,
>
> /*
> * If there is a managed saved state restore it instead of starting
> - * from scratch. In any case the old state is removed.
> + * from scratch. The old state is removed once the restoring succeeded.
> */
> managed_save = qemuDomainManagedSavePath(driver, vm);
> +
> + if (!managed_save)
> + goto cleanup;
> +
> if ((managed_save) && (virFileExists(managed_save))) {
> ret = qemuDomainObjRestore(conn, driver, vm, managed_save);
>
> - if (unlink(managed_save) < 0) {
> + if ((ret == 0) && (unlink(managed_save) < 0))
> VIR_WARN("Failed to remove the managed state %s", managed_save);
> - }
>
> - if (ret == 0)
> - goto cleanup;
> + goto cleanup;
> }
>
> ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, -1, NULL,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index f4bd294..6319373 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -546,7 +546,11 @@ I<on_reboot> parameter in the domain's XML definition.
>
> =item B<restore> I<state-file>
>
> -Restores a domain from an B<virsh save> state file. See I<save> for more info.
> +Restores a domain from an B<virsh save> state file. See I<save> for more info.
> +
> +B<Note>: To avoid corrupting file system contents within the domain, you
> +should not reuse the saved state file to B<restore> unless you are convinced
> +with reverting the domain to the previous state.
>
> =item B<save> I<domain-id> I<state-file>
ACK, thanks !
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