From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44778) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwVwW-0004Dp-BH for qemu-devel@nongnu.org; Wed, 01 Aug 2012 06:14:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwVwS-0007z9-Cw for qemu-devel@nongnu.org; Wed, 01 Aug 2012 06:14:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2633) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwVwS-0007z5-43 for qemu-devel@nongnu.org; Wed, 01 Aug 2012 06:14:12 -0400 Message-ID: <5019016F.1050909@redhat.com> Date: Wed, 01 Aug 2012 12:14:07 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1343127865-16608-1-git-send-email-pbonzini@redhat.com> <1343127865-16608-14-git-send-email-pbonzini@redhat.com> In-Reply-To: <1343127865-16608-14-git-send-email-pbonzini@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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: jcody@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com Am 24.07.2012 13:03, schrieb Paolo Bonzini: > The following behaviors are possible: > > 'report': The behavior is the same as in 1.1. An I/O error, > respectively during a read or a write, will complete the job immediately > with an error code. > > 'ignore': An I/O error, respectively during a read or a write, will be > ignored. For streaming, the job will complete with an error and the > backing file will be left in place. For mirroring, the sector will be > marked again as dirty and re-examined later. > > 'stop': The job will be paused and the job iostatus will be set to > failed or nospace, while the VM will keep running. This can only be > specified if the block device has rerror=stop and werror=stop or enospc. > > 'enospc': Behaves as 'stop' for ENOSPC errors, 'report' for others. > > In all cases, even for 'report', the I/O error is reported as a QMP > event BLOCK_JOB_ERROR, with the same arguments as BLOCK_IO_ERROR. > > It is possible that while stopping the VM a BLOCK_IO_ERROR event will be > reported and will clobber the event from BLOCK_JOB_ERROR, or vice versa. > This is not really avoidable since stopping the VM completes all pending > I/O requests. In fact, it is already possible now that a series of > BLOCK_IO_ERROR events are reported with rerror=stop, because vm_stop > calls bdrv_drain_all and this can generate further errors. > > Signed-off-by: Paolo Bonzini 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. > +BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs, > + BlockdevOnError on_err, > + int is_read, int error) > +{ > + BlockErrorAction action; > + > + switch (on_err) { > + case BLOCKDEV_ON_ERROR_ENOSPC: > + action = (error == ENOSPC) ? BDRV_ACTION_STOP : BDRV_ACTION_REPORT; > + break; > + case BLOCKDEV_ON_ERROR_STOP: > + action = BDRV_ACTION_STOP; > + break; > + case BLOCKDEV_ON_ERROR_REPORT: > + action = BDRV_ACTION_REPORT; > + break; > + case BLOCKDEV_ON_ERROR_IGNORE: > + action = BDRV_ACTION_IGNORE; > + break; > + default: > + abort(); > + } > + 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. Kevin