Re: [Qemu-devel] [PATCH v4 3/6] qemu-io: Allow unaligned access by default

[Eric Blake]

On 05/06/2016 09:59 AM, Max Reitz wrote:
> On 05.05.2016 05:42, Eric Blake wrote:
>> There's no reason to require the user to specify a flag just so
>> they can pass in unaligned numbers.  Keep 'read -p' and 'write -p'
>> as no-ops so that I don't have to hunt down and update all users
>> of qemu-io, but otherwise make their behavior default as 'read' and
>> 'write'.  Also fix 'write -z', 'readv', 'writev', 'writev',
>> 'aio_read', 'aio_write', and 'aio_write -z'.  For now, 'read -b',
>> 'multiwrite', 'write -b', and 'write -c' still require alignment.
>>
>> qemu-iotest 23 is updated to match, as the only test that was
>> previously explicitly expecting an error on an unaligned request.
>>

>> -" -p, -- allow unaligned access\n"
>> +" -p, -- ignored for back-compat\n"
> 
> We're not so close to the 80 character limit here to justify shortening
> this as much. ;-)
> 
> I really wouldn't mind a full "backwards compatibility".

sure, in all affected spots.


>> @@ -968,7 +951,7 @@ static const cmdinfo_t write_cmd = {
>>      .cfunc      = write_f,
>>      .argmin     = 2,
>>      .argmax     = -1,
>> -    .args       = "[-bcCpqz] [-P pattern ] off len",
>> +    .args       = "[-bcCqz] [-P pattern ] off len",
> 
> Would you mind removing the space after "pattern" along with this change?

Done


>> -    if (bflag + pflag + zflag > 1) {
>> -        printf("-b, -p, or -z cannot be specified at the same time\n");
>> +    if (bflag + zflag > 1) {
> 
> Could you make that "bflag && zflag" instead?
> 
> Adding booleans was fine for the meantime between patch 2 and this one,
> but I'd rather not keep it in the long run.

Testing that at most one of three booleans is set really needs addition
(the same expression using && and || is ugly and long); but you are
correct that for just two booleans, && is so much more legible.  Will fix.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature