Re: [Qemu-devel] [PATCH v1 2/2] ide: support reporting of rotation rate

[Kevin Wolf]

[ Cc: qemu-block ]

Am 04.10.2017 um 13:40 hat Daniel P. Berrange geschrieben:
> 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 <address@hidden>
> ---
>  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 */

Coverity points out that all other dereferences of dev have a NULL check
first. Are we sure that it is always non-NULL?

A follow-up patch is necessary either way. Either fix the missing NULL
check here or remove useless NULL checks in the other places.

Kevin