From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44084) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THELT-0007gM-Il for qemu-devel@nongnu.org; Thu, 27 Sep 2012 09:41:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1THELO-0000px-74 for qemu-devel@nongnu.org; Thu, 27 Sep 2012 09:41:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47623) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THELN-0000pl-Ur for qemu-devel@nongnu.org; Thu, 27 Sep 2012 09:41:34 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8RDfSJx019041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 Sep 2012 09:41:32 -0400 Message-ID: <50645785.3020600@redhat.com> Date: Thu, 27 Sep 2012 15:41:25 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1348675011-8794-1-git-send-email-pbonzini@redhat.com> <1348675011-8794-15-git-send-email-pbonzini@redhat.com> In-Reply-To: <1348675011-8794-15-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 14/45] block: introduce block job error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: jcody@redhat.com, qemu-devel@nongnu.org Am 26.09.2012 17:56, 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. Why is that? I don't see the dependency on rerror/werror in the code, and documentation doesn't mention it either. > '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 > --- > v1->v2: introduced block_job_iostatus_reset. Removed sorting > of iostatus values with "failed" overriding "nospace" but not > vice versa. Documented that block-job-resume clears the > iostatus field. Always set errors on the block job even if > they happen on the target; this removes the need to expose > the target's BlockInfo in "query-blockjobs". > +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(); > + } Isn't this a duplication of bdrv_get_error_action()? Kevin