Libguestfs - Aug 2018 - Re: [PATCH v2 4/4] file: Zero for block devices on old kernels

On 08/03/2018 02:28 PM, Nir Soffer wrote:
> fallocate(FALLOC_FL_ZERO_RANGE) is supportd for block devices with
s/supportd/supported/
> modern kernel, but when it is not, we fall back to manual zeroing.
> 
> For block device, try also to use ioctl(BLKZEROOUT) if offset and count
> are aligned to block device sector size.
> 
> Here is an example run without this change on RHEL 7.5:
> 
> +++ b/plugins/file/file.c
> @@ -41,14 +41,21 @@
>   #include <unistd.h>
>   #include <sys/types.h>
>   #include <sys/stat.h>
> +#include <sys/ioctl.h>
Linux-specific header; will it cause grief on BSD compilation?

(POSIX declares ioctl() in <stropts.h>, but for the obsolete STREAMS 
extension that no one but Solaris ever implemented, and which no one 
uses today - and thus which has little bearing on the Linux use of ioctl).

Otherwise looks okay.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Mon, Aug 13, 2018 at 9:00 PM Eric Blake <eblake@redhat.com> wrote:
> On 08/03/2018 02:28 PM, Nir Soffer wrote:
> > fallocate(FALLOC_FL_ZERO_RANGE) is supportd for block devices with
>
> s/supportd/supported/
>
> > modern kernel, but when it is not, we fall back to manual zeroing.
> >
> > For block device, try also to use ioctl(BLKZEROOUT) if offset and
count
> > are aligned to block device sector size.
> >
> > Here is an example run without this change on RHEL 7.5:
> >
>
> > +++ b/plugins/file/file.c
> > @@ -41,14 +41,21 @@
> >   #include <unistd.h>
> >   #include <sys/types.h>
> >   #include <sys/stat.h>
> > +#include <sys/ioctl.h>
>
> Linux-specific header; will it cause grief on BSD compilation?
>
I did not know that, will wrap it with #ifdef __linux__

>
> (POSIX declares ioctl() in <stropts.h>, but for the obsolete STREAMS
> extension that no one but Solaris ever implemented, and which no one
> uses today - and thus which has little bearing on the Linux use of ioctl).
>
> Otherwise looks okay.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266 <(919)%20301-3266>
> Virtualization:  qemu.org | libvirt.org
>

On Tue, Aug 14, 2018 at 12:09 AM Nir Soffer <nsoffer@redhat.com> wrote:
> On Mon, Aug 13, 2018 at 9:00 PM Eric Blake <eblake@redhat.com> wrote:
>
>> On 08/03/2018 02:28 PM, Nir Soffer wrote:
>>
> ...
> > +++ b/plugins/file/file.c
>> > @@ -41,14 +41,21 @@
>> >   #include <unistd.h>
>> >   #include <sys/types.h>
>> >   #include <sys/stat.h>
>> > +#include <sys/ioctl.h>
>>
>> Linux-specific header; will it cause grief on BSD compilation?
>>
>
> I did not know that, will wrap it with #ifdef __linux__
>
I tested this on FreeBSD 11.0 and 10.3:

# cat test.c
#include <sys/ioctl.h>



$ cc -E test.c
# 1 "test.c"
# 1 "<built-in>" 1
# 1 "<built-in>" 3
# 316 "<built-in>" 3
# 1 "<command line>" 1
# 1 "<built-in>" 2
# 1 "test.c" 2
# 1 "/usr/include/sys/ioctl.h" 1 3 4
# 45 "/usr/include/sys/ioctl.h" 3 4
# 1 "/usr/include/sys/ioccom.h" 1 3 4
# 72 "/usr/include/sys/ioccom.h" 3 4
# 1 "/usr/include/sys/cdefs.h" 1 3 4
# 73 "/usr/include/sys/ioccom.h" 2 3 4


int ioctl(int, unsigned long, ...);
# 46 "/usr/include/sys/ioctl.h" 2 3 4
...

So ioctl() does come from <sys/ioctl.h> on FreeBSD.

Here is netbsd version:
https://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/sys/ioctl.h

OpenBSD:
https://github.com/openbsd/src/blob/master/sys/sys/ioctl.h

Looks like it is safe if our goal to be compatible at least with the BSDs.

>
>
>>
>> (POSIX declares ioctl() in <stropts.h>, but for the obsolete
STREAMS
>> extension that no one but Solaris ever implemented, and which no one
>> uses today - and thus which has little bearing on the Linux use of
ioctl).
>>
>