<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Jul 30, 2018 at 9:23 PM Eric Blake <<a href="mailto:eblake@redhat.com">eblake@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 07/30/2018 12:02 PM, Nir Soffer wrote:<br>
> Fix issues Eric found in the original patch:<br>
> <a href="https://www.redhat.com/archives/libguestfs/2018-July/msg00072.html" rel="noreferrer" target="_blank">https://www.redhat.com/archives/libguestfs/2018-July/msg00072.html</a><br>
> <br>
> - When handling ENODEV, the caller is expecting EOPNOTSUPP to trigger<br>
>    fallback.<br>
> - ENODEV should be ignored in file_trim.<br>
> <br>
> Tested only on Fedora 28 and RHEL 7.5.<br>
> ---<br>
>   plugins/file/file.c | 33 ++++++++++++++++++++++++---------<br>
>   1 file changed, 24 insertions(+), 9 deletions(-)<br>
> <br>
<br>
> +#if defined(FALLOC_FL_PUNCH_HOLE) || defined(FALLOC_FL_ZERO_RANGE)<br>
> +static int<br>
> +do_fallocate(int fd, int mode, off_t offset, off_t len)<br>
> +{<br>
> +  int r = -1;<br>
> +  r = fallocate (fd, mode, offset, len);<br>
<br>
Dead assignment to r in the declaration. Could merge these two lines <br>
into one.  Not necessarily worth a respin just for that.<br></blockquote><div><br></div><div>I tried to keep the style used in this file, but here it is indeed never needed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +  /* kernel 3.10 fails with ENODEV for block device. Kernel >= 4.9 fails<br>
> +     with EOPNOTSUPP in this case. Normalize errno to simplify callers. */<br>
<br>
Comment is slightly misleading - new enough kernels coupled with decent <br>
enough block device drivers actually succeed rather than failing. But <br>
I'm fine with checking in the comment as worded.<br></blockquote><div><br></div><div>The comment is only about the error flow. Moving it into the block would</div><div>avoid the confusion.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
ACK<br>
<br>
-- <br>
Eric Blake, Principal Software Engineer<br>
Red Hat, Inc.           <a href="tel:(919)%20301-3266" value="+19193013266" target="_blank">+1-919-301-3266</a><br>
Virtualization:  <a href="http://qemu.org" rel="noreferrer" target="_blank">qemu.org</a> | <a href="http://libvirt.org" rel="noreferrer" target="_blank">libvirt.org</a><br>
</blockquote></div></div>