On Wed, Sep 20, 2017 at 04:45:35PM +0300, Nikolay Shirokovskiy wrote: > > > 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. Ah ok I see what's happening - its a short read in the sense that we have hit end-of-file, not a short read in the middle of the file. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
