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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]