From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56086) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJkAl-0003v8-AV for qemu-devel@nongnu.org; Thu, 14 Jan 2016 10:50:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJkAf-0002ma-Jy for qemu-devel@nongnu.org; Thu, 14 Jan 2016 10:50:51 -0500 References: <1452744489-3513-1-git-send-email-famz@redhat.com> <1452744489-3513-2-git-send-email-famz@redhat.com> <5697C2E5.2010908@redhat.com> From: Max Reitz Message-ID: <5697C3C8.9080706@redhat.com> Date: Thu, 14 Jan 2016 16:50:32 +0100 MIME-Version: 1.0 In-Reply-To: <5697C2E5.2010908@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0uJkVqWiFeJf3MlkkUQ87wDRT2GGVAidg" Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v3 1/2] blockdev: Error out on negative throttling option values List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Markus Armbruster , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --0uJkVqWiFeJf3MlkkUQ87wDRT2GGVAidg Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 14.01.2016 16:46, Max Reitz wrote: > On 14.01.2016 05:08, Fam Zheng wrote: >> The implicit casting from unsigned int to double changes negative valu= es >> into large positive numbers and accepts them. We should instead print= >> an error. >> >> Check the number range so this case is caught and reported. >> >> Signed-off-by: Fam Zheng >> --- >> blockdev.c | 3 ++- >> include/qemu/throttle.h | 2 ++ >> util/throttle.c | 16 ++++++---------- >> 3 files changed, 10 insertions(+), 11 deletions(-) >=20 > Reviewed-by: Max Reitz >=20 >> diff --git a/blockdev.c b/blockdev.c >> index 2df0c6d..1afef87 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -348,7 +348,8 @@ static bool check_throttle_config(ThrottleConfig *= cfg, Error **errp) >> } >> =20 >> if (!throttle_is_valid(cfg)) { >> - error_setg(errp, "bps/iops/maxs values must be 0 or greater")= ; >> + error_setg(errp, "bps/iops/maxs values must be within [0, %" = PRId64 Pre-existing, but you might want to fix this to "bps/iops/max" while touching this line. Max >> + ")", (int64_t)THROTTLE_VALUE_MAX); >=20 > I personally would have liked a simpler %lli and no cast, but I can see= > why you want an explicit int64_t here. >=20 >> return false; >> } >> =20 >> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h >> index 12faaad..d0c98ed 100644 >> --- a/include/qemu/throttle.h >> +++ b/include/qemu/throttle.h >> @@ -29,6 +29,8 @@ >> #include "qemu-common.h" >> #include "qemu/timer.h" >> =20 >> +#define THROTTLE_VALUE_MAX 1000000000000000LL >=20 > > But then you could use UINT64_C(1000000000000000) here. > >=20 --0uJkVqWiFeJf3MlkkUQ87wDRT2GGVAidg 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 iQEcBAEBCAAGBQJWl8PIAAoJEDuxQgLoOKytxWoH/iKwzU6jFzWI8nblOfr3l80t hoGLpC3jKZmuG3GKL/Gfp8SGuJr4ZKAgRfjzJlCi99z9V1JW+PAlcQvvwjeU9miD edMDl+Zz1GkjBxKT0Td4K5MCNz7BQ0Z4qWoHo7IdqqelJl8aiNG8B6r0TWVtIcb/ WTxxfcatScxLVNRV4YbzBgLpIIpVYLnl3Vw9AQ3AdMpMlqLFGznEP1hZT77AL1O1 o46dLkgj1vdbhFdBcrkzraqSn917qlLNUVGNpireWFOnZBw61BNo5xJEk15pyDyD Ghmij414B2a3aYZwgiG9Myt5OIfN0VHaxZu4sZjtIuBqfAhhpC3RxLiAJZTe2tE= =sZNZ -----END PGP SIGNATURE----- --0uJkVqWiFeJf3MlkkUQ87wDRT2GGVAidg--