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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]

On 27.05.19 15:44, Anton Nefedov wrote:
> 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 <mreitz@redhat.com> 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?

That’s a good point.  Yes, file-posix should probably take an option to
override the mode.  Actually, that may be a useful option in general (if
the file is on some file system where we cannot query this information
(like glusterfs), the user may want to manually provide it).

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]