From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:52802)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1SwWvm-0001Ih-Hr
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 07:17:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1SwWvi-0002No-Fp
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 07:17:34 -0400
Received: from mx1.redhat.com ([209.132.183.28]:17352)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1SwWvi-0002Nf-7O
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 07:17:30 -0400
Message-ID: <50191044.7070109@redhat.com>
Date: Wed, 01 Aug 2012 13:17:24 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1343127865-16608-1-git-send-email-pbonzini@redhat.com>
	<1343127865-16608-14-git-send-email-pbonzini@redhat.com>
	<5019016F.1050909@redhat.com>
In-Reply-To: <5019016F.1050909@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 13/47] block: introduce block job error
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: jcody@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com

Il 01/08/2012 12:14, Kevin Wolf ha scritto:
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> If we want to switch to named target block devices later, it would
> probably make sense to use the io_status of that block device rather
> than adding it to the job.
> 
> Maybe what results is a duplication that can be tolerated, though.

We are probably thinking of two opposite implementations.

You are thinking:

- errors in streaming, or in the mirroring source go to the block device

- errors in the mirroring target go to the block job

What I implemented is:

- errors in streaming, or in the mirroring source go to the block job

- errors in the mirroring target go to the target block device (which as
of this series could be inspected with query-block-jobs).


The reason is that an error in streaming or in the mirroring source does
not stop the VM.  A hypothetical management that polled for errors with
"info block" would see a mismatch between the error state ("failed") and
the VM RunState ("running").

So this is already ready for making the target block device visible.

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?...

Paolo

>> +    }
>> +    bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
>> +    if (action == BDRV_ACTION_STOP) {
>> +        block_job_pause(job);
>> +        if (bs == job->bs) {
>> +            block_job_iostatus_set_err(job, error);
>> +        } else {
>> +            bdrv_iostatus_set_err(bs, error);
>> +        }
> 
> However, so that everything just falls into place once we make the
> target block device visible, I'd make the bdrv_iostatus_set_err() call
> unconditional then.


Paolo