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

Re: [libvirt] [PATCHv2 07/20] snapshot: qemu: Add async job type for snapshots



On 11/01/2012 10:22 AM, Peter Krempa wrote:
> The new external system checkpoints will require an async job while the
> snapshot is taken. This patch adds QEMU_ASYNC_JOB_SNAPSHOT to track this
> job type.
> ---
>  src/qemu/qemu_domain.c  |  3 +++
>  src/qemu/qemu_domain.h  |  1 +
>  src/qemu/qemu_process.c | 19 +++++++++++++++++++
>  3 files changed, 23 insertions(+)
> 

> +++ b/src/qemu/qemu_process.c
> @@ -3046,6 +3046,25 @@ qemuProcessRecoverJob(struct qemud_driver *driver,
>          }
>          break;
> 
> +    case QEMU_ASYNC_JOB_SNAPSHOT:
> +        qemuDomainObjEnterMonitor(driver, vm);
> +        ignore_value(qemuMonitorMigrateCancel(priv->mon));
> +        qemuDomainObjExitMonitor(driver, vm);
> +         /* resume the domain but only if it was paused as a result of
> +          * creating the snapshot. */
> +        if (state == VIR_DOMAIN_PAUSED &&
> +            ((job->asyncJob == QEMU_ASYNC_JOB_SNAPSHOT &&

In this particular case branch, job->asyncJob is always equal to
QEMU_ASYNC_JOB_SNAPSHOT.

> +              reason == VIR_DOMAIN_PAUSED_MIGRATION) ||

Don't you mean VIR_DOMAIN_PAUSED_SNAPSHOT here?

I think you can simplify the code by squashing this in for less
redundant code.  ACK with this change:

diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index 67a7157..84be755 100644
--- i/src/qemu/qemu_process.c
+++ w/src/qemu/qemu_process.c
@@ -3025,18 +3025,21 @@ qemuProcessRecoverJob(struct qemud_driver *driver,

     case QEMU_ASYNC_JOB_SAVE:
     case QEMU_ASYNC_JOB_DUMP:
+    case QEMU_ASYNC_JOB_SNAPSHOT:
         qemuDomainObjEnterMonitor(driver, vm);
         ignore_value(qemuMonitorMigrateCancel(priv->mon));
         qemuDomainObjExitMonitor(driver, vm);
         /* resume the domain but only if it was paused as a result of
-         * running save/dump operation.  Although we are recovering an
-         * async job, this function is run at startup and must resume
-         * things using sync monitor connections.  */
+         * running a migration-to-file operation.  Although we are
+         * recovering an async job, this function is run at startup
+         * and must resume things using sync monitor connections.  */
         if (state == VIR_DOMAIN_PAUSED &&
             ((job->asyncJob == QEMU_ASYNC_JOB_DUMP &&
               reason == VIR_DOMAIN_PAUSED_DUMP) ||
              (job->asyncJob == QEMU_ASYNC_JOB_SAVE &&
               reason == VIR_DOMAIN_PAUSED_SAVE) ||
+             (job->asyncJob == QEMU_ASYNC_JOB_SNAPSHOT &&
+              reason == VIR_DOMAIN_PAUSED_SNAPSHOT) ||
              reason == VIR_DOMAIN_PAUSED_UNKNOWN)) {
             if (qemuProcessStartCPUs(driver, vm, conn,
                                      VIR_DOMAIN_RUNNING_UNPAUSED,
@@ -3046,25 +3049,6 @@ qemuProcessRecoverJob(struct qemud_driver *driver,
         }
         break;

-    case QEMU_ASYNC_JOB_SNAPSHOT:
-        qemuDomainObjEnterMonitor(driver, vm);
-        ignore_value(qemuMonitorMigrateCancel(priv->mon));
-        qemuDomainObjExitMonitor(driver, vm);
-         /* resume the domain but only if it was paused as a result of
-          * creating the snapshot. */
-        if (state == VIR_DOMAIN_PAUSED &&
-            ((job->asyncJob == QEMU_ASYNC_JOB_SNAPSHOT &&
-              reason == VIR_DOMAIN_PAUSED_MIGRATION) ||
-             reason == VIR_DOMAIN_PAUSED_UNKNOWN)) {
-            if (qemuProcessStartCPUs(driver, vm, conn,
-                                     VIR_DOMAIN_RUNNING_UNPAUSED,
-                                     QEMU_ASYNC_JOB_NONE) < 0) {
-                VIR_WARN("Could not resume domain %s after snapshot",
-                         vm->def->name);
-            }
-        }
-        break;
-
     case QEMU_ASYNC_JOB_NONE:
     case QEMU_ASYNC_JOB_LAST:
         break;


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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