[libvirt] [PATCH 0/8] qemu: blockdevize qemuDomainGetBlockJobInfo and qemuDomainBlockJobAbort (blockdev-add saga)
In the effort to use blockdev we need a few tweaks for the support APIs. Peter Krempa (8): qemu: driver: blockdevize qemuDomainGetBlockJobInfo qemu: Make checks in qemuDomainBlockPivot depend on data of the job qemu: blockjob: Add block job states for abort and pivot operations qemu: Use QEMU_BLOCKJOB_STATE_PIVOTING/ABORTING in qemuDomainBlockJobAbort qemu: driver: Report error if pivoting fails in qemuDomainBlockJobAbort qemu: driver: Blockdevize qemuDomainBlockJobAbort/Pivot qemu: Remove stale comment outlining how to extend qemuDomainBlockPivot qemu: driver: Remove semi-stale comment about asynchronous job abort src/qemu/qemu_blockjob.c | 13 +++++- src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_driver.c | 88 ++++++++++++++++++++++++++-------------- src/qemu/qemu_monitor.c | 2 +- 4 files changed, 72 insertions(+), 33 deletions(-) -- 2.21.0
Use the stored job name rather than passing in the disk alias when refering to the job which allows the same code to work also when -blockdev will be used. Note that this API does not require the change to use 'query-job' as it will ever only work with blockjobs bound to disks due to the arguments which allow only refering to a disk. For the disk-less jobs we'll need to add a separate API later. The change to qemuMonitorGetBlockJobInfo is required as the API was striping the 'drive-' prefix when returning the data which is not desired any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 9 +++++++-- src/qemu/qemu_monitor.c | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 329c166255..739cbc5ed3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17292,6 +17292,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, virDomainDiskDefPtr disk; int ret = -1; qemuMonitorBlockJobInfo rawInfo; + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; virCheckFlags(VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES, -1); @@ -17314,9 +17315,13 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, goto endjob; } + if (!(job = qemuBlockJobDiskGetJob(disk))) { + ret = 0; + goto endjob; + } + qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm), - disk->info.alias, &rawInfo); + ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm), job->name, &rawInfo); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; if (ret <= 0) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 5ad66d1dca..a880da3ab6 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3463,7 +3463,7 @@ qemuMonitorGetBlockJobInfo(qemuMonitorPtr mon, VIR_DEBUG("alias=%s, info=%p", alias, info); - if (!(all = qemuMonitorGetAllBlockJobInfo(mon, false))) + if (!(all = qemuMonitorGetAllBlockJobInfo(mon, true))) return -1; if ((data = virHashLookup(all, alias))) { -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:36PM +0200, Peter Krempa wrote:
Use the stored job name rather than passing in the disk alias when refering to the job which allows the same code to work also when
referring
-blockdev will be used.
Note that this API does not require the change to use 'query-job' as it will ever only work with blockjobs bound to disks due to the arguments which allow only refering to a disk. For the disk-less jobs we'll need
referring
to add a separate API later.
The change to qemuMonitorGetBlockJobInfo is required as the API was striping the 'drive-' prefix when returning the data which is not
stripping, unless you meant qemuMonitorGetBlockZebraInfo
desired any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 9 +++++++-- src/qemu/qemu_monitor.c | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Do decisions based on the configuration of the job rather than the data stored with the disk. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 739cbc5ed3..fa9e3c2bfc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16982,17 +16982,26 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; - if (!disk->mirror) { + switch ((qemuBlockJobType) job->type) { + case QEMU_BLOCKJOB_TYPE_NONE: + case QEMU_BLOCKJOB_TYPE_PULL: + case QEMU_BLOCKJOB_TYPE_COMMIT: + case QEMU_BLOCKJOB_TYPE_INTERNAL: + case QEMU_BLOCKJOB_TYPE_LAST: virReportError(VIR_ERR_OPERATION_INVALID, - _("pivot of disk '%s' requires an active copy job"), - disk->dst); + _("job type '%s' does not support pivot"), + NULLSTR(qemuBlockjobTypeToString(job->type))); goto cleanup; + + case QEMU_BLOCKJOB_TYPE_COPY: + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + break; } - if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { + if (job->state != QEMU_BLOCKJOB_STATE_READY) { virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, - _("disk '%s' not ready for pivot yet"), - disk->dst); + _("block job '%s' not ready for pivot yet"), + job->name); goto cleanup; } @@ -17017,7 +17026,8 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, if (ret < 0) goto cleanup; - disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT; + if (disk && disk->mirror) + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT; cleanup: return ret; -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:37PM +0200, Peter Krempa wrote:
Do decisions based on the configuration of the job rather than the data stored with the disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 739cbc5ed3..fa9e3c2bfc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16982,17 +16982,26 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData;
- if (!disk->mirror) { + switch ((qemuBlockJobType) job->type) { + case QEMU_BLOCKJOB_TYPE_NONE: + case QEMU_BLOCKJOB_TYPE_PULL: + case QEMU_BLOCKJOB_TYPE_COMMIT: + case QEMU_BLOCKJOB_TYPE_INTERNAL: + case QEMU_BLOCKJOB_TYPE_LAST:
Strictly speaking, TYPE_LAST in an enum error, not a correct block job type
virReportError(VIR_ERR_OPERATION_INVALID, - _("pivot of disk '%s' requires an active copy job"), - disk->dst); + _("job type '%s' does not support pivot"), + NULLSTR(qemuBlockjobTypeToString(job->type)));
That would remove the need for this NULLSTR
goto cleanup; + + case QEMU_BLOCKJOB_TYPE_COPY: + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + break; }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
When initiating a pivot or abort of a block job we need to track which one was initiated. Currently it was done via data stashed in virDomainDiskDef. Add possibility to track this also together with the job itself. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 9 ++++++++- src/qemu/qemu_blockjob.h | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 292610d089..fe0114bf26 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -53,7 +53,9 @@ VIR_ENUM_IMPL(qemuBlockjobState, "ready", "new", "running", - "concluded"); + "concluded", + "aborting", + "pivoting"); VIR_ENUM_IMPL(qemuBlockjob, QEMU_BLOCKJOB_TYPE_LAST, @@ -599,6 +601,8 @@ qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, case QEMU_BLOCKJOB_STATE_NEW: case QEMU_BLOCKJOB_STATE_RUNNING: case QEMU_BLOCKJOB_STATE_CONCLUDED: + case QEMU_BLOCKJOB_STATE_ABORTING: + case QEMU_BLOCKJOB_STATE_PIVOTING: case QEMU_BLOCKJOB_STATE_LAST: default: break; @@ -724,6 +728,9 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, case QEMU_BLOCKJOB_STATE_NEW: case QEMU_BLOCKJOB_STATE_RUNNING: case QEMU_BLOCKJOB_STATE_LAST: + /* these are never processed as 'newstate' */ + case QEMU_BLOCKJOB_STATE_ABORTING: + case QEMU_BLOCKJOB_STATE_PIVOTING: default: job->newstate = -1; } diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index d07ab75c8b..1f353116c8 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -40,6 +40,8 @@ typedef enum { QEMU_BLOCKJOB_STATE_RUNNING, QEMU_BLOCKJOB_STATE_CONCLUDED, /* job has finished, but it's unknown whether it has failed or not */ + QEMU_BLOCKJOB_STATE_ABORTING, + QEMU_BLOCKJOB_STATE_PIVOTING, QEMU_BLOCKJOB_STATE_LAST } qemuBlockjobState; verify((int)QEMU_BLOCKJOB_STATE_NEW == VIR_DOMAIN_BLOCK_JOB_LAST); -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:38PM +0200, Peter Krempa wrote:
When initiating a pivot or abort of a block job we need to track which one was initiated. Currently it was done via data stashed in virDomainDiskDef. Add possibility to track this also together with the job itself.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 9 ++++++++- src/qemu/qemu_blockjob.h | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Set the correct job states after the operation is requested in qemu. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 4 +++- src/qemu/qemu_driver.c | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index fe0114bf26..0f08cf7967 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -256,7 +256,9 @@ bool qemuBlockJobIsRunning(qemuBlockJobDataPtr job) { return job->state == QEMU_BLOCKJOB_STATE_RUNNING || - job->state == QEMU_BLOCKJOB_STATE_READY; + job->state == QEMU_BLOCKJOB_STATE_READY || + job->state == QEMU_BLOCKJOB_STATE_ABORTING || + job->state == QEMU_BLOCKJOB_STATE_PIVOTING; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fa9e3c2bfc..39b5ea5e7e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17028,6 +17028,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, if (disk && disk->mirror) disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT; + job->state = QEMU_BLOCKJOB_STATE_PIVOTING; cleanup: return ret; @@ -17182,10 +17183,10 @@ qemuDomainBlockJobAbort(virDomainPtr dom, goto endjob; } - if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE && - disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { + if (job->state == QEMU_BLOCKJOB_STATE_ABORTING || + job->state == QEMU_BLOCKJOB_STATE_PIVOTING) { virReportError(VIR_ERR_OPERATION_INVALID, - _("another job on disk '%s' is still being ended"), + _("block job on disk '%s' is still being ended"), disk->dst); goto endjob; } @@ -17209,6 +17210,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom, if (disk->mirror) disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT; + job->state = QEMU_BLOCKJOB_STATE_ABORTING; } ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)); -- 2.21.0
On Thu, Jul 18, 2019 at 18:31:39 +0200, Peter Krempa wrote:
Set the correct job states after the operation is requested in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 4 +++- src/qemu/qemu_driver.c | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-)
later in my blockdev branch I found the following hunk which is needed to correctly transition into the cancelled state when aborting the job which I forgot to merge into this patch: diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 0f08cf7967..575292ada2 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -673,6 +673,10 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) goto cleanup; + if (job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED && + job->state == QEMU_BLOCKJOB_STATE_ABORTING) + job->newstate = QEMU_BLOCKJOB_STATE_CANCELLED; + if (refreshed) qemuDomainSaveStatus(vm);
On Mon, Jul 22, 2019 at 06:24:43PM +0200, Peter Krempa wrote:
On Thu, Jul 18, 2019 at 18:31:39 +0200, Peter Krempa wrote:
Set the correct job states after the operation is requested in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 4 +++- src/qemu/qemu_driver.c | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-)
later in my blockdev branch I found the following hunk which is needed to correctly transition into the cancelled state when aborting the job which I forgot to merge into this patch:
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 0f08cf7967..575292ada2 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -673,6 +673,10 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) goto cleanup;
+ if (job->newstate == QEMU_BLOCKJOB_STATE_COMPLETED && + job->state == QEMU_BLOCKJOB_STATE_ABORTING) + job->newstate = QEMU_BLOCKJOB_STATE_CANCELLED; + if (refreshed) qemuDomainSaveStatus(vm);
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
As the error message is now available and we know whether the job failed we can report an error straight away rather than having the user check the event. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39b5ea5e7e..7a69a0e084 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17230,6 +17230,21 @@ qemuDomainBlockJobAbort(virDomainPtr dom, } qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); } + + if (pivot && + job->state == QEMU_BLOCKJOB_STATE_FAILED) { + if (job->errmsg) + virReportError(VIR_ERR_OPERATION_FAILED, + _("block job '%s' failed while pivoting"), + job->name); + else + virReportError(VIR_ERR_OPERATION_FAILED, + _("block job '%s' failed while pivoting: %s"), + job->name, NULLSTR(job->errmsg)); + + ret = -1; + goto endjob; + } } endjob: -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:40PM +0200, Peter Krempa wrote:
As the error message is now available and we know whether the job failed we can report an error straight away rather than having the user check the event.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39b5ea5e7e..7a69a0e084 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17230,6 +17230,21 @@ qemuDomainBlockJobAbort(virDomainPtr dom, } qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); } + + if (pivot && + job->state == QEMU_BLOCKJOB_STATE_FAILED) { + if (job->errmsg) + virReportError(VIR_ERR_OPERATION_FAILED, + _("block job '%s' failed while pivoting"), + job->name); + else
Please, put some braces around these multi-line bodies.
+ virReportError(VIR_ERR_OPERATION_FAILED, + _("block job '%s' failed while pivoting: %s"), + job->name, NULLSTR(job->errmsg));
It is self-evident that job->errormsg cannot be NULL here. Jano
+ + ret = -1;
I'm not a fan of the numerous alterations of the 'ret' value throughout the function.
+ goto endjob; + } }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use job-complete/job-abort instead of the blockjob-* variants for blockdev. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a69a0e084..12ae31b9a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16981,6 +16981,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); switch ((qemuBlockJobType) job->type) { case QEMU_BLOCKJOB_TYPE_NONE: @@ -17016,7 +17017,10 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, * that pivot failed, we need to reflect that failure into the * overall return value. */ qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorDrivePivot(priv->mon, job->name); + if (blockdev) + ret = qemuMonitorJobComplete(priv->mon, job->name); + else + ret = qemuMonitorDrivePivot(priv->mon, job->name); if (qemuDomainObjExitMonitor(driver, vm) < 0) { ret = -1; goto cleanup; @@ -17157,6 +17161,8 @@ qemuDomainBlockJobAbort(virDomainPtr dom, bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC); qemuBlockJobDataPtr job = NULL; virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv = NULL; + bool blockdev = false; int ret = -1; virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC | @@ -17183,6 +17189,9 @@ qemuDomainBlockJobAbort(virDomainPtr dom, goto endjob; } + priv = vm->privateData; + blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); + if (job->state == QEMU_BLOCKJOB_STATE_ABORTING || job->state == QEMU_BLOCKJOB_STATE_PIVOTING) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -17199,7 +17208,10 @@ qemuDomainBlockJobAbort(virDomainPtr dom, goto endjob; } else { qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), job->name); + if (blockdev) + ret = qemuMonitorJobCancel(priv->mon, job->name, false); + else + ret = qemuMonitorBlockJobCancel(priv->mon, job->name); if (qemuDomainObjExitMonitor(driver, vm) < 0) { ret = -1; goto endjob; -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:41PM +0200, Peter Krempa wrote:
Use job-complete/job-abort instead of the blockjob-* variants for blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
With -blockdev: - we track the job and check it after restart - have the ability to ask qemu to persist it to collect result - have the ability to report errors. This solves all points the comment outlined so remove it. Also all jobs handle the disk state modification along with the event so there's nothing special the comment says. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 12ae31b9a7..cd0e1d5cf8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17006,16 +17006,6 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, goto cleanup; } - /* Attempt the pivot. Record the attempt now, to prevent duplicate - * attempts; but the actual disk change will be made when emitting - * the event. - * XXX On libvirtd restarts, if we missed the qemu event, we need - * to double check what state qemu is in. - * XXX We should be using qemu's rerror flag to make sure the job - * remains alive until we know its final state. - * XXX If the abort command is synchronous but the qemu event says - * that pivot failed, we need to reflect that failure into the - * overall return value. */ qemuDomainObjEnterMonitor(driver, vm); if (blockdev) ret = qemuMonitorJobComplete(priv->mon, job->name); -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:42PM +0200, Peter Krempa wrote:
With -blockdev:
- we track the job and check it after restart - have the ability to ask qemu to persist it to collect result - have the ability to report errors.
This solves all points the comment outlined so remove it. Also all jobs handle the disk state modification along with the event so there's nothing special the comment says.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 ---------- 1 file changed, 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Now that we track the job separately we watch only for the abort of the one single block job so the comment is no longer accurate. Also describing asynchronous operation is not really necessary. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cd0e1d5cf8..22166c0745 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17217,12 +17217,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom, ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)); - /* - * With the ABORT_ASYNC flag we don't need to do anything, the event will - * come from qemu and will update the XML as appropriate, but without the - * ABORT_ASYNC flag, we must block to guarantee synchronous operation. We - * do the waiting while still holding the VM job, to prevent newly - * scheduled block jobs from confusing us. */ if (!async) { qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); while (qemuBlockJobIsRunning(job)) { -- 2.21.0
On Thu, Jul 18, 2019 at 06:31:43PM +0200, Peter Krempa wrote:
Now that we track the job separately we watch only for the abort of the one single block job so the comment is no longer accurate. Also describing asynchronous operation is not really necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa