Support for HAVE_POSIX_FADVISE on Bacula Client

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Support for HAVE_POSIX_FADVISE on Bacula Client

Robert Heinzmann

Hello,

 

we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?)

 

From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices:

 

commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31

Author: Kern Sibbald <[hidden email]>

Date:   Thu May 24 19:58:07 2007 +0000

 

    kes  Add code to tell the OS that we no longer need a cached

         file that we were reading. In findlib/bfile.c.  Also,

         only cache files that we are reading.

    kes  Tweak to bsmtp to eliminate compiler warnings on Win32.

    kes  Implement script to automatically generate cats and dll .def

         files for Win32 dll.

    kes  Update README.mingw32 to include new .def file generation.

 

    git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89

 

bopen()

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)

   if (bfd->fid != -1 && flags & O_RDONLY) {

      int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);

      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);

   }

#endif

 

bclose()

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)

   if (bfd->m_flags & O_RDONLY) {

      fdatasync(bfd->fid);            /* sync the file */

      /* Tell OS we don't need it any more */

      posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);

   }

#endif

 

I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages).

 

I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request.

 

Is this a bug or a feature ?

 

How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ?

 

Regards,

Robert


------------------------------------------------------------------------------
Full-scale, agent-less Infrastructure Monitoring from a single dashboard
Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
Physical-Virtual-Cloud Infrastructure monitoring from one console
Real user monitoring with APM Insights and performance trend reports
Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Robert Heinzmann

Hello,

 

just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase.

 

The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; })

 

This diff fixes the issue:

 

diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c

index e315675..29f04a3 100644

--- a/bacula/src/findlib/bfile.c

+++ b/bacula/src/findlib/bfile.c

@@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode)

    bfd->win32DecompContext.liNextHeader = 0;

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)

-   if (bfd->fid != -1 && flags & O_RDONLY) {

+   if (bfd->fid != -1 && flags == O_RDONLY) {

       int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);

-      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);

+      Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat);

    }

#endif

 

@@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd)

       return 0;

    }

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)

-   if (bfd->m_flags & O_RDONLY) {

+   if (bfd->m_flags == O_RDONLY) {

       fdatasync(bfd->fid);            /* sync the file */

       /* Tell OS we don't need it any more */

       posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);

+      Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid);

    }

#endif

 

After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd –d 400 also shows the debugging messages.

 

However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also.

 

Current behavior:

-          backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases

-          backup Jobs without snapshots: backup job warms up the cache of all files

 

Behaviour with fix:

-          backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases

-          backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.)

 

 

Conclusion:

-          As the impact on behavior is huge, fixing the bug in place can be problematic

-          Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is “right” way to fix this issue

-          Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql  databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code).

 

Regards,

Robert

 

Robert Heinzmann

[hidden email]

IT-Operations

Travian Games GmbH

 

Von: Robert Heinzmann [mailto:r.he[hidden email]]
Gesendet: Mittwoch, 7. Oktober 2015 11:16
An: [hidden email]
Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client

 

Hello,

 

we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?)

 

From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices:

 

commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31

Author: Kern Sibbald <[hidden email]>

Date:   Thu May 24 19:58:07 2007 +0000

 

    kes  Add code to tell the OS that we no longer need a cached

         file that we were reading. In findlib/bfile.c.  Also,

         only cache files that we are reading.

    kes  Tweak to bsmtp to eliminate compiler warnings on Win32.

    kes  Implement script to automatically generate cats and dll .def

         files for Win32 dll.

    kes  Update README.mingw32 to include new .def file generation.

 

    git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89

 

bopen()

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)

   if (bfd->fid != -1 && flags & O_RDONLY) {

      int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);

      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);

   }

#endif

 

bclose()

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)

   if (bfd->m_flags & O_RDONLY) {

      fdatasync(bfd->fid);            /* sync the file */

      /* Tell OS we don't need it any more */

      posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);

   }

#endif

 

I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages).

 

I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request.

 

Is this a bug or a feature ?

 

How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ?

 

Regards,

Robert


