[libvirt] [PATCH 18/25] qemu: blockjob: Add modern block job event handler

Peter Krempa posted 25 patches 6 years, 7 months ago
[libvirt] [PATCH 18/25] qemu: blockjob: Add modern block job event handler
Posted by Peter Krempa 6 years, 7 months ago
Add the infrastructure to handle block job events in the -blockdev era.

Some complexity is required as qemu does not bother to notify whether
the job was concluded successfully or failed. Thus it's necessary to
re-query the monitor.

To minimize the possibility of stuck jobs save the state into the XML
prior to handling everything so that the reconnect code can potentially
continue with the cleanup.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_blockjob.c | 168 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 167 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index dd6071dae1..8b142f1aba 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -423,6 +423,169 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
 }


+static void
+qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
+                                            virQEMUDriverPtr driver,
+                                            virDomainObjPtr vm,
+                                            qemuDomainAsyncJob asyncJob ATTRIBUTE_UNUSED)
+{
+    switch ((qemuBlockjobState) job->newstate) {
+    case QEMU_BLOCKJOB_STATE_COMPLETED:
+        switch ((qemuBlockJobType) job->type) {
+        case QEMU_BLOCKJOB_TYPE_PULL:
+        case QEMU_BLOCKJOB_TYPE_COMMIT:
+        case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+        case QEMU_BLOCKJOB_TYPE_COPY:
+        case QEMU_BLOCKJOB_TYPE_NONE:
+        case QEMU_BLOCKJOB_TYPE_INTERNAL:
+        case QEMU_BLOCKJOB_TYPE_LAST:
+        default:
+            break;
+        }
+        break;
+
+    case QEMU_BLOCKJOB_STATE_FAILED:
+    case QEMU_BLOCKJOB_STATE_CANCELLED:
+        switch ((qemuBlockJobType) job->type) {
+        case QEMU_BLOCKJOB_TYPE_PULL:
+        case QEMU_BLOCKJOB_TYPE_COMMIT:
+        case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
+        case QEMU_BLOCKJOB_TYPE_COPY:
+        case QEMU_BLOCKJOB_TYPE_NONE:
+        case QEMU_BLOCKJOB_TYPE_INTERNAL:
+        case QEMU_BLOCKJOB_TYPE_LAST:
+        default:
+            break;
+        }
+        break;
+
+    /* states below are impossible in this handler */
+    case QEMU_BLOCKJOB_STATE_READY:
+    case QEMU_BLOCKJOB_STATE_NEW:
+    case QEMU_BLOCKJOB_STATE_RUNNING:
+    case QEMU_BLOCKJOB_STATE_CONCLUDED:
+    case QEMU_BLOCKJOB_STATE_LAST:
+    default:
+        break;
+    }
+
+    qemuBlockJobEmitEvents(driver, vm, job->disk, job->type, job->newstate);
+    job->state = job->newstate;
+    job->newstate = -1;
+}
+
+
+static void
+qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
+                                  virQEMUDriverPtr driver,
+                                  virDomainObjPtr vm,
+                                  qemuDomainAsyncJob asyncJob)
+{
+    qemuMonitorJobInfoPtr *jobinfo = NULL;
+    size_t njobinfo = 0;
+    size_t i;
+    int rc = 0;
+    bool dismissed = false;
+    bool refreshed = false;
+
+    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+        goto cleanup;
+
+    /* we need to fetch the error state as the event does not propagate it */
+    if (job->newstate == QEMU_BLOCKJOB_STATE_CONCLUDED &&
+        (rc = qemuMonitorGetJobInfo(qemuDomainGetMonitor(vm), &jobinfo, &njobinfo)) == 0) {
+
+        for (i = 0; i < njobinfo; i++) {
+            if (STRNEQ_NULLABLE(job->name, jobinfo[i]->id))
+                continue;
+
+            if (VIR_STRDUP(job->errmsg, jobinfo[i]->error) < 0)
+                rc = -1;
+
+            if (job->errmsg)
+                job->newstate = QEMU_BLOCKJOB_STATE_FAILED;
+            else
+                job->newstate = QEMU_BLOCKJOB_STATE_COMPLETED;
+
+            refreshed = true;
+
+            break;
+        }
+
+        if (i == njobinfo) {
+            VIR_WARN("failed to refresh job '%s'", job->name);
+            rc = -1;
+        }
+    }
+
+    /* dismiss job in qemu */
+    if (rc >= 0) {
+        if ((rc = qemuMonitorJobDismiss(qemuDomainGetMonitor(vm), job->name)) >= 0)
+            dismissed = true;
+    }
+
+    if (job->invalidData) {
+        VIR_WARN("terminating job '%s' with invalid data", job->name);
+        goto cleanup;
+    }
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+        goto cleanup;
+
+    if (refreshed)
+        qemuDomainSaveStatus(vm);
+
+    VIR_DEBUG("handling job '%s' state '%d' newstate '%d'", job->name, job->state, job->newstate);
+
+    qemuBlockJobEventProcessConcludedTransition(job, driver, vm, asyncJob);
+
+ cleanup:
+    if (dismissed) {
+        qemuBlockJobUnregister(job, vm);
+        job = NULL;
+        qemuDomainSaveConfig(vm);
+    }
+
+    for (i = 0; i < njobinfo; i++)
+        qemuMonitorJobInfoFree(jobinfo[i]);
+    VIR_FREE(jobinfo);
+}
+
+
+static void
+qemuBlockJobEventProcess(virQEMUDriverPtr driver,
+                         virDomainObjPtr vm,
+                         qemuBlockJobDataPtr job,
+                         qemuDomainAsyncJob asyncJob)
+
+{
+    switch ((qemuBlockjobState) job->newstate) {
+    case QEMU_BLOCKJOB_STATE_COMPLETED:
+    case QEMU_BLOCKJOB_STATE_FAILED:
+    case QEMU_BLOCKJOB_STATE_CANCELLED:
+    case QEMU_BLOCKJOB_STATE_CONCLUDED:
+        qemuBlockJobEventProcessConcluded(job, driver, vm, asyncJob);
+        break;
+
+    case QEMU_BLOCKJOB_STATE_READY:
+        if (job->disk && job->disk->mirror) {
+            job->disk->mirrorState = VIR_DOMAIN_BLOCK_JOB_READY;
+            qemuBlockJobEmitEvents(driver, vm, job->disk, job->type, job->newstate);
+        }
+        job->state = job->newstate;
+        job->newstate = -1;
+        qemuDomainSaveStatus(vm);
+        break;
+
+    case QEMU_BLOCKJOB_STATE_NEW:
+    case QEMU_BLOCKJOB_STATE_RUNNING:
+    case QEMU_BLOCKJOB_STATE_LAST:
+    default:
+        job->newstate = -1;
+    }
+}
+
+
 /**
  * qemuBlockJobUpdate:
  * @vm: domain
@@ -444,7 +607,10 @@ qemuBlockJobUpdate(virDomainObjPtr vm,
     if (job->newstate == -1)
         return -1;

-    qemuBlockJobEventProcessLegacy(priv->driver, vm, job, asyncJob);
+    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
+        qemuBlockJobEventProcess(priv->driver, vm, job, asyncJob);
+    else
+        qemuBlockJobEventProcessLegacy(priv->driver, vm, job, asyncJob);

     return job->state;
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 18/25] qemu: blockjob: Add modern block job event handler
Posted by Ján Tomko 6 years, 6 months ago
On Fri, Jul 12, 2019 at 06:05:59PM +0200, Peter Krempa wrote:
>Add the infrastructure to handle block job events in the -blockdev era.
>
>Some complexity is required as qemu does not bother to notify whether
>the job was concluded successfully or failed. Thus it's necessary to
>re-query the monitor.
>
>To minimize the possibility of stuck jobs save the state into the XML
>prior to handling everything so that the reconnect code can potentially
>continue with the cleanup.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_blockjob.c | 168 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 167 insertions(+), 1 deletion(-)
>
>diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
>index dd6071dae1..8b142f1aba 100644
>--- a/src/qemu/qemu_blockjob.c
>+++ b/src/qemu/qemu_blockjob.c
>+static void
>+qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
>+                                  virQEMUDriverPtr driver,
>+                                  virDomainObjPtr vm,
>+                                  qemuDomainAsyncJob asyncJob)
>+{
>+    qemuMonitorJobInfoPtr *jobinfo = NULL;
>+    size_t njobinfo = 0;
>+    size_t i;
>+    int rc = 0;
>+    bool dismissed = false;
>+    bool refreshed = false;
>+
>+    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>+        goto cleanup;
>+
>+    /* we need to fetch the error state as the event does not propagate it */
>+    if (job->newstate == QEMU_BLOCKJOB_STATE_CONCLUDED &&
>+        (rc = qemuMonitorGetJobInfo(qemuDomainGetMonitor(vm), &jobinfo, &njobinfo)) == 0) {
>+
>+        for (i = 0; i < njobinfo; i++) {
>+            if (STRNEQ_NULLABLE(job->name, jobinfo[i]->id))
>+                continue;
>+
>+            if (VIR_STRDUP(job->errmsg, jobinfo[i]->error) < 0)
>+                rc = -1;
>+
>+            if (job->errmsg)
>+                job->newstate = QEMU_BLOCKJOB_STATE_FAILED;
>+            else
>+                job->newstate = QEMU_BLOCKJOB_STATE_COMPLETED;
>+
>+            refreshed = true;
>+
>+            break;
>+        }
>+
>+        if (i == njobinfo) {
>+            VIR_WARN("failed to refresh job '%s'", job->name);
>+            rc = -1;
>+        }
>+    }
>+
>+    /* dismiss job in qemu */
>+    if (rc >= 0) {
>+        if ((rc = qemuMonitorJobDismiss(qemuDomainGetMonitor(vm), job->name)) >= 0)
>+            dismissed = true;
>+    }
>+
>+    if (job->invalidData) {
>+        VIR_WARN("terminating job '%s' with invalid data", job->name);
>+        goto cleanup;
>+    }
>+
>+    if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
>+        goto cleanup;
>+
>+    if (refreshed)
>+        qemuDomainSaveStatus(vm);
>+
>+    VIR_DEBUG("handling job '%s' state '%d' newstate '%d'", job->name, job->state, job->newstate);
>+
>+    qemuBlockJobEventProcessConcludedTransition(job, driver, vm, asyncJob);
>+
>+ cleanup:
>+    if (dismissed) {
>+        qemuBlockJobUnregister(job, vm);
>+        job = NULL;

job is a local parameter here so this statement has no real effect.
Moreover, I'd expect that the caller of the function still holds a
reference.

>+        qemuDomainSaveConfig(vm);
>+    }
>+
>+    for (i = 0; i < njobinfo; i++)
>+        qemuMonitorJobInfoFree(jobinfo[i]);
>+    VIR_FREE(jobinfo);
>+}
>+
>+

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list