Re: [libvirt] [PATCH v2 22/25] qemu: driver: Allow cancellation of the backup job
On 12/3/19 11:17 AM, Peter Krempa wrote: Use the helper which cancels all blockjobs to perform the backup job cancellation in qemuDomainAbortJob. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 22/25] qemu: driver: Allow cancellation of the backup job
On Thu, Dec 05, 2019 at 02:18:38PM +0100, Peter Krempa wrote:
On Wed, Dec 04, 2019 at 11:20:55 +, Daniel Berrange wrote:
On Tue, Dec 03, 2019 at 06:17:44PM +0100, Peter Krempa wrote:
> Use the helper which cancels all blockjobs to perform the backup job
> cancellation in qemuDomainAbortJob.
>
> Signed-off-by: Peter Krempa
> ---
> src/qemu/qemu_driver.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 95882d9d14..2408b08106 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14054,11 +14054,16 @@ static int qemuDomainAbortJob(virDomainPtr dom)
> }
>
> VIR_DEBUG("Cancelling job at client request");
> -qemuDomainObjAbortAsyncJob(vm);
> -qemuDomainObjEnterMonitor(driver, vm);
> -ret = qemuMonitorMigrateCancel(priv->mon);
> -if (qemuDomainObjExitMonitor(driver, vm) < 0)
> -ret = -1;
> +if (priv->job.asyncJob == QEMU_ASYNC_JOB_BACKUP) {
> +qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
> +ret = 0;
> +} else {
> +qemuDomainObjAbortAsyncJob(vm);
> +qemuDomainObjEnterMonitor(driver, vm);
> +ret = qemuMonitorMigrateCancel(priv->mon);
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +ret = -1;
> +}
Hmm, this makes me thing we should have had some better error checking
in here already. IIUC, we have other types async job that are not
related to either migration or backups, so should we do
switch (priv->job.asyncJob) {
case QEMU_ASYNC_JOB_BACKUP:
...
case QEMU_ASYNC_JOB_MIGRATE:
...
case QEMU_ASYNC_JOB
default:
report error
This is now done upstream. The new version of the patch after merging
with upstream:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 93b6107f6c..72694dc8d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14091,7 +14091,8 @@ static int qemuDomainAbortJob(virDomainPtr dom)
break;
case QEMU_ASYNC_JOB_BACKUP:
-/* TODO: to be implemented later */
+qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
+ret = 0;
break;
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 22/25] qemu: driver: Allow cancellation of the backup job
On Wed, Dec 04, 2019 at 11:20:55 +, Daniel Berrange wrote:
> On Tue, Dec 03, 2019 at 06:17:44PM +0100, Peter Krempa wrote:
> > Use the helper which cancels all blockjobs to perform the backup job
> > cancellation in qemuDomainAbortJob.
> >
> > Signed-off-by: Peter Krempa
> > ---
> > src/qemu/qemu_driver.c | 15 ++-
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 95882d9d14..2408b08106 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -14054,11 +14054,16 @@ static int qemuDomainAbortJob(virDomainPtr dom)
> > }
> >
> > VIR_DEBUG("Cancelling job at client request");
> > -qemuDomainObjAbortAsyncJob(vm);
> > -qemuDomainObjEnterMonitor(driver, vm);
> > -ret = qemuMonitorMigrateCancel(priv->mon);
> > -if (qemuDomainObjExitMonitor(driver, vm) < 0)
> > -ret = -1;
> > +if (priv->job.asyncJob == QEMU_ASYNC_JOB_BACKUP) {
> > +qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
> > +ret = 0;
> > +} else {
> > +qemuDomainObjAbortAsyncJob(vm);
> > +qemuDomainObjEnterMonitor(driver, vm);
> > +ret = qemuMonitorMigrateCancel(priv->mon);
> > +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> > +ret = -1;
> > +}
>
> Hmm, this makes me thing we should have had some better error checking
> in here already. IIUC, we have other types async job that are not
> related to either migration or backups, so should we do
>
>switch (priv->job.asyncJob) {
> case QEMU_ASYNC_JOB_BACKUP:
> ...
> case QEMU_ASYNC_JOB_MIGRATE:
> ...
> case QEMU_ASYNC_JOB
> default:
> report error
This is now done upstream. The new version of the patch after merging
with upstream:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 93b6107f6c..72694dc8d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14091,7 +14091,8 @@ static int qemuDomainAbortJob(virDomainPtr dom)
break;
case QEMU_ASYNC_JOB_BACKUP:
-/* TODO: to be implemented later */
+qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
+ret = 0;
break;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 22/25] qemu: driver: Allow cancellation of the backup job
On Tue, Dec 03, 2019 at 06:17:44PM +0100, Peter Krempa wrote:
> Use the helper which cancels all blockjobs to perform the backup job
> cancellation in qemuDomainAbortJob.
>
> Signed-off-by: Peter Krempa
> ---
> src/qemu/qemu_driver.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 95882d9d14..2408b08106 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14054,11 +14054,16 @@ static int qemuDomainAbortJob(virDomainPtr dom)
> }
>
> VIR_DEBUG("Cancelling job at client request");
> -qemuDomainObjAbortAsyncJob(vm);
> -qemuDomainObjEnterMonitor(driver, vm);
> -ret = qemuMonitorMigrateCancel(priv->mon);
> -if (qemuDomainObjExitMonitor(driver, vm) < 0)
> -ret = -1;
> +if (priv->job.asyncJob == QEMU_ASYNC_JOB_BACKUP) {
> +qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
> +ret = 0;
> +} else {
> +qemuDomainObjAbortAsyncJob(vm);
> +qemuDomainObjEnterMonitor(driver, vm);
> +ret = qemuMonitorMigrateCancel(priv->mon);
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +ret = -1;
> +}
Hmm, this makes me thing we should have had some better error checking
in here already. IIUC, we have other types async job that are not
related to either migration or backups, so should we do
switch (priv->job.asyncJob) {
case QEMU_ASYNC_JOB_BACKUP:
...
case QEMU_ASYNC_JOB_MIGRATE:
...
case QEMU_ASYNC_JOB
default:
report error
}
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 22/25] qemu: driver: Allow cancellation of the backup job
Use the helper which cancels all blockjobs to perform the backup job
cancellation in qemuDomainAbortJob.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 95882d9d14..2408b08106 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14054,11 +14054,16 @@ static int qemuDomainAbortJob(virDomainPtr dom)
}
VIR_DEBUG("Cancelling job at client request");
-qemuDomainObjAbortAsyncJob(vm);
-qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorMigrateCancel(priv->mon);
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
-ret = -1;
+if (priv->job.asyncJob == QEMU_ASYNC_JOB_BACKUP) {
+qemuBackupJobCancelBlockjobs(vm, priv->backup, true);
+ret = 0;
+} else {
+qemuDomainObjAbortAsyncJob(vm);
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorMigrateCancel(priv->mon);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+}
endjob:
qemuDomainObjEndJob(driver, vm);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
