On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
> Special error code helps gracefully handle race conditions on
> blockjob cancelling. Consider for example pull backup. We want
> to stop it and in the process we cancel blockjob for some of the
> exported disks. But at the time this command reaches qemu blockjob
> fail for some reason and cancel result and result of stopping
> will be an error with quite unexpecte message - no such job (sort of).
> Instead we can detect this race and treat as correct cancelling
> and wait until fail event will arrive. Then we can report corect
> error message with some details from qemu probably.
>
> Users of domainBlockJobAbort can be affected as -2 is new possible
> return code. Not sure if I should stick to the original behaviour in
> this case.
> ---
> src/qemu/qemu_monitor.c | 5 +++++
> src/qemu/qemu_monitor_json.c | 11 +++++++----
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
I haven't looked forward to patch 3/3 yet, but perhaps it'd be better in
the "caller" that cares to :
if (qemu*() < 0) {
virErrorPtr err = virGetLastError();
if (err && err->code == VIR_ERR_OPERATION_INVALID) {
/* Do something specific */
}
}
Returning -2 alters virDomainBlockJobAbort expected return values and
well that's probably not a good idea since we don't know if some caller
has said (if virDomainBlockJobAbort() == -1) instead of <= -1
John
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 4171914..944856d 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3335,6 +3335,11 @@ qemuMonitorBlockStream(qemuMonitorPtr mon,
> }
>
>
> +/* return:
> + * 0 in case of success
> + * -1 in case of general error
> + * -2 in case there is no such job
> + */
> int
> qemuMonitorBlockJobCancel(qemuMonitorPtr mon,
> const char *device,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 7903b47..42b05c4 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4351,6 +4351,7 @@ qemuMonitorJSONBlockJobError(virJSONValuePtr reply,
> if (qemuMonitorJSONErrorIsClass(error, "DeviceNotActive")) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> _("No active operation on device: %s"), device);
> + return -2;
> } else if (qemuMonitorJSONErrorIsClass(error, "DeviceInUse")) {
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("Device %s in use"), device);
> @@ -4408,6 +4409,11 @@ qemuMonitorJSONBlockStream(qemuMonitorPtr mon,
> }
>
>
> +/* return:
> + * 0 in case of success
> + * -1 in case of general error
> + * -2 in case there is no such job
> + */
> int
> qemuMonitorJSONBlockJobCancel(qemuMonitorPtr mon,
> const char *device,
> @@ -4426,10 +4432,7 @@ qemuMonitorJSONBlockJobCancel(qemuMonitorPtr mon,
> if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> goto cleanup;
>
> - if (qemuMonitorJSONBlockJobError(reply, cmd_name, device) < 0)
> - goto cleanup;
> -
> - ret = 0;
> + ret = qemuMonitorJSONBlockJobError(reply, cmd_name, device);
>
> cleanup:
> virJSONValueFree(cmd);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list