Re: [libvirt] [PATCH 2/3] qemu: use blockjob completed event's error field to detect errors

On Wed, Oct 05, 2016 at 16:52:09 +0300, Nikolay Shirokovskiy wrote: > BLOCK_JOB_COMPLETED has error field set on error from day one (12bd451f) > thus there is no need to guess for error. Is it true that when > len == offset then can be no error? That is the question you should be able to answer when sending a patch, not a content for the commit message. AFAIK a copy job can fail even after all data was copied while syncing the buffers so this change makes sense. Additionally qemu also supports the BLOCK_JOB_ERROR event which is currently not handled by qemu. I'll need to have a look into it. > --- > src/qemu/qemu_monitor_json.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 6c2884d..8218e12 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -801,6 +801,7 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon, > int event) > { > const char *device; > + const char *error = NULL; > const char *type_str; > int type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; > unsigned long long offset, len; > @@ -834,8 +835,7 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon, > > switch ((virConnectDomainEventBlockJobStatus) event) { > case VIR_DOMAIN_BLOCK_JOB_COMPLETED: > - /* Make sure the whole device has been processed */ > - if (offset != len) > + if ((error = virJSONValueObjectGetString(data, "error"))) I'd prefer if you'd keep the original check as long as adding the check for the error field.

The 'error' variable is not used besides setting it twice, so you apparently don't need it at all.

> event = VIR_DOMAIN_BLOCK_JOB_FAILED; > break; > case VIR_DOMAIN_BLOCK_JOB_CANCELED: