[Qemu-devel] [PATCH v7 7/9] qemu/units: add SI decimal units

Vladimir Sementsov-Ogievskiy posted 9 patches 6 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v7 7/9] qemu/units: add SI decimal units
Posted by Vladimir Sementsov-Ogievskiy 6 years, 4 months ago
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/qemu/units.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/qemu/units.h b/include/qemu/units.h
index 692db3fbb2..52ccc7445c 100644
--- a/include/qemu/units.h
+++ b/include/qemu/units.h
@@ -17,4 +17,11 @@
 #define PiB     (INT64_C(1) << 50)
 #define EiB     (INT64_C(1) << 60)
 
+#define SI_k 1000LL
+#define SI_M 1000000LL
+#define SI_G 1000000000LL
+#define SI_T 1000000000000LL
+#define SI_P 1000000000000000LL
+#define SI_E 1000000000000000000LL
+
 #endif
-- 
2.18.0


Re: [Qemu-devel] [PATCH v7 7/9] qemu/units: add SI decimal units
Posted by Eric Blake 6 years, 3 months ago
On 6/18/19 6:43 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/qemu/units.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/qemu/units.h b/include/qemu/units.h
> index 692db3fbb2..52ccc7445c 100644
> --- a/include/qemu/units.h
> +++ b/include/qemu/units.h
> @@ -17,4 +17,11 @@
>  #define PiB     (INT64_C(1) << 50)
>  #define EiB     (INT64_C(1) << 60)
>  
> +#define SI_k 1000LL
> +#define SI_M 1000000LL
> +#define SI_G 1000000000LL
> +#define SI_T 1000000000000LL
> +#define SI_P 1000000000000000LL
> +#define SI_E 1000000000000000000LL

Looks correct; and it's the sort of thing that if we do once here, we
don't have to repeat elsewhere. But bike-shedding a bit, would it be any
easier to read as:

#define SI_k 1000LL
#define SI_M (1000LL * 1000)
#define SI_G (1000LL * 1000 * 1000)
...

or even:

#define SI_k 1000LL
#define SI_M (SI_k * 1000)
#define SI_G (SI_M * 1000)
...

Also, would it be worth swapping out existing constants in the code base
that should instead be using these macros, so that they actually have a
use and so that we can see whether using them adds legibility?

For example, block/nvme.c, block/qapi.c, block/sheepdog.c, blockdev.c,
util/async.c, util/oslib-win32.c, util/qemu-thread-posix.c,
util/qemu-timer.c all seem to be dealing with conversions between
seconds and subdivisions thereof, where constants 1000000 or larger are
in use and could be rewritten with these.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v7 7/9] qemu/units: add SI decimal units
Posted by Peter Maydell 6 years, 3 months ago
On Fri, 9 Aug 2019 at 16:39, Eric Blake <eblake@redhat.com> wrote:
> Also, would it be worth swapping out existing constants in the code base
> that should instead be using these macros, so that they actually have a
> use and so that we can see whether using them adds legibility?
>
> For example, block/nvme.c, block/qapi.c, block/sheepdog.c, blockdev.c,
> util/async.c, util/oslib-win32.c, util/qemu-thread-posix.c,
> util/qemu-timer.c all seem to be dealing with conversions between
> seconds and subdivisions thereof, where constants 1000000 or larger are
> in use and could be rewritten with these.

I'm not sure that it would be more readable to replace
1000000000LL with SI_G -- I would tend to assume the latter
would be 2^30. Using "1000LL * 1000 * 1000" inline at
the point of use, or better still abstracting any particular use
into something more semantically meaningful as we already
do with NANOSECONDS_PER_SECOND, would be my personal preference.

thanks
-- PMM