Re: [PATCH] block: make BlockConf.*_size properties 32-bit

[Eric Blake <eblake@redhat.com>]

On 2/13/20 7:55 AM, Roman Kagan wrote:
> On Thu, Feb 13, 2020 at 06:47:10AM -0600, Eric Blake wrote:
>> On 2/13/20 2:01 AM, Roman Kagan wrote:
>>> On Wed, Feb 12, 2020 at 03:44:19PM -0600, Eric Blake wrote:
>>>> On 2/11/20 5:54 AM, Roman Kagan wrote:
>>>>> Devices (virtio-blk, scsi, etc.) and the block layer are happy to use
>>>>> 32-bit for logical_block_size, physical_block_size, and min_io_size.
>>>>> However, the properties in BlockConf are defined as uint16_t limiting
>>>>> the values to 32768.
>>>>>
>>>>> This appears unnecessary tight, and we've seen bigger block sizes handy
>>>>> at times.
>>>>
>>>> What larger sizes?  I could see 64k or maybe even 1M block sizes,...
>>>
>>> We played exactly with these two :)
>>>
>>>>>
>>>>> Make them 32 bit instead and lift the limitation.

>>>>> @@ -716,30 +716,32 @@ const PropertyInfo qdev_prop_pci_devfn = {
>>>>>     /* --- blocksize --- */
>>>>> +#define MIN_BLOCK_SIZE 512
>>>>> +#define MAX_BLOCK_SIZE 2147483648
>>>>
>>>> ...but 2G block sizes are going to have tremendous performance problems.
>>>>
>>>> I'm not necessarily opposed to the widening to a 32-bit type, but think you
>>>> need more justification or a smaller number for the max block size,
>>>
>>> I thought any smaller value would just be arbitrary and hard to reason
>>> about, so I went ahead with the max value that fit in the type and could
>>> be made visibile to the guest.
>>
>> You've got bigger problems than what is visible to the guest. block/qcow2.c
>> operates on a cluster at a time; if you are stating that it now requires
>> reading multiple clusters to operate on one, qcow2 will have to do lots of
>> wasteful read-modify-write cycles.
> 
> I'm failing to see how this is supposed to happen.  The guest will issue
> requests bigger than the cluster size; why would it cause RMW?
> 
> Big logical_block_size would cause RMW in the guest if it wants to
> perform smaller writes, but that's up to the user to take this tradeoff,
> isn't it?
> 
>> You really need a strong reason to
>> support a maximum larger than 2M other than just "so the guest can
>> experiment with it".
> 
> Do I get you right that your suggestion is to cap the block size
> property at 2MB?

Yes, for now, I think 2M is a better maximum than 2G or 4G unless you 
have benchmark data to prove that a larger maximum does not cause problems.

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