Re: [Qemu-devel] [RFC 0/3] block: Inquire images’ rotational info

From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Anton Nefedov <anton.nefedov@virtuozzo.com>,
	Alberto Garcia <berto@igalia.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC 0/3] block: Inquire images’ rotational info
Date: Fri, 24 May 2019 12:52:45 -0500	[thread overview]
Message-ID: <8a76f460-8a16-4621-bbe2-c71bacbbe988@redhat.com> (raw)
In-Reply-To: <20190524172812.27308-1-mreitz@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]

On 5/24/19 12:28 PM, Max Reitz wrote:
> Hi,
> 
> http://lists.nongnu.org/archive/html/qemu-block/2019-05/msg00569.html
> shows that optimizations affect HDDs and SSDs differently.  It would be
> nice if we could differentiate between the two and then choose to adjust
> our behavior depending on whether a given image is stored on an HDD or
> not.
> 
> Or maybe it isn’t so nice.  That’s one reason this is an RFC.

Not an entirely bad idea. The NBD spec advertises whether an image is
rotational, precisely because it CAN make a difference for optimizing
access patterns. Likewise, commit 3b19f450 added support for ide
emulation to report rotation or non-rotational through to the guest.  So
plumbing it up in more places makes sense.

> 
> The other is that I implemented recognition of the rotational status by
> querying sysfs.  That looks stupid, but I didn’t find a better way
> (there is a BLKROTATIONAL ioctl, but that only works on device files).
> 
> But, hey, if you look through block/file-posix.c, you’ll find that I’m
> not the first to query sysfs.  hdev_get_max_segments() does so, too.  So
> maybe it isn’t that bad of an idea.
> 
> 
> What do you think?  Is the whole idea stupid?  Just the implementation?
> Or does it make sense?
> 

I'm in favor of the idea, whether or not review of the patches points
out tweaks to make before dropping RFC.

> 
> Max Reitz (3):
>   block: Add ImageRotationalInfo
>   file-posix: Inquire rotational status
>   qcow2: Evaluate rotational info
> 
>  qapi/block-core.json  | 19 ++++++++++-
>  block/qcow2.h         |  3 ++
>  include/block/block.h |  7 +++++
>  block.c               | 20 +++++++++++-
>  block/file-posix.c    | 73 +++++++++++++++++++++++++++++++++++++++++++
>  block/qapi.c          |  3 ++
>  block/qcow2.c         | 34 +++++++++++++++++---
>  7 files changed, 153 insertions(+), 6 deletions(-)

Given my comments about NBD, I'd expect a patch to block/nbd{,-client}.c
to expose this as well.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]