[libvirt] [PATCHv2 2/2] qemu: snapshot: Avoid libvirtd crash when qemu crashes while snapshotting

[Ján Tomko]

On 01/20/2014 06:09 PM, Peter Krempa wrote:
> We shouldn't access the domain definition while we are in the monitor
> section as the domain is unlocked. Additionaly after we exit from the
> monitor we need to check if the VM is still alive. Not doing so resulted
> into crash if qemu exits while attempting to do a external VM snapshot.
> ---
> 
> Notes:
>     Version 1 was reported to break locking of a VM but withoit enough data to reproduce/trace.
>     Version 2:
>     - changed some of the conditions so that proper cleanup paths are taken
>     - fixed returned error value in case the job can't be entered
> 
>  src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 41 insertions(+), 12 deletions(-)
> 

> @@ -12605,8 +12606,25 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
> +    if (!actions) {
> +        qemuDomainObjExitMonitor(driver, vm);
> +        if (!virDomainObjIsActive(vm)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Domain crashed while taking the snapshot"));
> +            ret = -1;
> +        }
> +    }
> +

You use 'Domain' here...

> @@ -12724,24 +12739,36 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
>      if (actions) {
> -        if (ret == 0)
> -            ret = qemuMonitorTransaction(priv->mon, actions);
> +        if (ret == 0) {
> +            if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
> +                ret = qemuMonitorTransaction(priv->mon, actions);
> +                qemuDomainObjExitMonitor(driver, vm);
> +                if (!virDomainObjIsActive(vm)) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("domain crashed while taking the snapshot"));
> +                    ret = -1;
> +                }
> +            } else {

and 'domain' here. It would be better to be consistent, if only because
translations are case-sensitive.

I'd suggest going with lowercase, as it seems to be used in most of the
snapshot errors.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140121/1a8eabb9/attachment-0001.sig>