------------------------------------------------------------------------------
Full-scale, agent-less Infrastructure Monitoring from a single dashboard
Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
Physical-Virtual-Cloud Infrastructure monitoring from one console
Real user monitoring with APM Insights and performance trend reports
Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Gary R. Schmidt-3
On 7/10/2015 10:57 PM, Robert Heinzmann wrote:

> Hello,
>
> just a short followup. It seems I found the issue causing our file
> system cache to fill up during backup jobs and our virtual platform
> memory usage to increase.
>
> The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to
> always fail. So the patch regarding posix_fadvise() in commit
> 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if
> (Anything & 00) { dead code; })
>
It's a real bug - O_RDONLY is 0 on Solaris, AIX, HP-UX, and Windows as well.

        Cheers,
                Gary B-)


------------------------------------------------------------------------------
Full-scale, agent-less Infrastructure Monitoring from a single dashboard
Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
Physical-Virtual-Cloud Infrastructure monitoring from one console
Real user monitoring with APM Insights and performance trend reports
Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Eric Bollengier
In reply to this post by Robert Heinzmann
Hello Robert,

I think you did a really nice investigation work :-) I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea.

Thanks,
Eric

Le 2015-10-07 13:57, Robert Heinzmann <[hidden email]> a écrit :

>
> Hello,
>
>  
>
> just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase.
>
>  
>
> The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; })
>
>  
>
> This diff fixes the issue:
>
>  
>
> diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c
>
> index e315675..29f04a3 100644
>
> --- a/bacula/src/findlib/bfile.c
>
> +++ b/bacula/src/findlib/bfile.c
>
> @@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode)
>
>     bfd->win32DecompContext.liNextHeader = 0;
>
>  
>
> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)
>
> -   if (bfd->fid != -1 && flags & O_RDONLY) {
>
> +   if (bfd->fid != -1 && flags == O_RDONLY) {
>
>        int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);
>
> -      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);
>
> +      Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat);
>
>     }
>
> #endif
>
>  
>
> @@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd)
>
>        return 0;
>
>     }
>
> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)
>
> -   if (bfd->m_flags & O_RDONLY) {
>
> +   if (bfd->m_flags == O_RDONLY) {
>
>        fdatasync(bfd->fid);            /* sync the file */
>
>        /* Tell OS we don't need it any more */
>
>        posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);
>
> +      Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid);
>
>     }
>
> #endif
>
>  
>
> After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd –d 400 also shows the debugging messages.
>
>  
>
> However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also.
>
>  
>
> Current behavior:
>
> -          backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases
>
> -          backup Jobs without snapshots: backup job warms up the cache of all files
>
>  
>
> Behaviour with fix:
>
> -          backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases
>
> -          backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.)
>
>  
>
>  
>
> Conclusion:
>
> -          As the impact on behavior is huge, fixing the bug in place can be problematic
>
> -          Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is “right” way to fix this issue
>
> -          Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql  databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code).
>
>  
>
> Regards,
>
> Robert
>
>  
>
> Robert Heinzmann
>
> [hidden email]
>
> IT-Operations
>
> Travian Games GmbH
>
>  
>
> Von: Robert Heinzmann [mailto:[hidden email]]
> Gesendet: Mittwoch, 7. Oktober 2015 11:16
> An: [hidden email]
> Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client
>
>  
>
> Hello,
>
>  
>
> we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?)
>
>  
>
> From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices:
>
>  
>
> commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31
>
> Author: Kern Sibbald <[hidden email]>
>
> Date:   Thu May 24 19:58:07 2007 +0000
>
>  
>
>     kes  Add code to tell the OS that we no longer need a cached
>
>          file that we were reading. In findlib/bfile.c.  Also,
>
>          only cache files that we are reading.
>
>     kes  Tweak to bsmtp to eliminate compiler warnings on Win32.
>
>     kes  Implement script to automatically generate cats and dll .def
>
>          files for Win32 dll.
>
>     kes  Update README.mingw32 to include new .def file generation.
>
>  
>
>     git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89
>
>  
>
> bopen()
>
>  
>
> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)
>
>    if (bfd->fid != -1 && flags & O_RDONLY) {
>
>       int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);
>
>       Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);
>
>    }
>
> #endif
>
>  
>
> bclose()
>
>  
>
> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)
>
>    if (bfd->m_flags & O_RDONLY) {
>
>       fdatasync(bfd->fid);            /* sync the file */
>
>       /* Tell OS we don't need it any more */
>
>       posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);
>
>    }
>
> #endif
>
>  
>
> I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages).
>
>  
>
> I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request.
>
>  
>
> Is this a bug or a feature ?
>
>  
>
> How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ?
>
>  
>
> Regards,
>
> Robert
------------------------------------------------------------------------------
Full-scale, agent-less Infrastructure Monitoring from a single dashboard
Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
Physical-Virtual-Cloud Infrastructure monitoring from one console
Real user monitoring with APM Insights and performance trend reports
Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Josh Fisher


