Re: [Qemu-devel] [RFC 1/3] block: Add ImageRotationalInfo

[Max Reitz]

On 30.05.19 00:10, Kevin Wolf wrote:
> Am 24.05.2019 um 19:28 hat Max Reitz geschrieben:
>> This enum indicates whether a file is stored on a rotating disk or a
>> solid-state drive.  Drivers report it via the .bdrv_get_info() callback,
>> and if they do not, the global bdrv_get_info() implementation
>> automatically takes it from bs->file or bs->backing, if available.
> 
> Good that you wrote "bs->file or bs->backing" explicitly. Otherwise, I
> might have missed that it begs one big question: What is the correct
> answer for a qcow2 file that has bs->file on an SSD, but bs->backing on
> a rotating disk?
> 
> I don't think there is a correct answer for the whole device, so maybe
> this information shouldn't be per device in BlockDriverInfo, but per
> block in bdrv_co_block_status() (optionally determined if the caller
> requests it)?

I think that’s taking it a bit too far.  There is no heavy implication
in making the wrong choice here, it’s just a performance problem.
Having to call block_status for every block where we want to know what
to do seems like the opposite of performance optimization to me.  (We
could add a flag to block_status to only query that status, but that
sounds plainly wrong.)

So, in this series I decided that since all writes go to bs->file, that
seemed like what mostly determines the behavior of @bs.  (After my “Deal
with filters” series, that would become a decision of bdrv_storage_bs()
vs. bdrv_filtered_cow_bs().)

(Note that it has to get even funnier with vmdk, if your extents are on
an HDD, but your descriptor file is on an SSD.  I don’t care too much
about vmdk’s performance, though.)

In my v1, I’ll add a per-node @rotational parameter with which the user
can override the status we guessed.  In fact, currently, my commit
message explicitly notes that case:

https://git.xanclic.moe/XanClic/qemu/commit/0834f1ce77b4c27f0c00f1e4fbee099278e530b2

(Point 4)

(from
https://git.xanclic.moe/XanClic/qemu/commits/branch/spinning-rust-next)


Alternatively to making bs->file take precedence over bs->backing, we
could also just set the status to unknown if bs->file and bs->backing
differ in their status.


I think it’s generally better to prefer what bs->file says.  This is an
optimization case, so I think it’s more important to get it right most
of the time (and guess wrong sometimes) than to stop guessing in all
cases where we could be wrong.

Max

signature.asc
Description: OpenPGP digital signature