From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b1Bkq-0007Cg-QQ for qemu-devel@nongnu.org; Fri, 13 May 2016 07:59:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b1Bko-0006uG-8w for qemu-devel@nongnu.org; Fri, 13 May 2016 07:59:39 -0400 References: <1463089170-30550-1-git-send-email-eblake@redhat.com> <1463089170-30550-4-git-send-email-eblake@redhat.com> From: Max Reitz Message-ID: Date: Fri, 13 May 2016 13:59:26 +0200 MIME-Version: 1.0 In-Reply-To: <1463089170-30550-4-git-send-email-eblake@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="woheju3MLSQlxUvq8F4IfKE0ObqfFojBg" Subject: Re: [Qemu-devel] [PATCH 3/3] qemu-iotests: Fix regression in 136 on aio_read invalid List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: kwolf@redhat.com, qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --woheju3MLSQlxUvq8F4IfKE0ObqfFojBg From: Max Reitz To: Eric Blake , qemu-devel@nongnu.org Cc: kwolf@redhat.com, qemu-block@nongnu.org Message-ID: Subject: Re: [PATCH 3/3] qemu-iotests: Fix regression in 136 on aio_read invalid References: <1463089170-30550-1-git-send-email-eblake@redhat.com> <1463089170-30550-4-git-send-email-eblake@redhat.com> In-Reply-To: <1463089170-30550-4-git-send-email-eblake@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 12.05.2016 23:39, Eric Blake wrote: > Commit 093ea232 removed the ability for aio_read and aio_write > to artificially inflate the invalid statistics counters for > block devices, since it no longer flags unaligned offset or > length. Add 'aio_read -i' and 'aio_write -i' to restore > the ability, and update test 136 to use it. >=20 > Reported-by: Kevin Wolf > Signed-off-by: Eric Blake > --- > qemu-io-cmds.c | 20 ++++++++++++++++---- > tests/qemu-iotests/136 | 18 +++++++----------- > 2 files changed, 23 insertions(+), 15 deletions(-) >=20 > diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c > index 415be25..059b8ee 100644 > --- a/qemu-io-cmds.c > +++ b/qemu-io-cmds.c > @@ -1476,6 +1476,7 @@ static void aio_read_help(void) > " used to ensure all outstanding aio requests have been completed.\n" > " -C, -- report statistics in a machine parsable format\n" > " -P, -- use a pattern to verify read data\n" > +" -i, -- treat request as invalid, for exercising stats\n" > " -v, -- dump buffer to standard output\n" > " -q, -- quiet mode, do not show I/O statistics\n" > "\n"); > @@ -1488,7 +1489,7 @@ static const cmdinfo_t aio_read_cmd =3D { > .cfunc =3D aio_read_f, > .argmin =3D 2, > .argmax =3D -1, > - .args =3D "[-Cqv] [-P pattern] off len [len..]", > + .args =3D "[-Ciqv] [-P pattern] off len [len..]", > .oneline =3D "asynchronously reads a number of bytes", > .help =3D aio_read_help, > }; > @@ -1499,7 +1500,7 @@ static int aio_read_f(BlockBackend *blk, int argc= , char **argv) > struct aio_ctx *ctx =3D g_new0(struct aio_ctx, 1); >=20 > ctx->blk =3D blk; > - while ((c =3D getopt(argc, argv, "CP:qv")) !=3D -1) { > + while ((c =3D getopt(argc, argv, "CP:iqv")) !=3D -1) { > switch (c) { > case 'C': > ctx->Cflag =3D true; > @@ -1512,6 +1513,11 @@ static int aio_read_f(BlockBackend *blk, int arg= c, char **argv) > return 0; > } > break; > + case 'i': > + printf("injecting invalid read request\n"); > + block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ); > + g_free(ctx); > + return 0; > case 'q': > ctx->qflag =3D true; > break; > @@ -1569,6 +1575,7 @@ static void aio_write_help(void) > " -P, -- use different pattern to fill file\n" > " -C, -- report statistics in a machine parsable format\n" > " -f, -- use Force Unit Access semantics\n" > +" -i, -- treat request as invalid, for exercising stats\n" > " -q, -- quiet mode, do not show I/O statistics\n" > " -u, -- with -z, allow unmapping\n" > " -z, -- write zeroes using blk_aio_write_zeroes\n" > @@ -1582,7 +1589,7 @@ static const cmdinfo_t aio_write_cmd =3D { > .cfunc =3D aio_write_f, > .argmin =3D 2, > .argmax =3D -1, > - .args =3D "[-Cfquz] [-P pattern] off len [len..]", > + .args =3D "[-Cfiquz] [-P pattern] off len [len..]", > .oneline =3D "asynchronously writes a number of bytes", > .help =3D aio_write_help, > }; > @@ -1595,7 +1602,7 @@ static int aio_write_f(BlockBackend *blk, int arg= c, char **argv) > int flags =3D 0; >=20 > ctx->blk =3D blk; > - while ((c =3D getopt(argc, argv, "CfqP:uz")) !=3D -1) { > + while ((c =3D getopt(argc, argv, "CfiqP:uz")) !=3D -1) { > switch (c) { > case 'C': > ctx->Cflag =3D true; > @@ -1616,6 +1623,11 @@ static int aio_write_f(BlockBackend *blk, int ar= gc, char **argv) > return 0; > } > break; > + case 'i': > + printf("injecting invalid write request\n"); > + block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE); > + g_free(ctx); > + return 0; > case 'z': > ctx->zflag =3D true; > break; > diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136 > index e8c6937..5e92c4b 100644 > --- a/tests/qemu-iotests/136 > +++ b/tests/qemu-iotests/136 > @@ -226,18 +226,14 @@ sector =3D "%d" >=20 > highest_offset =3D wr_ops * wr_size >=20 > - # Two types of invalid operations: unaligned length and unalig= ned offset > - for i in range(invalid_rd_ops / 2): > - ops.append("aio_read 0 511") > + # Block layer abstracts away unaligned length and offset, so w= e > + # can't trigger an invalid op with any addresses; use qemu-io'= s > + # invalid injection feature instead This comment makes sense when seeing this patch, but it doesn't make a lot of sense in this file after the patch has been applied. We needed the comment to explain how the following commands are invalid requests, but after this patch it's obvious, so it would be completely fine to scratch the comment altogether. Or make it something short and simple like "Generate invalid requests". This being a comment and it just being a bit weird instead of wrong, I'd be fine with applying the patch as-is, though, if you'd rather not send a v2. What would you like it to be? :-) Max > + for i in range(invalid_rd_ops): > + ops.append("aio_read -i 0 512") >=20 > - for i in range(invalid_rd_ops / 2, invalid_rd_ops): > - ops.append("aio_read 13 512") > - > - for i in range(invalid_wr_ops / 2): > - ops.append("aio_write 0 511") > - > - for i in range(invalid_wr_ops / 2, invalid_wr_ops): > - ops.append("aio_write 13 512") > + for i in range(invalid_wr_ops): > + ops.append("aio_write -i 0 512") >=20 > for i in range(failed_rd_ops): > ops.append("aio_read %d 512" % bad_offset) >=20 --woheju3MLSQlxUvq8F4IfKE0ObqfFojBg Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJXNcGeAAoJEDuxQgLoOKytOPoH/A4fKyGet0wZUMyFRLfQwkC4 ZXTq3FmF6kbZVBshFEfJeHnRkM+Ij/0r0Dgt/W3ii0MrzDA+AK8Y1zYdsYHC+hv9 vVBlftswvfMef+xJQM7OQDSvJ24baSJAURf+FwgADEN9GmedatMtUwhoNtDhkm7t brLg40VdLbZH/YqN6j/IAGFe+dMwX3fsKemzlRetHHUc4nPDPGb3OkWUf5jrCnyG XOwt8pK7cwJ43oNsQhKTF4cOKz0OH89aVnE1tq1SZU1iHI4jCONGQ5fYqCqko9n5 VuntAg5BIxlMPETAV74Pr5qYJhhF6OnkEPaXQrE5g/AePKCu6TVFbBg02c+5vys= =eSKC -----END PGP SIGNATURE----- --woheju3MLSQlxUvq8F4IfKE0ObqfFojBg--