On 10/7/2015 10:49 AM, Eric Bollengier wrote:
> Hello Robert,
>
> I think you did a really nice investigation work :-) I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea.

Definitely a good job in finding the bug. I still believe that it is
best to treat bitwise flags as bitwise flags. For example,:

    if (bfd->fid != -1 && (flags & O_RDONLY) == O_RDONLY)

However, Robert's patch is correct to use:

    if (bfd->fid != -1 && flags == O_RDONLY)

since flags is an 'access mode', and not bitwise values or'd together.
Still, had it been properly treated as a convolution of bitwise values,
it would not have resulted in a bug

And, yes, the OS will cache the file with or without the FADV_DONTNEED
call. It is a question of how long it will remain cached. With the
FADV_DONTNEED call, each file is cached and then immediately released
before the next file read. Without the FADV_DONTNEED, the OS will cache
as many files as it possibly can.

My problem with this approach is that it probably degrades performance
for small files, particularly with many small files. It forces fdatasync
and fadvise system calls after each file. If that is what is needed,
then perhaps it is better to just open with O_SYNC | O_DIRECT and avoid
caching in the first place.

In any case, it is going to perform very differently backing up huge
files that it does backing up many small files. So I agree with Robert
that it should have a directive.


>
> Thanks,
> Eric
>
> Le 2015-10-07 13:57, Robert Heinzmann <[hidden email]> a écrit :
>> Hello,
>>
>>  
>>
>> just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase.
>>
>>  
>>
>> The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; })
>>
>>  
>>
>> This diff fixes the issue:
>>
>>  
>>
>> diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c
>>
>> index e315675..29f04a3 100644
>>
>> --- a/bacula/src/findlib/bfile.c
>>
>> +++ b/bacula/src/findlib/bfile.c
>>
>> @@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode)
>>
>>      bfd->win32DecompContext.liNextHeader = 0;
>>
>>  
>>
>> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)
>>
>> -   if (bfd->fid != -1 && flags & O_RDONLY) {
>>
>> +   if (bfd->fid != -1 && flags == O_RDONLY) {
>>
>>         int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);
>>
>> -      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);
>>
>> +      Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat);
>>
>>      }
>>
>> #endif
>>
>>  
>>
>> @@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd)
>>
>>         return 0;
>>
>>      }
>>
>> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)
>>
>> -   if (bfd->m_flags & O_RDONLY) {
>>
>> +   if (bfd->m_flags == O_RDONLY) {
>>
>>         fdatasync(bfd->fid);            /* sync the file */
>>
>>         /* Tell OS we don't need it any more */
>>
>>         posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);
>>
>> +      Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid);
>>
>>      }
>>
>> #endif
>>
>>  
>>
>> After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd –d 400 also shows the debugging messages.
>>
>>  
>>
>> However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also.
>>
>>  
>>
>> Current behavior:
>>
>> -          backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases
>>
>> -          backup Jobs without snapshots: backup job warms up the cache of all files
>>
>>  
>>
>> Behaviour with fix:
>>
>> -          backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases
>>
>> -          backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.)
>>
>>  
>>
>>  
>>
>> Conclusion:
>>
>> -          As the impact on behavior is huge, fixing the bug in place can be problematic
>>
>> -          Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is “right” way to fix this issue
>>
>> -          Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql  databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code).
>>
>>  
>>
>> Regards,
>>
>> Robert
>>
>>  
>>
>> Robert Heinzmann
>>
>> [hidden email]
>>
>> IT-Operations
>>
>> Travian Games GmbH
>>
>>  
>>
>> Von: Robert Heinzmann [mailto:[hidden email]]
>> Gesendet: Mittwoch, 7. Oktober 2015 11:16
>> An: [hidden email]
>> Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client
>>
>>  
>>
>> Hello,
>>
>>  
>>
>> we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?)
>>
>>  
>>
>>  From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices:
>>
>>  
>>
>> commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31
>>
>> Author: Kern Sibbald <[hidden email]>
>>
>> Date:   Thu May 24 19:58:07 2007 +0000
>>
>>  
>>
>>      kes  Add code to tell the OS that we no longer need a cached
>>
>>           file that we were reading. In findlib/bfile.c.  Also,
>>
>>           only cache files that we are reading.
>>
>>      kes  Tweak to bsmtp to eliminate compiler warnings on Win32.
>>
>>      kes  Implement script to automatically generate cats and dll .def
>>
>>           files for Win32 dll.
>>
>>      kes  Update README.mingw32 to include new .def file generation.
>>
>>  
>>
>>      git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89
>>
>>  
>>
>> bopen()
>>
>>  
>>
>> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)
>>
>>     if (bfd->fid != -1 && flags & O_RDONLY) {
>>
>>        int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);
>>
>>        Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);
>>
>>     }
>>
>> #endif
>>
>>  
>>
>> bclose()
>>
>>  
>>
>> #if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)
>>
>>     if (bfd->m_flags & O_RDONLY) {
>>
>>        fdatasync(bfd->fid);            /* sync the file */
>>
>>        /* Tell OS we don't need it any more */
>>
>>        posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);
>>
>>     }
>>
>> #endif
>>
>>  
>>
>> I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages).
>>
>>  
>>
>> I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request.
>>
>>  
>>
>> Is this a bug or a feature ?
>>
>>  
>>
>> How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ?
>>
>>  
>>
>> Regards,
>>
>> Robert
> ------------------------------------------------------------------------------
> Full-scale, agent-less Infrastructure Monitoring from a single dashboard
> Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
> Physical-Virtual-Cloud Infrastructure monitoring from one console
> Real user monitoring with APM Insights and performance trend reports
> Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
> _______________________________________________
> Bacula-users mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/bacula-users


