On 25.06.2018 15:07, Cornelia Huck wrote: > On Mon, 25 Jun 2018 09:42:11 -0300 > Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > >> It eases code review, unit is explicit. >> >> Patch generated using: >> >> $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/ >> >> and modified manually. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> Reviewed-by: Thomas Huth <th...@redhat.com> >> Acked-by: Cornelia Huck <coh...@redhat.com> > > Hm, I had not looked at the v2+ patches, as this already carried my > ack... > >> --- >> hw/s390x/s390-skeys.c | 3 ++- >> hw/s390x/s390-stattrib.c | 3 ++- > > ...but these two were added later on. > >> hw/s390x/sclp.c | 3 ++- >> 3 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c >> index 76241c240e..15f7ab0e53 100644 >> --- a/hw/s390x/s390-skeys.c >> +++ b/hw/s390x/s390-skeys.c >> @@ -10,6 +10,7 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qemu/units.h" >> #include "hw/boards.h" >> #include "hw/s390x/storage-keys.h" >> #include "qapi/error.h" >> @@ -19,7 +20,7 @@ >> #include "sysemu/kvm.h" >> #include "migration/register.h" >> >> -#define S390_SKEYS_BUFFER_SIZE 131072 /* Room for 128k storage keys */ >> +#define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage keys */ > > This one looks confusing to me. We're not allocating 128 chunks of 1 > KiB size, but space enough for 128k byte values. What do others think?
Hm, as this define is called "_SIZE" it should be the right thing to do. I would agree if it would be "_SKEY.*_COUNT" -- Thanks, David / dhildenb
