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
