On 10/04/2017 07:40 AM, Daniel P. Berrange wrote: > The Linux kernel will query the ATA IDENTITY DEVICE data, word 217 > to determine the rotations per minute of the disk. If this has > the value 1, it is taken to be an SSD and so Linux sets the > 'rotational' flag to 0 for the I/O queue and will stop using that > disk as a source of random entropy. Other operating systems may > also take into account rotation rate when setting up default > behaviour. > > Mgmt apps should be able to set the rotation rate for virtualized > block devices, based on characteristics of the host storage in use, > so that the guest OS gets sensible behaviour out of the box. This > patch thus adds a 'rotation-rate' parameter for 'ide-hd' device > types. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > hw/ide/core.c | 1 + > hw/ide/qdev.c | 1 + > include/hw/ide/internal.h | 8 ++++++++ > 3 files changed, 10 insertions(+) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 5f1cd3b91f..a04766aee7 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -208,6 +208,7 @@ static void ide_identify(IDEState *s) > if (dev && dev->conf.discard_granularity) { > put_le16(p + 169, 1); /* TRIM support */ > } > + put_le16(p + 217, dev->rotation_rate); /* Nominal media rotation rate */ > > ide_identify_size(s); > s->identify_set = 1; > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c > index d60ac25be0..a5181b4448 100644 > --- a/hw/ide/qdev.c > +++ b/hw/ide/qdev.c > @@ -299,6 +299,7 @@ static Property ide_hd_properties[] = { > DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf), > DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans", > IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO), > + DEFINE_PROP_UINT16("rotation_rate", IDEDrive, dev.rotation_rate, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h > index e641012b48..31851b44d1 100644 > --- a/include/hw/ide/internal.h > +++ b/include/hw/ide/internal.h > @@ -508,6 +508,14 @@ struct IDEDevice { > char *serial; > char *model; > uint64_t wwn; > + /* > + * 0x0000 - rotation rate not reported > + * 0x0001 - non-rotating medium (SSD) > + * 0x0002-0x0400 - reserved > + * 0x0401-0xffe - rotations per minute > + * 0xffff - reserved > + */ > + uint16_t rotation_rate; > }; > > /* These are used for the error_status field of IDEBus */ >
With Eric's comment addressed: Reviewed-by: John Snow <js...@redhat.com> It'd be nice if we could have a magic autodetect value, but this is a strict improvement anyway. (Actually, probably most of the identify data needs to be audited, but the perceived cost:benefit ratio doesn't look too favorable.)