On 20.09.2017 18:05, Daniel P. Berrange wrote:
> On Wed, Sep 20, 2017 at 06:00:59PM +0300, Nikolay Shirokovskiy wrote:
>>
>>
>> On 20.09.2017 17:48, Daniel P. Berrange wrote:
>>>
>>> IOW can simplify your patch to just this I believe:>
>>> diff --git a/src/util/iohelper.c b/src/util/iohelper.c
>>> index fe15a92e6..5416d4506 100644
>>> --- a/src/util/iohelper.c
>>> +++ b/src/util/iohelper.c
>>> @@ -109,7 +109,9 @@ runIO(const char *path, int fd, int oflags)
>>> while (1) {
>>> ssize_t got;
>>>
>>> - if ((got = saferead(fdin, buf, buflen)) < 0) {
>>> + if ((got = read(fdin, buf, buflen)) < 0) {
>>> + if (errno == EINTR)
>>> + continue;
>>> virReportSystemError(errno, _("Unable to read %s"), fdinname);
>>> goto cleanup;
>>> }
>>
>> But we still read half the buffer at the file end and then try misaligned read
>> after that.
>
> No, we shouldn't be mis-aligned. The 'buf' variable we use is aligned
> correctly. If we read half a buffer, we'll then write that data back
> to libvirtd. So when we go back into read(), we'll be reading into the
> start of 'buf' again, so we're still correctly aligned. This is the
> key difference against saferead() - that tries to fill up the buffer
> before we can write any data, whereas with this patch we'll always
> write whatever we read immediately
>
I've tested this patch and this works. From read(2)
EINVAL fd is attached to an object which is unsuitable for reading; or the file was opened with the O_DIRECT
flag, and either the address specified in buf, the value specified in count, or the current file off‐
set is not suitably aligned.
I thought everything should be aligned. But after giving the above sentence a thought - it is not stated )
So in case of EOF at least position may not be aligned.
But in open(2):
The O_DIRECT flag may impose alignment restrictions on the length and address of user-space buffers and the
file offset of I/Os. In Linux alignment restrictions vary by file system and kernel version and might be
absent entirely. However there is currently no file system-independent interface for an application to dis‐
cover these restrictions for a given file or file system. Some file systems provide their own interfaces
for doing so, for example the XFS_IOC_DIOINFO operation in xfsctl(3).
So finally it is not clear should such EOF read be possible or not. But I vote for the simple approach while it works.
Nikolay
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