------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Robert Heinzmann
In reply to this post by Eric Bollengier
Hi,

> I think you did a really nice investigation work :-)
Thanks ... was fun :)

> I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea.

the point is, does it cache the file and evict the cache after the backup of the file (what is with posix call)

Example WITH fix:
----------------------

  Server: 8 GB unused memory
  Running Application (aka workload) = 4 of working set and cached files
  Bacula backing up 800 x 100 MB FILE (8 GB in total)

=> After backup

Running Application:
  If bacula backs up the working set data: 0 GB cached
  If bacula backs up other sets of data: 4 GB cached (e.g. a Snapshot of the dataset which also has new filehandles) <=== Our case !!!!
Bacula 100MB of cached (and never reused) data

OR

does bacula polute the whole file system cache

Example WITHOUT fix:
-----------------------------

  Server: 8 GB unused memory
  Running Application (aka workload) = 4 of working set and cached files
  Bacula backing up 800 x 100 MB FILE (8 GB in total)

=> After backup

Running Application: 0 GB cached
Bacula 8 GB of cached (and never reused) data


Regards,
Robert

Robert Heinzmann
[hidden email]
IT-Operations
Travian Games GmbH

-----Ursprüngliche Nachricht-----
Von: Eric Bollengier [mailto:[hidden email]]
Gesendet: Mittwoch, 7. Oktober 2015 16:50
An: Robert Heinzmann <[hidden email]>
Cc: [hidden email] <[hidden email]>
Betreff: Re: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client

Hello Robert,

I think you did a really nice investigation work :-) I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea.

Thanks,
Eric

------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Eric Bollengier
Hello Robert,

Le 08. 10. 15 13:54, Robert Heinzmann a écrit :

