On 06.10.2016 11:02, Peter Krempa wrote:
> 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.
Like for safety reasons? Ok.
>
> The 'error' variable is not used besides setting it twice, so you
> apparently don't need it at all.
This is intentional. Or next patch will touch this place one more time.
If it does't matter then ok.
>
>> event = VIR_DOMAIN_BLOCK_JOB_FAILED;
>> break;
>> case VIR_DOMAIN_BLOCK_JOB_CANCELED:
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list