[pve-devel] [PATCH pve-manager] Add "SSD" property to IDE, SATA, and SCSI hard drives

[Dominik Csapak]

After some discussion off-list, there did come up one thing:

would it not make more sense to set this on the storage level and
inherit it on the disk then? since the storage will most likely be
the defining factor for this setting

having this setting per disk in the config makes sense ofc,
but for the average user it would make more sense to set it
on the storage, no?


On 11/7/18 9:23 AM, Thomas Lamprecht wrote:
> On 11/6/18 5:45 PM, Nick Chevsky wrote:
>> On Tue, Nov 6, 2018 at 4:28 AM Thomas Lamprecht <t.lamprecht at proxmox.com>
>> wrote:
>>
>>> On 11/6/18 10:19 AM, Dominik Csapak wrote:
>>>> patch looks good, but maybe we want to put this in the advanced options
>>> instead ? do you have any strong opinion on that @thomas?
>>>
>>> Sounds OK, I guess - no strong feeling either way...
>>>
>>
>> I decided on the top section because:
>>
>>     1. Rotational vs. solid-state is a major defining property of a drive;
>>     if you were shopping for a physical one, this would probably be your first
>>     filter.
>>     2. This property affects how the drive is presented to the guest, unlike
>>     most of the advanced properties which are transparent to the guest.
>>     3. Putting this in the advanced section would've meant adding a new row
>>     to the already crowded bottom half of the dialog box.
>>
>> That said, I don't mind moving it at all—just let me know your final
>> thoughts.
> 
> In general I don't mind either way, your arguments sound OK and I
> do not find it intrusive, even OK, so it's Dominik's call.
> 
> But two things:
> 
> 1) I also have the checkbox enabled for SCSI with virtio-scsi controller,
>     and it's not needed in that case, AFAICT.
> 
> 2) I'd add a boxLabel with gettext('autodetected') (or similar) when it gets
>     disabled, so that user know that they just do not need to set it in those
>     cases, not that someone thinks that the virtio stuff is inferior because it
>     can't even handle SSDs properly :) Somewhat similar in principle to:
>     https://pve.proxmox.com/pipermail/pve-devel/2018-November/034260.html
> 
>>
>>> But in general an sentence/note in qm.adoc would be nice regarding this,
>>> SSD may be a bit confusing (see also below for another comment) else.
>>>
>>
>> I do have a pve-docs patch coming up; I wanted to wait until this one was
>> merged in case changes were requested that would affect the documentation.
>>
> 
> Awesome!
> 
>>
>>>>> +        xtype: 'proxmoxcheckbox',
>>>>> +        fieldLabel: gettext('SSD'),
>>>
>>> remove the gettext, SSD is verbatim OK and I cannot imagine translations
>>> of it.
>>>
>>
>> Done! I'll send an updated patch once you guys confirm your preference on
>> the location of the checkbox.
>>
> 
> OK, much thanks!
>