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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux