From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Vn2-0000r0-2D for qemu-devel@nongnu.org; Fri, 26 Jun 2015 11:43:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8Vn1-000722-2a for qemu-devel@nongnu.org; Fri, 26 Jun 2015 11:43:39 -0400 Date: Fri, 26 Jun 2015 16:35:14 +0100 From: Stefan Hajnoczi Message-ID: <20150626153514.GC31186@stefanha-thinkpad.redhat.com> References: <1435018875-22527-1-git-send-email-jsnow@redhat.com> <1435018875-22527-7-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="xo44VMWPx7vlQ2+2" Content-Disposition: inline In-Reply-To: <1435018875-22527-7-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH 06/16] ahci: record ncq failures List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org --xo44VMWPx7vlQ2+2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 22, 2015 at 08:21:05PM -0400, John Snow wrote: > Handle NCQ failures for cases where we want to halt the VM on IO errors. >=20 > Signed-off-by: John Snow > --- > hw/ide/ahci.c | 17 +++++++++++++++-- > hw/ide/ahci.h | 1 + > hw/ide/internal.h | 1 + > 3 files changed, 17 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 71b5085..a838317 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -959,13 +959,25 @@ static void ncq_cb(void *opaque, int ret) > return; > } > =20 > + ncq_tfs->halt =3D false; Why does halt need to be cleared here? > if (ret < 0) { > - ncq_err(ncq_tfs); > + bool is_read =3D ncq_tfs->cmd =3D=3D READ_FPDMA_QUEUED; > + BlockErrorAction action =3D blk_get_error_action(ide_state->blk, > + is_read, -ret); > + if (action =3D=3D BLOCK_ERROR_ACTION_STOP) { > + ncq_tfs->halt =3D true; > + ide_state->bus->error_status =3D IDE_RETRY_HBA; > + } else if (action =3D=3D BLOCK_ERROR_ACTION_REPORT) { > + ncq_err(ncq_tfs); > + } > + blk_error_action(ide_state->blk, action, is_read, -ret); > } else { > ide_state->status =3D READY_STAT | SEEK_STAT; > } > =20 > - ncq_finish(ncq_tfs); > + if (!ncq_tfs->halt) { > + ncq_finish(ncq_tfs); > + } > } > =20 > static int is_ncq(uint8_t ata_cmd) > @@ -1042,6 +1054,7 @@ static void process_ncq_command(AHCIState *s, int p= ort, uint8_t *cmd_fis, > } > =20 > ncq_tfs->used =3D 1; > + ncq_tfs->halt =3D false; > ncq_tfs->drive =3D ad; > ncq_tfs->slot =3D slot; > ncq_tfs->cmd =3D ncq_fis->command; > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index 33607d7..47a3122 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -262,6 +262,7 @@ typedef struct NCQTransferState { > uint8_t cmd; > int slot; > int used; > + bool halt; > } NCQTransferState; > =20 > struct AHCIDevice { > diff --git a/hw/ide/internal.h b/hw/ide/internal.h > index 7a4a86d..5abee19 100644 > --- a/hw/ide/internal.h > +++ b/hw/ide/internal.h > @@ -499,6 +499,7 @@ struct IDEDevice { > #define IDE_RETRY_READ 0x20 > #define IDE_RETRY_FLUSH 0x40 > #define IDE_RETRY_TRIM 0x80 > +#define IDE_RETRY_HBA 0x100 Feel free to squash this patch together with the next one. It is hard to review in isolation since IDE_RETRY_HBA and ->halt aren't used yet. --xo44VMWPx7vlQ2+2 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVjXEyAAoJEJykq7OBq3PIHLIH/3FAcY7NTnGMj4MilaUoZuzF 98kLIQj9WKJIXhpet0WgH5UjKqCL1m0fk/qA/Mqfb/6wjpg5pzV/jRfiEr1uZB7/ rVij7n6cqyJd2NxEwcd9Gj8yuJmOpzBTkuopkf8Z+y4tKEUt77SdgdkKBLptHQF2 Wl3/yKGT5sF9VeW83osT8QApsLF+/qqWzgMOUwaaCovqiVsSuLJR7sqka0sEtT88 DXiLefxs8FDxcGJ9lji5J8zz3RrS4Qaz9H/13ZcH+HJMhOkJ30+haGCPbQUL2k75 DJJsPeaFU5tyPdlnqbiZ1N9Fix+SL30xMBR5paJhS+OUQzztsmmVxiwVJwAWBtQ= =PG11 -----END PGP SIGNATURE----- --xo44VMWPx7vlQ2+2--