Re: [libvirt] [PATCH v2] iohelper: fix reading with O_DIRECT

On 20.09.2017 16:41, Daniel P. Berrange wrote: > On Wed, Sep 20, 2017 at 04:35:58PM +0300, Nikolay Shirokovskiy wrote: >> >> >> On 20.09.2017 16:30, Daniel P. Berrange wrote: >>> On Wed, Sep 20, 2017 at 02:58:55PM +0300, Nikolay Shirokovskiy wrote: >>>> saferead is not suitable for direct reads. If file size is not multiple >>>> of align size then we get EINVAL on the read(2) that is supposed to >>>> return 0 because read buffer will not be aligned at this point. >>>> >>>> Let's not read again after partial read and check that we read >>>> everything by comparing the number of total bytes read against file size. >>> >>> What scenario did you actually hit this problem in ? IIUC, we should >>> only be using O_DIRECT against block devices or plain files, and in both >>> those cases we should never see short-read unless we hit EOF. >> >> Yes. But saferead is generic function and rereads file after short read. >> Here we got EINVAL because of misalignement. > > I understand that, but how have you actually hit this in the real world. > AFAICT, the short-read and subsequent problem with misalignemt should be > impossible to hit, because any files we use O_DIRECT on should not ever > return a shortread. virsh restore --bypass-cache just fails (at least on my kernel). The problem is that if the state file size is not multiple of buffer then last read will definetly be short read.