> Hi,
>
>> I think you did a really nice investigation work :-)
> Thanks ... was fun :)
>
>> I think that your patch is almost right, and we need to figure if we want to add specific directives for that. I tend to believe right now that with or without the POSIX call, the os will cache the file, and it sounds better to do it in advance, let see what other people are thinking about your idea.
>
> the point is, does it cache the file and evict the cache after the backup of the file (what is with posix call)
>
> Example WITH fix:
> ----------------------
>
>    Server: 8 GB unused memory
>    Running Application (aka workload) = 4 of working set and cached files
>    Bacula backing up 800 x 100 MB FILE (8 GB in total)
>
> => After backup
>
> Running Application:
>    If bacula backs up the working set data: 0 GB cached
>    If bacula backs up other sets of data: 4 GB cached (e.g. a Snapshot of the dataset which also has new filehandles) <=== Our case !!!!
> Bacula 100MB of cached (and never reused) data
>
> OR
>
> does bacula polute the whole file system cache
>
> Example WITHOUT fix:
> -----------------------------
>
>    Server: 8 GB unused memory
>    Running Application (aka workload) = 4 of working set and cached files
>    Bacula backing up 800 x 100 MB FILE (8 GB in total)
>
> => After backup
>
> Running Application: 0 GB cached
> Bacula 8 GB of cached (and never reused) data
>
>

I think that if I understand correctly what you wrote, the "fix" is
better in all cases (and I agree with you), so I'm not convinced that
we should have a directive.

Best Regards,
Eric

------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Kern Sibbald
In reply to this post by Robert Heinzmann
Hello,

Thanks for pointing this out. I still have problems imagining that they would define O_RDONLY as 0 in a bitmapped variable!!!

Unfortunately, the patch below will not work in all cases because O_RDONLY does not correspond to any status in the flags variable.

Tomorrow, I will post a fix to this to the public git repository.

Again, thanks for pointing out the problem and getting *very* close to a solution.

Best regards,
Kern

On 10/07/2015 04:57 AM, Robert Heinzmann wrote:

Hello,

 

just a short followup. It seems I found the issue causing our file system cache to fill up during backup jobs and our virtual platform memory usage to increase.

 

The O_RDONLY mask is “00” on Linux, casing the if flags & O_RDONLY to always fail. So the patch regarding posix_fadvise() in commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31 is actually dead code. (if (Anything & 00) { dead code; })

 

This diff fixes the issue:

 

diff --git a/bacula/src/findlib/bfile.c b/bacula/src/findlib/bfile.c

index e315675..29f04a3 100644

--- a/bacula/src/findlib/bfile.c

+++ b/bacula/src/findlib/bfile.c

@@ -997,9 +997,9 @@ int bopen(BFILE *bfd, const char *fname, int flags, mode_t mode)

    bfd->win32DecompContext.liNextHeader = 0;

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)

-   if (bfd->fid != -1 && flags & O_RDONLY) {

+   if (bfd->fid != -1 && flags == O_RDONLY) {

       int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);

-      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);

+      Dmsg2(400, "Did posix_fadvise POSIX_FADV_WILLNEED on %s stat=%d\n", fname, stat);

    }

#endif

 

@@ -1043,10 +1043,11 @@ int bclose(BFILE *bfd)

       return 0;

    }

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)

-   if (bfd->m_flags & O_RDONLY) {

+   if (bfd->m_flags == O_RDONLY) {

       fdatasync(bfd->fid);            /* sync the file */

       /* Tell OS we don't need it any more */

       posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);

+      Dmsg1(400, "Did posix_fadvise POSIX_FADV_DONTNEED on File Desriptor %d\n", bfd->fid);

    }

#endif

 

After applying this patch, the cache memory does not increase a lot during backups on my test machine. Running widh bacula-fd –d 400 also shows the debugging messages.

 

However this fix has a HUGE impact on behavior of existing backup jobs - altought it has huge benefits on virtual platforms with short backup windows also.

 

Current behavior:

-          backup jobs with snapshots: caching of unneeded files in memory, filesystem cache is polluted, virtual platform memory usage increases

-          backup Jobs without snapshots: backup job warms up the cache of all files

 

Behaviour with fix:

-          backup jobs with snapshots: only caching of 1 unneeded file in memory, filesystem cache is less polluted (largest file to backup), virtual platform memory usage decreases

