Re: [Qemu-devel] [PATCH 13/47] block: introduce block job error

[Kevin Wolf]

Am 01.08.2012 14:09, schrieb Paolo Bonzini:
> Il 01/08/2012 13:49, Kevin Wolf ha scritto:
>>> The real question is: if I remove the possibility to inspect the (so far
>>> anonymous) target device with query-block-jobs, how do you read the
>>> status of the target device?...
>>
>> You don't? :-)
> 
> That's a possibility. :)
> 
> You can just report it in the block job.  It's more consistent with the
> fact that you got a BLOCK_JOB_ERROR and not a BLOCK_IO_ERROR.  So I
> would do:
> 
> +    bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR,
> +                              action, is_read);

Isn't job->bs the source?

Also, if you miss the event (e.g. libvirt crashed), can you still tell
which bs caused the error? Do we need another BlockJobInfo field for the
name (*cough* ;-)) of the failed bs?

If I understand it right, error reporting on the source and on the
target would be completely symmetrical then, which I think is a good thing.

job->bs makes one image special, which isn't really useful for a generic
interface. So we should keep the fact that it exists internal and get
rid of it sometime.

> +    if (action == BDRV_ACTION_STOP) {
> +        block_job_pause(job);
> +        block_job_iostatus_set_err(job, error);
> +        if (bs != job->bs) {
> +            bdrv_iostatus_set_err(bs, error);
> +        }
> +    }
> 
> where the bdrv_iostatus_set_err is mostly to "prepare for the future"
> usage of named block devices.

Again, I'd make it unconditional to get symmetric behaviour.

> As you said for ENOSPC vs. EIO, management must be ready to retry
> multiple times, if it has only the final state at its disposal.
> 
> On the other hand, if you see the exact sequence of BLOCK_IO_ERROR vs.
> BLOCK_JOB_ERROR you know exactly how the error happened and you can fix it.
> 
>> Maybe we should use named block devices from the beginning.
> 
> Hmm, but I'm a bit wary of introducing such a big change now.  We know
> what it makes nicer, but we don't know of anything irremediably broken
> without them, and we haven't thought enough of any warts it introduces.

On the one hand, I can understand your concerns, but on the other hand,
introducing an API now and then making such a big change afterwards
scares me much more.

Kevin