On 21/11/12 17:03, Paolo Bonzini wrote: > Il 21/11/2012 10:15, Kevin Wolf ha scritto: >>>> + if ((bs->open_flags & BDRV_O_NOCACHE)) { >>>> + bs->file->buffer_alignment = align; >>>> + } >> Any reason to restrict this to BDRV_O_NOCACHE? >> >> There have been patches to change the BDRV_O_NOCACHE flag from the >> monitor, in which case bdrv_set_buffer_alignment() wouldn't be called >> anew and O_DIRECT requests start to fail again. >> > > bdrv_set_buffer_alignment() is completely broken. It should set host > alignment, but in fact it is passed the guest alignment. > > In practice, we only support logical_block_size matching the host's or > bigger (which is unsafe due to torn writes, but works).
For other reasons (partition table format) we want to have host block size == guest block size on s390 anyway - so it would not really matter for us. But I certainly agree that it makes more sense to use the host block size for the alignment checks. > So I suggest that we just look at writes outside the device models, and > "fix" them to always read a multiple of 4k. Wouldnt that cause performance regressions for block devices with 512 byte block size, because we read more than necessary. Wouldnt that also require read/update/write combinations for valid 512 byte writes? Christian