-          backup Jobs without snapshots: backup job does not warms up the cache of all files, but also drops existing cache for cached files (may have performance impact for webservers, fileservers etc.)

 

 

Conclusion:

-          As the impact on behavior is huge, fixing the bug in place can be problematic

-          Unfortunately the right way to impelement posix_fadvise() with POSIX_FADV_NOREUSE is not implemented on Linux (See http://lxr.free-electrons.com/source/mm/fadvise.c#L115), so there is “right” way to fix this issue

-          Best approach: there should be a Bacula Option to enable the POSIX_FADV_NOREUSE explicitly for selective bacula Jobs to allow usage of posix_fadvise() for direct I/O workloads (like mysql  databases etc with snapshots etc.) and avoid it for webserver and fileserver workloads. Admins can decide. The default should be the current behavior to not call posix_fadvise(). Also the code path for SD/Dir und FD should be separate so enable this for the FD bfile.c code only (it seems currently it is the same code).

 

Regards,

Robert

 

Robert Heinzmann

[hidden email]

IT-Operations

Travian Games GmbH

 

Von: Robert Heinzmann [[hidden email][hidden email]]
Gesendet: Mittwoch, 7. Oktober 2015 11:16
An: [hidden email]
Betreff: [Bacula-users] Support for HAVE_POSIX_FADVISE on Bacula Client

 

Hello,

 

we are using bacula on a virtual plattform (> 600 clients) and investiate moderate swap usage issues / cache usage issues. It seems that during backup of mysql datafiles (LVM Snapshot), the “cached” memory usage of the server increases and thus bacula backup reads (only done one) are cached (… polluting the rest of the vfs cache ?)

 

From reading the source of src/findlib/bfile.c, bacula supports posix_fadvise for a long time now (2007), by those two code pices:

 

commit 4df486dd5b04cc16178efcb75eaf3dbd4be44b31

Author: Kern Sibbald <[hidden email]>

Date:   Thu May 24 19:58:07 2007 +0000

 

    kes  Add code to tell the OS that we no longer need a cached

         file that we were reading. In findlib/bfile.c.  Also,

         only cache files that we are reading.

    kes  Tweak to bsmtp to eliminate compiler warnings on Win32.

    kes  Implement script to automatically generate cats and dll .def

         files for Win32 dll.

    kes  Update README.mingw32 to include new .def file generation.

 

    git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4898 91ce42f0-d328-0410-95d8-f526ca767f89

 

bopen()

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_WILLNEED)

   if (bfd->fid != -1 && flags & O_RDONLY) {

      int stat = posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_WILLNEED);

      Dmsg2(400, "Did posix_fadvise on %s stat=%d\n", fname, stat);

   }

#endif

 

bclose()

 

#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_DONTNEED)

   if (bfd->m_flags & O_RDONLY) {

      fdatasync(bfd->fid);            /* sync the file */

      /* Tell OS we don't need it any more */

      posix_fadvise(bfd->fid, 0, 0, POSIX_FADV_DONTNEED);

   }

#endif

 

I checked that our bacula client was compiled with HAVE_POSIX_FADVISE (added debug messages).

 

I also added more debugging code and it seems that during a normal backup run, the code is never executed on bacula-fd as O_RDONLY is not set in the flags of the originating bopen request.

 

Is this a bug or a feature ?

 

How can I prevent cache usage of bacula-fd on the client side at all ? Is there a Flag in bacula-fd to force O_READONLY opening of backed up files ?

 

Regards,

Robert



------------------------------------------------------------------------------
Full-scale, agent-less Infrastructure Monitoring from a single dashboard
Integrate with 40+ ManageEngine ITSM Solutions for complete visibility
Physical-Virtual-Cloud Infrastructure monitoring from one console
Real user monitoring with APM Insights and performance trend reports 
Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140


_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users


------------------------------------------------------------------------------

_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Phil Stracchino-2
On 10/09/15 22:05, Kern Sibbald wrote:
> Hello,
>
> Thanks for pointing this out. I still have problems imagining that they
> would define O_RDONLY as 0 in a bitmapped variable!!!

