5:54 a.m.
Review of my incremental backup series prompted for refactor of
qemuDomainAbortJob
Peter Krempa (2):
qemu: driver: Split out cancellation of migration from
qemuDomainAbortJob
qemu: Convert aborting of a domain job to a switch statement
src/qemu/qemu_driver.c | 90 +++++++++++++++++++++++++++++-------------
1 file changed, 63 insertions(+), 27 deletions(-)
--
2.23.0
5:54 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: driver: Split out cancellation of migration from qemuDomainAbortJob
Following patch will refactor qemuDomainAbortJob to use a per-job-type
switch where we will need to abort a migration job in various branches.
Save some code duplication by introducing a helper.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1911073f3e..4cfc1ca9e7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13996,6 +13996,24 @@ qemuDomainGetJobStats(virDomainPtr dom,
}
+static int
+qemuDomainAbortJobMigration(virDomainObjPtr vm)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret;
+
+ VIR_DEBUG("Cancelling migration job at client request");
+
+ qemuDomainObjAbortAsyncJob(vm);
+ qemuDomainObjEnterMonitor(priv->driver, vm);
+ ret = qemuMonitorMigrateCancel(priv->mon);
+ if (qemuDomainObjExitMonitor(priv->driver, vm) < 0)
+ ret = -1;
+
+ return ret;
+}
+
+
static int qemuDomainAbortJob(virDomainPtr dom)
{
virQEMUDriverPtr driver = dom->conn->privateData;
@@ -14047,12 +14065,7 @@ static int qemuDomainAbortJob(virDomainPtr dom)
goto endjob;
}
- VIR_DEBUG("Cancelling job at client request");
- qemuDomainObjAbortAsyncJob(vm);
- qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorMigrateCancel(priv->mon);
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- ret = -1;
+ ret = qemuDomainAbortJobMigration(vm);
endjob:
qemuDomainObjEndJob(driver, vm);
--
2.23.0
5:54 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Convert aborting of a domain job to a switch statement
Until now we only really aborted migration via qemuDomainAbortJob. This
will change with the upcoming addition of the backup job. Additionally
there were a bunch of if statements checking various aspects of the
current job.
To make it more obvious convert qemuDomainAbortJob to use a switch
statement and move the individual conditions to the appropriate job
type.
Every job type has now it's own case despite multiple job types just
plainly cancelling the job for clarity and future extension.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 67 ++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4cfc1ca9e7..bad2fb52f3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14036,36 +14036,59 @@ static int qemuDomainAbortJob(virDomainPtr dom)
priv = vm->privateData;
- if (!priv->job.asyncJob) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("no job is active on the domain"));
- goto endjob;
- }
+ switch (priv->job.asyncJob) {
+ case QEMU_ASYNC_JOB_NONE:
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("no job is active on the domain"));
+ break;
- if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) {
+ case QEMU_ASYNC_JOB_MIGRATION_IN:
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot abort incoming migration;"
" use virDomainDestroy instead"));
- goto endjob;
- }
+ break;
- if (priv->job.asyncJob == QEMU_ASYNC_JOB_DUMP &&
- priv->job.apiFlags & VIR_DUMP_MEMORY_ONLY) {
+ case QEMU_ASYNC_JOB_START:
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot abort memory-only dump"));
- goto endjob;
- }
+ _("cannot abort VM start;"
+ " use virDomainDestroy instead"));
+ break;
- if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT &&
- (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY ||
- (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED &&
- reason == VIR_DOMAIN_PAUSED_POSTCOPY))) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot abort migration in post-copy mode"));
- goto endjob;
- }
+ case QEMU_ASYNC_JOB_MIGRATION_OUT:
+ if ((priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY ||
+ (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED &&
+ reason == VIR_DOMAIN_PAUSED_POSTCOPY))) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot abort migration in post-copy mode"));
+ goto endjob;
+ }
- ret = qemuDomainAbortJobMigration(vm);
+ ret = qemuDomainAbortJobMigration(vm);
+ break;
+
+ case QEMU_ASYNC_JOB_SAVE:
+ ret = qemuDomainAbortJobMigration(vm);
+ break;
+
+ case QEMU_ASYNC_JOB_DUMP:
+ if (priv->job.apiFlags & VIR_DUMP_MEMORY_ONLY) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot abort memory-only dump"));
+ goto endjob;
+ }
+
+ ret = qemuDomainAbortJobMigration(vm);
+ break;
+
+ case QEMU_ASYNC_JOB_SNAPSHOT:
+ ret = qemuDomainAbortJobMigration(vm);
+ break;
+
+ case QEMU_ASYNC_JOB_LAST:
+ default:
+ virReportEnumRangeError(qemuDomainAsyncJob, priv->job.asyncJob);
+ break;
+ }
endjob:
qemuDomainObjEndJob(driver, vm);
--
2.23.0
6:39 a.m.
On Thu, Dec 05, 2019 at 12:54:37 +0100, Peter Krempa wrote:
Review of my incremental backup series prompted for refactor of
qemuDomainAbortJob
Peter Krempa (2):
qemu: driver: Split out cancellation of migration from
qemuDomainAbortJob
qemu: Convert aborting of a domain job to a switch statement
src/qemu/qemu_driver.c | 90 +++++++++++++++++++++++++++++-------------
1 file changed, 63 insertions(+), 27 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
1814
days inactive
1814
days old
3 comments
2 participants
participants (2)
-
Jiri Denemark -
Peter Krempa