On 04/28/2017 02:46 PM, Max Reitz wrote: > On 27.04.2017 03:46, Eric Blake wrote: >> For the 'alloc' command, accepting an offset in bytes but a length >> in sectors, and reporting output in sectors, is confusing. Do >> everything in bytes, and adjust the expected output accordingly. >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> >>
>> } >> + if (!QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)) { >> + printf("bytes %" PRId64 " is not sector aligned\n", > > This isn't real English. :-) But, it's just copy-and-paste from the other instances you just reviewed in 6/17! [Translation - if I change this one, I also get to redo that one] Which of these various alternatives (if any) looks better: bytes=511 is not sector-aligned 511 is not a sector-aligned value for 'bytes' requested 'bytes' of 511 is not sector-aligned alignment error: 511 bytes is not sector-aligned 'bytes' must be sector-aligned: 511 your clever entry here... > > With that fixed (somehow, you know better than me how to): Re-reading my various alternatives, I do think that /sector aligned/sector-aligned/ helps no matter what; and that the remaining trick is to use quoting or '=' or some other lexical trick to make it obvious that 'bytes' is a parameter name whose value 511 is invalid, rather than part of the actual error of a value that is not properly aligned. > > Reviewed-by: Max Reitz <mre...@redhat.com> If you state a preference for one of my variants, then the respin will use that variant consistently and add your R-b. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature