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 <[email protected]>
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!
_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
