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

[Nick Chevsky]

I think there may be some confusion here that the "SSD" flag should only be
used with storage that is actually backed by physical SSDs, and that's not
the case: Just like you can emulate hardware in a guest that you don't
actually have in the host, you may want to emulate SSDs in a guest even if
the storage is backed by rotational HDDs. Benefits of this include:

   - the ability to reclaim unused space on thin-provisioned storage*
   backed by any type of media,
   - guest features that may be arbitrarily restricted to SSDs (e.g. APFS
   in a certain fruity OS),
   - distinctive visual identification of SSDs in some desktop environments.

(*) Even though Proxmox currently restricts the use of the "Discard"
checkbox to SCSI drives, QEMU actually supports this on the IDE and AHCI
buses as well, and turning it on in combination with SSD emulation allows
thin-provisioned space to be reclaimed in guests that can't use the VirtIO
SCSI controller. This is extremely important, and the very reason I
developed these SSD patches in the first place.

Ultimately, I don't think this setting belongs in the storage layer because
it has no dependency on the backing storage, it can be used with backing
media of any type (even mixed), and people may want to have a mix of
HDD/SSD emulation in any given guest. As I explained in my previous reply
to Thomas, we should merely think of this as a new type of emulated
hardware device like any other.

Nick

On Wed, Nov 7, 2018 at 4:19 AM Dominik Csapak <d.csapak at proxmox.com> wrote:

> 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!
> >
>
>