Re: [Qemu-devel] [RFC 1/3] block: Add ImageRotationalInfo

[Anton Nefedov]

On 27/5/2019 3:57 PM, Max Reitz wrote:
> On 27.05.19 14:37, Alberto Garcia wrote:
>> On Mon 27 May 2019 02:16:53 PM CEST, Max Reitz wrote:
>>> On 26.05.19 17:08, Alberto Garcia wrote:
>>>> On Fri 24 May 2019 07:28:10 PM CEST, Max Reitz <address@hidden> wrote:
>>>>> +##
>>>>> +# @ImageRotationalInfo:
>>>>> +#
>>>>> +# Indicates whether an image is stored on a rotating disk or not.
>>>>> +#
>>>>> +# @solid-state: Image is stored on a solid-state drive
>>>>> +#
>>>>> +# @rotating:    Image is stored on a rotating disk
>>>>
>>>> What happens when you cannot tell? You assume it's solid-state?
>>>
>>> When *I* cannot tell?  This field is generally optional, so in that case
>>> it just will not be present.
>>>
>>> (When Linux cannot tell?  I don’t know :-))
>>>

Linux defaults to rotational == 1 unless the driver sets
QUEUE_FLAG_NONROT.

By the way as far as I can tell, qemu does not report this flag unless
explicitly set in a device property.

     DEFINE_PROP_UINT16("rotation_rate", IDEDrive, dev.rotation_rate, 0),
and
     DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),

>>> Do you think there should be an explicit value for that?
>>
>> I'll try to rephrase:
>>
>> we have a new optimization that improves performance on SSDs but reduces
>> performance on HDDs, so this series would detect where an image is
>> stored in order to enable the faster code path for each case.
>>
>> What happens if QEMU cannot detect if we have a solid drive or a
>> rotational drive? (e.g. a remote storage backend). Will it default to
>> enabling preallocation using write_zeroes()?
> 
> In this series, yes.  That is the default I chose.
> 
> We have to make a separate decision for each case.  In the case of
> filling newly allocated areas with zeroes, I think the performance gain
> for SSDs is more important than the performance loss for HDDs.  That is
> what I wrote in my response to Anton’s series.  So I took the series
> even without it being able to distinguish both cases at all.
> Consequentially, I believe it is reasonable for that to be the default
> behavior if we cannot tell.
> 
> I think in general optimizing for SSDs should probably be the default.
> HDDs are slow anyway, so whoever uses them probably doesn’t care about
> performance too much anyway...?  Whereas people using SSDs probably do.
>   But as I said, we can and should always make a separate decision for
> each case.
> 

Overall it looks good to me but I wonder how do we ensure both variants
are test covered? Need a blockdev option to enforce the mode?

/Anton