It makes more sense if you think of it as the absence of O_RDWR.


--
  Phil Stracchino
  Babylon Communications
  [hidden email]
  [hidden email]
  Landline: 603.293.8485

------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Kern Sibbald
On 10/10/2015 10:16 AM, Phil Stracchino wrote:
> On 10/09/15 22:05, Kern Sibbald wrote:
>> Hello,
>>
>> Thanks for pointing this out. I still have problems imagining that they
>> would define O_RDONLY as 0 in a bitmapped variable!!!
> It makes more sense if you think of it as the absence of O_RDWR.

Unfortunately, it is even more complicated than that, because it is
actually the absence of O_RWRD and O_WRONLY.

In my opinion the original implementation was faulty because O_RDONLY
should really be defined as:

#define O_RDONLY !(O_RWRD&O_RONLY)

See my second to last commit ...

Goan, but that is how it is so now we know, and hopefully it should now
work correctly.

Best regards,
Kern

------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Richard Hall
On 10/10/2015 19:30, Kern Sibbald wrote:

> On 10/10/2015 10:16 AM, Phil Stracchino wrote:
>> On 10/09/15 22:05, Kern Sibbald wrote:
>>> Hello,
>>>
>>> Thanks for pointing this out. I still have problems imagining that they
>>> would define O_RDONLY as 0 in a bitmapped variable!!!
>> It makes more sense if you think of it as the absence of O_RDWR.
>
> Unfortunately, it is even more complicated than that, because it is
> actually the absence of O_RWRD and O_WRONLY.
>
> In my opinion the original implementation was faulty because O_RDONLY
> should really be defined as:
>
> #define O_RDONLY !(O_RWRD&O_RONLY)
>
> See my second to last commit ...
>
> Goan, but that is how it is so now we know, and hopefully it should now
> work correctly.
>
> Best regards,
> Kern

Aren't people over-complicating this? You need to think of this as a
2-bit integer field, not as a bitmap. Then looking at the constants
defined in fcntl.h, it becomes clear (to me, anyway!) that the code
should be (sorry, I forget the exact context, but you'll see what I
mean, I hope)

   (flags & O_ACCMODE) == O_RDONLY

(The Solaris box I am looking at defines

#define O_ACCMODE       3       /* Mask for file access modes */

while a nearby Linux system has

#define O_ACCMODE       00000003
#define O_RDONLY        00000000
#define O_WRONLY        00000001
#define O_RDWR          00000002

)

Regards,
  Richard

------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Kern Sibbald
Hello,

>From what I read on the web, this whole thing is a bit of a mess, and
apparently POSIX requires that O_ACCMODE not be 3 because there are at
least two bits besides O_RWRD and O_RDONLY.  However, on most systems
O_ACCMODE is 3.  It would be hard to consider any part of flags as an
integer because it is a bit field, and on Linux (and most other
systems), the value of the "integer"  produced by anding with O_ACMODE
cannot take on the value 3 (or more precisely, the value of 3 is illogical).

In any case, I have fixed the current Bacula code to work correctly with
all the modes that Bacula currently uses.

I thank Robert Heinzmann for finding and identifying the problem :-)

Best regards,
Kern



On 10/10/2015 02:44 PM, Richard Hall wrote:

> On 10/10/2015 19:30, Kern Sibbald wrote:
>> On 10/10/2015 10:16 AM, Phil Stracchino wrote:
>>> On 10/09/15 22:05, Kern Sibbald wrote:
>>>> Hello,
>>>>
>>>> Thanks for pointing this out. I still have problems imagining that they
>>>> would define O_RDONLY as 0 in a bitmapped variable!!!
>>> It makes more sense if you think of it as the absence of O_RDWR.
>> Unfortunately, it is even more complicated than that, because it is
>> actually the absence of O_RWRD and O_WRONLY.
>>
>> In my opinion the original implementation was faulty because O_RDONLY
>> should really be defined as:
>>
>> #define O_RDONLY !(O_RWRD&O_RONLY)
>>
>> See my second to last commit ...
>>
>> Goan, but that is how it is so now we know, and hopefully it should now
>> work correctly.
>>
>> Best regards,
>> Kern
> Aren't people over-complicating this? You need to think of this as a
> 2-bit integer field, not as a bitmap. Then looking at the constants
> defined in fcntl.h, it becomes clear (to me, anyway!) that the code
> should be (sorry, I forget the exact context, but you'll see what I
> mean, I hope)
>
>    (flags & O_ACCMODE) == O_RDONLY
>
> (The Solaris box I am looking at defines
>
> #define O_ACCMODE       3       /* Mask for file access modes */
>
> while a nearby Linux system has
>
> #define O_ACCMODE       00000003
> #define O_RDONLY        00000000
> #define O_WRONLY        00000001
> #define O_RDWR          00000002
>
> )
>
> Regards,
>   Richard
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Bacula-users mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/bacula-users
>
a


