From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49345) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7ymC-0001fU-EY for qemu-devel@nongnu.org; Wed, 14 Mar 2012 20:42:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7ymA-0001TU-MX for qemu-devel@nongnu.org; Wed, 14 Mar 2012 20:42:44 -0400 Received: from spam1.wiktel.com ([69.89.207.151]:57778) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7ymA-0001TH-GE for qemu-devel@nongnu.org; Wed, 14 Mar 2012 20:42:42 -0400 From: Richard Laager In-Reply-To: <4F604B98.9090606@redhat.com> References: <1331226917-6658-1-git-send-email-pbonzini@redhat.com> <1331226917-6658-7-git-send-email-pbonzini@redhat.com> <4F5A31B2.3050701@redhat.com> <4F5A46A1.4000508@redhat.com> <1331402560.8577.46.camel@watermelon.coderich.net> <4F5DEBCE.3040409@redhat.com> <1331665990.24052.42.camel@watermelon.coderich.net> <4F604B98.9090606@redhat.com> Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-RiTis8z4c5A1GBbQ8kI0" Date: Wed, 14 Mar 2012 19:42:35 -0500 Message-ID: <1331772155.24052.849.camel@watermelon.coderich.net> Mime-Version: 1.0 Subject: Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org --=-RiTis8z4c5A1GBbQ8kI0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2012-03-14 at 08:41 +0100, Paolo Bonzini wrote: > Il 13/03/2012 20:13, Richard Laager ha scritto: > >>> > > * For SCSI, report an unmap_granularity to the guest as fol= lows: > >>> > > max(logical_block_size, discard_granularity) / logical_bloc= k_size > >> >=20 > >> > This is more or less already in place later in the series. > > I didn't see it. Which patch number? >=20 > Patch 11: I was saying QEMU should pass the discard_granularity to the guest as OPTIMAL UNMAP GRANULARITY. This would almost surely need to be done in hw/scsi-disk.c, roughly around this change from your patch 10: --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1295,8 +1295,11 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r) outbuf[13] =3D get_physical_block_exp(&s->qdev.conf); =20 /* set TPE bit if the format supports discard */ - if (s->qdev.conf.discard_granularity) { + if (s->qdev.type =3D=3D TYPE_DISK && s->qdev.conf.discard_gran= ularity) { outbuf[14] =3D 0x80; + if (s->qdev.conf.discard_zeroes_data) { + outbuf[14] |=3D 0x40; + } } =20 /* Protection, exponent and lowest lba field left blank. */ The code from patch 11 is more along the lines of what I think QEMU should have in the block layer: > + } else if (discard_granularity < s->qdev.conf.logical_block_size) { > + error_report("scsi-block: invalid discard_granularity"); > + return -1; > > + } else if (discard_granularity & (discard_granularity - 1)) { > + error_report("scsi-block: discard_granularity not a power of two= "); > + return -1; > + } However, I think the first check is unnecessarily restrictive. As long as discard_granularity is a power of two, if it's less than the block size (which is also a power of two), the block size will always be a multiple of discard_granularity, so there's no problem. I'd also like to explicitly allow discard_granularity =3D 1, which is what fallocate() provides. > It is worse in that we do not want the hardware parameters exposed to the > guest to change behind the scenes, except if you change the machine type > or if you use the default unversioned type. You're saying that discard_granularity and discard_zeros_data need to be properties of the machine type? If you start with that as a requirement, I can see why you want to always report discard_granularity=3D512 & discard_zeros_data=3D1. But that design has many downsides. I'm not convinced that discard_granularity and discard_zeros_data need to be properties of the machine type. Why do you feel that's necessary? What's the harm in those properties changing across QEMU startups (i.e. guest boots)? --=20 Richard --=-RiTis8z4c5A1GBbQ8kI0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEABECAAYFAk9hOvAACgkQbfU6uV4fG84DpwCgtVOE5MSX0duqR35qQsxDTBJB wkYAoP2IcnH0FNt58ERxl2YKrWC45E9I =ib7V -----END PGP SIGNATURE----- --=-RiTis8z4c5A1GBbQ8kI0--