Re: [libvirt PATCH 78/80] qemu: Implement VIR_DOMAIN_ABORT_JOB_POSTCOPY flag

2022-05-12 Thread Peter Krempa
On Thu, May 12, 2022 at 14:52:11 +0200, Peter Krempa wrote:
> On Tue, May 10, 2022 at 17:21:39 +0200, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/qemu/qemu_driver.c   | 43 +---
> >  src/qemu/qemu_monitor.c  |  9 
> >  src/qemu/qemu_monitor.h  |  3 +++
> >  src/qemu/qemu_monitor_json.c | 22 ++
> >  src/qemu/qemu_monitor_json.h |  3 +++
> >  5 files changed, 72 insertions(+), 8 deletions(-)
> 
> [...]

[...]

> Also add the trivial qemumonitorjsontest case so that this undergoes QMP
> schema validation, even when it's a trivial command.
> 
> With both of the above addressed:
> 
> Reviewed-by: Peter Krempa 

One other thing to consider would be to reject the API if the postcopy
flag is used but the migration is not post-copy or it's a backup job.



Re: [libvirt PATCH 78/80] qemu: Implement VIR_DOMAIN_ABORT_JOB_POSTCOPY flag

2022-05-12 Thread Peter Krempa
On Tue, May 10, 2022 at 17:21:39 +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_driver.c   | 43 +---
>  src/qemu/qemu_monitor.c  |  9 
>  src/qemu/qemu_monitor.h  |  3 +++
>  src/qemu/qemu_monitor_json.c | 22 ++
>  src/qemu/qemu_monitor_json.h |  3 +++
>  5 files changed, 72 insertions(+), 8 deletions(-)

[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 44361ae003..66e0b870b4 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3458,6 +3458,28 @@ int qemuMonitorJSONMigrateCancel(qemuMonitor *mon)
>  }
>  
>  
> +int
> +qemuMonitorJSONMigratePause(qemuMonitor *mon)
> +{
> +g_autoptr(virJSONValue) cmd = NULL;
> +g_autoptr(virJSONValue) reply = NULL;
> +
> +if (!(cmd = qemuMonitorJSONMakeCommand("migrate-pause", NULL)))
> +return -1;
> +
> +if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
> +return -1;
> +
> +if (qemuMonitorJSONHasError(reply, "DeviceNotFound"))
> +return -2;

Please add an explanation when this can happen and document the return
values in the function comment.

> +
> +if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +return -1;
> +
> +return 0;
> +}

Also add the trivial qemumonitorjsontest case so that this undergoes QMP
schema validation, even when it's a trivial command.

With both of the above addressed:

Reviewed-by: Peter Krempa