Re: [Qemu-devel] [PATCH v2 31/45] mirror: add support for on-source-error/on-target-error

[Kevin Wolf]

Am 18.10.2012 15:10, schrieb Paolo Bonzini:
> Il 18/10/2012 15:07, Kevin Wolf ha scritto:
>>>> +    s->synced = false;
>>>> +    if (read) {
>>>> +        return block_job_error_action(&s->common, s->common.bs,
>>>> +                                      s->on_source_error, true, error);
>>>> +    } else {
>>>> +        return block_job_error_action(&s->common, s->target,
>>>> +                                      s->on_target_error, false, error);
>> Here we produce an event that reports an error on s->bs, i.e. on the
>> source, even though the error was on the target.
> 
> More precisely, this is an event that reports an error on s->bs's job.
> In principle there is no reason why asynchronous long-running operations
> are tied to a block device (in fact migration fits the definition quite
> well, with the only twist that the VM is stopped at the end), but that's
> the API we're stuck with.

Yes, I think I mentioned already more than once that it shouldn't be
block job, but background job without a reference to a (single)
BlockDriverState. What we have just doesn't make any sense - even for
block jobs, because block jobs working on a single BDS are the
exception, not the rule.

Should probably have tried to fix this when I first mentioned it, but
too many incoming patches prevent that I do any change myself...

>> This makes some sense
>> today that the target doesn't have a name, but once it has, we would
>> better use the target name here.
>>
>> Can we change this later on? If not, what's the way forward?
> 
> Yes, we can change it to one of these:
> 
> 1) produce both a BLOCK_JOB_ERROR event on the source and a
> BLOCK_IO_ERROR event on the target;
> 
> 2) add a "device" argument to the BLOCK_JOB_ERROR and fill it.
> 
> I think I prefer the latter, but it can be discussed separately.

I already hate it again. But yeah, we can muddle through somehow, not a
blocker at this moment.

Kevin