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

Re: [libvirt] [PATCH v5] qemu: Remove the managed state file only if restoring succeeded



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 veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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