9:17 a.m.
While playing with the incremental backup stuff I came across few corner
cases in the blockdev job handling.
Peter Krempa (10):
qemu: monitor: Finish implementation of infrastructure for
'query-jobs'
qemu: blockjob: Properly propagate cancellation of blockjobs
qemu: process: Move block job refresh after async job recovery
qemu: blockjob: Fix deadlock when terminating job with invalid data
qemu: blockjob: Log blockjobs which are dropped when untracked by qemu
qemu: blockjob: Mark job with broken data but tracked by qemu as
reconnected
qemu: blockjob: Don't stop processing the finished job early
qemu: blockjob: Separate clearing of per-job data
qemu: blockjob: Introduce "broken" block job type
qemu: blockjob: Finish handling job with broken data
src/qemu/qemu_blockjob.c | 70 ++++++++++++-------
src/qemu/qemu_blockjob.h | 3 +
src/qemu/qemu_domain.c | 13 +++-
src/qemu/qemu_driver.c | 1 +
src/qemu/qemu_monitor.h | 4 +-
src/qemu/qemu_monitor_json.c | 6 ++
src/qemu/qemu_process.c | 6 +-
.../blockjob-blockdev-in.xml | 1 +
8 files changed, 73 insertions(+), 31 deletions(-)
--
2.23.0
9:17 a.m.
New subject: [libvirt] [PATCH 01/10] qemu: monitor: Finish implementation of infrastructure for 'query-jobs'
Commit ed56851f1bc6f5 didn't wire up fetching of the statistics for the
job which are reported by 'query-jobs'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor.h | 4 ++--
src/qemu/qemu_monitor_json.c | 6 ++++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index a17d7200c2..e2bfc420bb 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -154,8 +154,8 @@ struct _qemuMonitorJobInfo {
qemuMonitorJobType type;
qemuMonitorJobStatus status;
char *error;
- long long progressCurrent;
- long long progressTotal;
+ unsigned long long progressCurrent;
+ unsigned long long progressTotal;
};
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 4d38030179..9f3783ab70 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9221,6 +9221,12 @@ qemuMonitorJSONGetJobInfoOne(virJSONValuePtr data)
job->id = g_strdup(id);
job->error = g_strdup(errmsg);
+ /* failure to fetch progress stats is not fatal */
+ ignore_value(virJSONValueObjectGetNumberUlong(data, "current-progress",
+ &job->progressCurrent));
+ ignore_value(virJSONValueObjectGetNumberUlong(data, "total-progress",
+ &job->progressTotal));
+
return g_steal_pointer(&job);
}
--
2.23.0
2:24 p.m.
New subject: [libvirt] [PATCH 01/10] qemu: monitor: Finish implementation of infrastructure for 'query-jobs'
On 11/26/19 10:17 AM, Peter Krempa wrote:
Commit ed56851f1bc6f5 didn't wire up fetching of the statistics
for the
job which are reported by 'query-jobs'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor.h | 4 ++--
src/qemu/qemu_monitor_json.c | 6 ++++++
2 files changed, 8 insertions(+), 2 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 02/10] qemu: blockjob: Properly propagate cancellation of blockjobs
qemu returns an error message in the job statistics even if the job was
cancelled to emphasize it was not successful. Libvirt didn't properly
transform it into QEMU_BLOCKJOB_STATE_CANCELLED though.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 92e4d391c9..2283d49c61 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1313,7 +1313,8 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
goto cleanup;
- if (job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED &&
+ if ((job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED ||
+ job->newstate == QEMU_BLOCKJOB_STATE_FAILED) &&
job->state == QEMU_BLOCKJOB_STATE_ABORTING)
job->newstate = QEMU_BLOCKJOB_STATE_CANCELLED;
--
2.23.0
2:26 p.m.
New subject: [libvirt] [PATCH 02/10] qemu: blockjob: Properly propagate cancellation of blockjobs
On 11/26/19 10:17 AM, Peter Krempa wrote:
qemu returns an error message in the job statistics even if the job
was
cancelled to emphasize it was not successful. Libvirt didn't properly
transform it into QEMU_BLOCKJOB_STATE_CANCELLED though.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 92e4d391c9..2283d49c61 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1313,7 +1313,8 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
goto cleanup;
- if (job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED &&
+ if ((job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED ||
+ job->newstate == QEMU_BLOCKJOB_STATE_FAILED) &&
job->state == QEMU_BLOCKJOB_STATE_ABORTING)
job->newstate = QEMU_BLOCKJOB_STATE_CANCELLED;
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 03/10] qemu: process: Move block job refresh after async job recovery
Block jobs may be members of async jobs so it makes more sense to
refresh block job state after we do steps for async job recovery.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_process.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index cb11da2401..a588ee25f8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8143,9 +8143,6 @@ qemuProcessReconnect(void *opaque)
qemuBlockNodeNamesDetect(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
goto error;
- if (qemuProcessRefreshBlockjobs(driver, obj) < 0)
- goto error;
-
if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
goto error;
@@ -8158,6 +8155,9 @@ qemuProcessReconnect(void *opaque)
if (qemuProcessRecoverJob(driver, obj, &oldjob, &stopFlags) < 0)
goto error;
+ if (qemuProcessRefreshBlockjobs(driver, obj) < 0)
+ goto error;
+
if (qemuProcessUpdateDevices(driver, obj) < 0)
goto error;
--
2.23.0
2:29 p.m.
New subject: [libvirt] [PATCH 03/10] qemu: process: Move block job refresh after async job recovery
On 11/26/19 10:17 AM, Peter Krempa wrote:
Block jobs may be members of async jobs so it makes more sense to
refresh block job state after we do steps for async job recovery.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_process.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 04/10] qemu: blockjob: Fix deadlock when terminating job with invalid data
We must exit the monitor prior to refusing other work, otherwise the VM
object will become unusable.
This bug was introduced in commit v5.5.0-244-gc412383796 but thankfully
the code path was not excercised without QEMU_CAPS_BLOCKDEV.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 2283d49c61..818e36435c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1305,14 +1305,14 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
dismissed = true;
}
+ if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+ goto cleanup;
+
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 ((job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED ||
job->newstate == QEMU_BLOCKJOB_STATE_FAILED) &&
job->state == QEMU_BLOCKJOB_STATE_ABORTING)
--
2.23.0
2:30 p.m.
New subject: [libvirt] [PATCH 04/10] qemu: blockjob: Fix deadlock when terminating job with invalid data
On 11/26/19 10:17 AM, Peter Krempa wrote:
We must exit the monitor prior to refusing other work, otherwise the
VM
object will become unusable.
This bug was introduced in commit v5.5.0-244-gc412383796 but thankfully
the code path was not excercised without QEMU_CAPS_BLOCKDEV.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 05/10] qemu: blockjob: Log blockjobs which are dropped when untracked by qemu
Since we don't know what happened to the job we can't do much about it
but we can at least log that this happened.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 818e36435c..b83d681f06 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -501,8 +501,10 @@ qemuBlockJobRefreshJobs(virQEMUDriverPtr driver,
/* remove data for job which qemu didn't report (the algorithm is
* inefficient, but the possibility of such jobs is very low */
- while ((job = virHashSearch(priv->blockjobs, qemuBlockJobRefreshJobsFindInactive,
NULL, NULL)))
+ while ((job = virHashSearch(priv->blockjobs, qemuBlockJobRefreshJobsFindInactive,
NULL, NULL))) {
+ VIR_WARN("dropping blockjob '%s' untracked by qemu",
job->name);
qemuBlockJobUnregister(job, vm);
+ }
ret = 0;
--
2.23.0
2:31 p.m.
New subject: [libvirt] [PATCH 05/10] qemu: blockjob: Log blockjobs which are dropped when untracked by qemu
On 11/26/19 10:17 AM, Peter Krempa wrote:
Since we don't know what happened to the job we can't do much
about it
but we can at least log that this happened.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 06/10] qemu: blockjob: Mark job with broken data but tracked by qemu as reconnected
Otherwise it would get dropped later on as untracked despite us knowing
about it. Additionally since we cancelled it we must wait to dismiss it
which would not be possible if we unregister it. This also opened a
window for a race condition since the job state change event of the
just-cancelled job might be delivered prior to us unregistering the job
in which case everything would work properly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index b83d681f06..4d14c3d27c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -465,6 +465,8 @@ qemuBlockJobRefreshJobs(virQEMUDriverPtr driver,
if (rc < 0)
qemuBlockJobUnregister(job, vm);
+ else
+ job->reconnected = true;
continue;
}
--
2.23.0
2:41 p.m.
New subject: [libvirt] [PATCH 06/10] qemu: blockjob: Mark job with broken data but tracked by qemu as reconnected
On 11/26/19 10:17 AM, Peter Krempa wrote:
Otherwise it would get dropped later on as untracked despite us
knowing
about it. Additionally since we cancelled it we must wait to dismiss it
which would not be possible if we unregister it. This also opened a
window for a race condition since the job state change event of the
just-cancelled job might be delivered prior to us unregistering the job
in which case everything would work properly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index b83d681f06..4d14c3d27c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -465,6 +465,8 @@ qemuBlockJobRefreshJobs(virQEMUDriverPtr driver,
if (rc < 0)
qemuBlockJobUnregister(job, vm);
+ else
+ job->reconnected = true;
continue;
}
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 07/10] qemu: blockjob: Don't stop processing the finished job early
Both failure to refresh and to dismiss the job are very unlikely but if
they happen there's not much we can do about the blockjob.
The concluded job handlers treat it as if the job failed if we don't
update the state to 'QEMU_BLOCKJOB_STATE_COMPLETED' which is probably
the safest thing to do here.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 4d14c3d27c..5a6c5542a6 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1270,8 +1270,6 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
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)
@@ -1279,7 +1277,7 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
/* 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) {
+ qemuMonitorGetJobInfo(qemuDomainGetMonitor(vm), &jobinfo, &njobinfo) ==
0) {
for (i = 0; i < njobinfo; i++) {
if (STRNEQ_NULLABLE(job->name, jobinfo[i]->id))
@@ -1297,19 +1295,14 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
break;
}
- if (i == njobinfo) {
+ 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;
- }
+ ignore_value(qemuMonitorJobDismiss(qemuDomainGetMonitor(vm), job->name));
- if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
if (job->invalidData) {
@@ -1340,10 +1333,8 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
}
cleanup:
- if (dismissed) {
- qemuBlockJobUnregister(job, vm);
- qemuDomainSaveConfig(vm);
- }
+ qemuBlockJobUnregister(job, vm);
+ qemuDomainSaveConfig(vm);
for (i = 0; i < njobinfo; i++)
qemuMonitorJobInfoFree(jobinfo[i]);
--
2.23.0
2:46 p.m.
New subject: [libvirt] [PATCH 07/10] qemu: blockjob: Don't stop processing the finished job early
On 11/26/19 10:17 AM, Peter Krempa wrote:
Both failure to refresh and to dismiss the job are very unlikely but
if
they happen there's not much we can do about the blockjob.
The concluded job handlers treat it as if the job failed if we don't
update the state to 'QEMU_BLOCKJOB_STATE_COMPLETED' which is probably
the safest thing to do here.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 08/10] qemu: blockjob: Separate clearing of per-job data
We will need to clear per-job type data when we will be marking a
blockjob as broken in the new way. Extract the code for future reuse.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 5a6c5542a6..6db3e0ca84 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -70,6 +70,13 @@ VIR_ENUM_IMPL(qemuBlockjob,
static virClassPtr qemuBlockJobDataClass;
+static void
+qemuBlockJobDataDisposeJobdata(qemuBlockJobDataPtr job)
+{
+ if (job->type == QEMU_BLOCKJOB_TYPE_CREATE)
+ virObjectUnref(job->data.create.src);
+}
+
static void
qemuBlockJobDataDispose(void *obj)
@@ -79,8 +86,7 @@ qemuBlockJobDataDispose(void *obj)
virObjectUnref(job->chain);
virObjectUnref(job->mirrorChain);
- if (job->type == QEMU_BLOCKJOB_TYPE_CREATE)
- virObjectUnref(job->data.create.src);
+ qemuBlockJobDataDisposeJobdata(job);
g_free(job->name);
g_free(job->errmsg);
--
2.23.0
2:49 p.m.
New subject: [libvirt] [PATCH 08/10] qemu: blockjob: Separate clearing of per-job data
On 11/26/19 10:17 AM, Peter Krempa wrote:
We will need to clear per-job type data when we will be marking a
blockjob as broken in the new way. Extract the code for future reuse.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 5a6c5542a6..6db3e0ca84 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -70,6 +70,13 @@ VIR_ENUM_IMPL(qemuBlockjob,
static virClassPtr qemuBlockJobDataClass;
Need an extra newline here to preserve previous spacing
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 09/10] qemu: blockjob: Introduce "broken" block job type
To better track jobs we couldn't parse let's introduce a new job type
which will clarify semantics internally in few places.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 25 ++++++++++++++++++-
src/qemu/qemu_blockjob.h | 3 +++
src/qemu/qemu_domain.c | 13 +++++++++-
src/qemu/qemu_driver.c | 1 +
.../blockjob-blockdev-in.xml | 1 +
5 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 6db3e0ca84..d957e3175e 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -66,7 +66,8 @@ VIR_ENUM_IMPL(qemuBlockjob,
"commit",
"active-commit",
"",
- "create");
+ "create",
+ "broken");
static virClassPtr qemuBlockJobDataClass;
@@ -127,6 +128,23 @@ qemuBlockJobDataNew(qemuBlockJobType type,
}
+/**
+ * qemuBlockJobMarkBroken:
+ * @job: job to mark as broken
+ *
+ * In case when we are unable to parse the block job data from the XML
+ * successfully we'll need to mark the job as broken and then attempt to abort
+ * it. This function marks the job as broken.
+ */
+static void
+qemuBlockJobMarkBroken(qemuBlockJobDataPtr job)
+{
+ qemuBlockJobDataDisposeJobdata(job);
+ job->brokentype = job->type;
+ job->type = QEMU_BLOCKJOB_TYPE_BROKEN;
+}
+
+
/**
* qemuBlockJobRegister:
* @job: job to register
@@ -460,6 +478,9 @@ qemuBlockJobRefreshJobs(virQEMUDriverPtr driver,
* in qemu and just forget about it in libvirt because there's not much
* we coud do besides killing the VM */
if (job->invalidData) {
+
+ qemuBlockJobMarkBroken(job);
+
qemuDomainObjEnterMonitor(driver, vm);
rc = qemuMonitorJobCancel(priv->mon, job->name, true);
@@ -1254,6 +1275,8 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr
job,
qemuBlockJobProcessEventConcludedCopyAbort(driver, vm, job, asyncJob);
break;
+
+ case QEMU_BLOCKJOB_TYPE_BROKEN:
case QEMU_BLOCKJOB_TYPE_NONE:
case QEMU_BLOCKJOB_TYPE_INTERNAL:
case QEMU_BLOCKJOB_TYPE_LAST:
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index d8da918f2f..fdfe2c57ec 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -63,6 +63,7 @@ typedef enum {
/* Additional enum values local to qemu */
QEMU_BLOCKJOB_TYPE_INTERNAL,
QEMU_BLOCKJOB_TYPE_CREATE,
+ QEMU_BLOCKJOB_TYPE_BROKEN,
QEMU_BLOCKJOB_TYPE_LAST
} qemuBlockJobType;
verify((int)QEMU_BLOCKJOB_TYPE_INTERNAL == VIR_DOMAIN_BLOCK_JOB_TYPE_LAST);
@@ -131,6 +132,8 @@ struct _qemuBlockJobData {
int newstate; /* qemuBlockjobState, subset of events emitted by qemu */
+ int brokentype; /* the previous type of a broken blockjob qemuBlockJobType */
+
bool invalidData; /* the job data (except name) is not valid */
bool reconnected; /* internal field for tracking whether job is live after reconnect
to qemu */
};
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c233a4ba96..d1596a28ca 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2475,6 +2475,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload,
virBufferEscapeString(&attrBuf, " type='%s'",
qemuBlockjobTypeToString(job->type));
virBufferEscapeString(&attrBuf, " state='%s'", state);
virBufferEscapeString(&attrBuf, " newstate='%s'", newstate);
+ if (job->brokentype != QEMU_BLOCKJOB_TYPE_NONE)
+ virBufferEscapeString(&attrBuf, " brokentype='%s'",
qemuBlockjobTypeToString(job->brokentype));
virBufferEscapeString(&childBuf, "<errmsg>%s</errmsg>",
job->errmsg);
if (job->disk) {
@@ -2536,6 +2538,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload,
virBufferAddLit(&attrBuf, " shallownew='yes'");
break;
+
+ case QEMU_BLOCKJOB_TYPE_BROKEN:
case QEMU_BLOCKJOB_TYPE_NONE:
case QEMU_BLOCKJOB_TYPE_INTERNAL:
case QEMU_BLOCKJOB_TYPE_LAST:
@@ -3100,6 +3104,8 @@ qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr
job,
}
break;
+
+ case QEMU_BLOCKJOB_TYPE_BROKEN:
case QEMU_BLOCKJOB_TYPE_NONE:
case QEMU_BLOCKJOB_TYPE_INTERNAL:
case QEMU_BLOCKJOB_TYPE_LAST:
@@ -3125,6 +3131,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm,
g_autoptr(qemuBlockJobData) job = NULL;
g_autofree char *name = NULL;
g_autofree char *typestr = NULL;
+ g_autofree char *brokentypestr = NULL;
int type;
g_autofree char *statestr = NULL;
int state = QEMU_BLOCKJOB_STATE_FAILED;
@@ -3146,13 +3153,17 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm,
* clean it up */
if (!(typestr = virXPathString("string(./@type)", ctxt)) ||
(type = qemuBlockjobTypeFromString(typestr)) < 0) {
- type = QEMU_BLOCKJOB_TYPE_NONE;
+ type = QEMU_BLOCKJOB_TYPE_BROKEN;
invalidData = true;
}
if (!(job = qemuBlockJobDataNew(type, name)))
return -1;
+ if ((brokentypestr = virXPathString("string(./@brokentype)", ctxt))
&&
+ (job->brokentype = qemuBlockjobTypeFromString(brokentypestr)) < 0)
+ job->brokentype = QEMU_BLOCKJOB_TYPE_NONE;
+
if (!(statestr = virXPathString("string(./@state)", ctxt)) ||
(state = qemuBlockjobStateTypeFromString(statestr)) < 0)
invalidData = true;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 669c12d6ca..fa8a948568 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17418,6 +17418,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
case QEMU_BLOCKJOB_TYPE_COMMIT:
case QEMU_BLOCKJOB_TYPE_INTERNAL:
case QEMU_BLOCKJOB_TYPE_CREATE:
+ case QEMU_BLOCKJOB_TYPE_BROKEN:
virReportError(VIR_ERR_OPERATION_INVALID,
_("job type '%s' does not support pivot"),
qemuBlockjobTypeToString(job->type));
diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
index 4f6930001e..67ab099bd9 100644
--- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
+++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
@@ -300,6 +300,7 @@
</source>
</src>
</blockjob>
+ <blockjob name='broken-test' type='broken' state='ready'
brokentype='commit'/>
<blockjob name='test-orphan-job0' type='copy'
state='ready'>
<chains>
<disk type='file' format='qcow2'>
--
2.23.0
5:47 p.m.
New subject: [libvirt] [PATCH 09/10] qemu: blockjob: Introduce "broken" block job type
On 11/26/19 10:17 AM, Peter Krempa wrote:
To better track jobs we couldn't parse let's introduce a new
job type
which will clarify semantics internally in few places.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 25 ++++++++++++++++++-
src/qemu/qemu_blockjob.h | 3 +++
src/qemu/qemu_domain.c | 13 +++++++++-
src/qemu/qemu_driver.c | 1 +
.../blockjob-blockdev-in.xml | 1 +
5 files changed, 41 insertions(+), 2 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH 10/10] qemu: blockjob: Finish handling job with broken data
Now that we have a separate job type which will not trigger normal code
paths for terminating job we can remove the ad-hoc handling.
This possibly fixes the issue of a broken job inheriting the disk and
then finishing in which case we'd not detach the backing chain.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index d957e3175e..2c51367c67 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1334,11 +1334,6 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- if (job->invalidData) {
- VIR_WARN("terminating job '%s' with invalid data",
job->name);
- goto cleanup;
- }
-
if ((job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED ||
job->newstate == QEMU_BLOCKJOB_STATE_FAILED) &&
job->state == QEMU_BLOCKJOB_STATE_ABORTING)
--
2.23.0
5:49 p.m.
New subject: [libvirt] [PATCH 10/10] qemu: blockjob: Finish handling job with broken data
On 11/26/19 10:17 AM, Peter Krempa wrote:
Now that we have a separate job type which will not trigger normal
code
paths for terminating job we can remove the ad-hoc handling.
This possibly fixes the issue of a broken job inheriting the disk and
then finishing in which case we'd not detach the backing chain.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index d957e3175e..2c51367c67 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1334,11 +1334,6 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- if (job->invalidData) {
- VIR_WARN("terminating job '%s' with invalid data",
job->name);
- goto cleanup;
- }
-
if ((job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED ||
job->newstate == QEMU_BLOCKJOB_STATE_FAILED) &&
job->state == QEMU_BLOCKJOB_STATE_ABORTING)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
1823
days inactive
1823
days old
20 comments
2 participants
participants (2)
-
Cole Robinson -
Peter Krempa