Re: [PATCH 4/6] qemu-nbd: add compression flag support

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

01.10.2019 23:47, Eric Blake wrote:
> On 10/1/19 2:27 PM, Andrey Shinkevich wrote:
>> Added possibility to write compressed data by using the
>> blk_write_compressed. This action has the limitations of the format
>> driver. For example we can't write compressed data over other.
>>
> 
>> +++ b/blockdev-nbd.c
>> @@ -191,7 +191,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
>>       }
>>       exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, !writable,
>> -                         NULL, false, on_eject_blk, errp);
>> +                         0, NULL, false, on_eject_blk, errp);
> 
> This is a lot of parameters.  Should we be combining some of them into a struct, or even at least the booleans into a flags parameter?
> 
> 
>> +++ b/include/block/nbd.h
>> @@ -25,6 +25,10 @@
>>   #include "crypto/tlscreds.h"
>>   #include "qapi/error.h"
>> +enum {
>> +  NBD_INTERNAL_FLAG_COMPRESS = 1 << 1, /* Use write compressed */
>> +};
> 
> What happened to flag 1 << 0?  What other flags do you anticipate adding?

Hmm, any way, creating flags parameter for only one flag seems useless.

I think, I'd prefer to cover all nbd_export_new parameters to a structure
with boolean fields, to avoid extra &/| arithmetic.

nbd_export_new is called from qmp_nbd_server_add and qemu-nbd main(), and
both places seems to benefit, if they will fill structure instead of local
variables.

-- 
Best regards,
Vladimir