------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users
Reply | Threaded
Open this post in threaded view
|

Re: Support for HAVE_POSIX_FADVISE on Bacula Client

Josh Fisher


On 10/10/2015 10:15 PM, Kern Sibbald wrote:
> Hello,
>
> >From what I read on the web, this whole thing is a bit of a mess, and
> apparently POSIX requires that O_ACCMODE not be 3 because there are at
> least two bits besides O_RWRD and O_RDONLY.  However, on most systems
> O_ACCMODE is 3.  It would be hard to consider any part of flags as an
> integer because it is a bit field, and on Linux (and most other
> systems), the value of the "integer"  produced by anding with O_ACMODE
> cannot take on the value 3 (or more precisely, the value of 3 is illogical).

Well, according to the open(2) man page from RHEL 7, the two least
significant bits are NOT part of the bitfield:

         Unlike the other values that can be specified in flags, the
access mode values O_RDONLY, O_WRONLY, and O_RDWR,
         do not specify individual bits.  Rather, they define the low
order two bits of flags, and are defined  respectively  as 0, 1, and 2.

Granted, it is strange and probably a holdover from long ago when memory
was scarce and CPU register sizes were small.
.

>
> In any case, I have fixed the current Bacula code to work correctly with
> all the modes that Bacula currently uses.
>
> I thank Robert Heinzmann for finding and identifying the problem :-)
>
> Best regards,
> Kern
>
>
>
> On 10/10/2015 02:44 PM, Richard Hall wrote:
>> On 10/10/2015 19:30, Kern Sibbald wrote:
>>> On 10/10/2015 10:16 AM, Phil Stracchino wrote:
>>>> On 10/09/15 22:05, Kern Sibbald wrote:
>>>>> Hello,
>>>>>
>>>>> Thanks for pointing this out. I still have problems imagining that they
>>>>> would define O_RDONLY as 0 in a bitmapped variable!!!
>>>> It makes more sense if you think of it as the absence of O_RDWR.
>>> Unfortunately, it is even more complicated than that, because it is
>>> actually the absence of O_RWRD and O_WRONLY.
>>>
>>> In my opinion the original implementation was faulty because O_RDONLY
>>> should really be defined as:
>>>
>>> #define O_RDONLY !(O_RWRD&O_RONLY)
>>>
>>> See my second to last commit ...
>>>
>>> Goan, but that is how it is so now we know, and hopefully it should now
>>> work correctly.
>>>
>>> Best regards,
>>> Kern
>> Aren't people over-complicating this? You need to think of this as a
>> 2-bit integer field, not as a bitmap. Then looking at the constants
>> defined in fcntl.h, it becomes clear (to me, anyway!) that the code
>> should be (sorry, I forget the exact context, but you'll see what I
>> mean, I hope)
>>
>>     (flags & O_ACCMODE) == O_RDONLY
>>
>> (The Solaris box I am looking at defines
>>
>> #define O_ACCMODE       3       /* Mask for file access modes */
>>
>> while a nearby Linux system has
>>
>> #define O_ACCMODE       00000003
>> #define O_RDONLY        00000000
>> #define O_WRONLY        00000001
>> #define O_RDWR          00000002
>>
>> )
>>
>> Regards,
>>    Richard
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> Bacula-users mailing list
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/bacula-users
>>
> a
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Bacula-users mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/bacula-users


------------------------------------------------------------------------------
_______________________________________________
Bacula-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/bacula-users