[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v10 07/17] qemu-io: Switch 'alloc' command to by

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v10 07/17] qemu-io: Switch 'alloc' command to byte-based length
Date: Fri, 28 Apr 2017 15:36:49 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0

On 04/28/2017 03:09 PM, Max Reitz wrote:
> On 28.04.2017 21:59, Eric Blake wrote:
>> 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 <address@hidden>
>>>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>>>>      }
>>>> +    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]
> No, those are "offset" and "count" -- all singular. "bytes" is plural. ;-)

Then an obvious solution: s/bytes/count/ in the parameter name :)

But I still get to redo those, to add the '-' in 'sector-aligned'.

>> 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...
> How about "byte count" instead of "bytes" or "bytes value", if really
> want to have the exact spelling in there?
> For your entries above: (1) and (2) work for me (I like (2) a bit
> better), (3) doesn't sound like real English either, and it should be
> s/is/are/ in (4) (but it still sounds off with that change). (5) I
> mostly dislike because I dislike error message of the form "This should
> be X: $foo", I like "$foo is not X" better.

Maybe this variation of (3) solves the singular/plural disconnect:

request of 511 for 'bytes' is not sector-aligned

which makes it obvious that the "request of 511" (singular) and not the
parameter name (whether singular 'count' or plural 'bytes') is the
subject.  But it's a bit wordier than (2).  So it looks like (2) may be
a winner in all the situations.  But I also think you convinced me to
rename the command parameter; in my next spin, the help text will read:

alloc offset [count] -- checks if offset is allocated in the file

which starts to be non-trivial enough to drop R-b that you were willing
to give for just an error message wording change.

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]