Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: When run through BackupPC_dump, 3.2.3 dumps file contents in log #95

Closed
haarp opened this issue Sep 6, 2020 · 11 comments
Closed

Comments

@haarp
Copy link

haarp commented Sep 6, 2020

Hello,

592059c has introduced a bug in combination with BackupPC 3. Dumps work fine until a vanished file is encountered, at which point file data is dumped directly into the log, breaking the backup process.

For reference, in my use case, BackupPC calls rsync on the client device via ssh and this cmdline: /usr/bin/rsync --server --sender --numeric-ids --perms --owner --group -D --links --hard-links --times --block-size=2048 --recursive --checksum-seed=32761 --ignore-times . /

I don't understand rsync quite well enough to tell if it's a BackupPC or rsync issue. However, due to the vast number of BackupPC installs out there that will not receive timely (or any) updates, but still have to backup clients which will receive rsync updates, I'd say that not having such a breaking change in rsync is preferable.

On top of this, it's very difficult to figure out why this breaks BackupPC. It's possible it also breaks other users of rsync. And while --no-msgs2stderr can be used to avoid this bug, older rsyncs won't recognize it and abort, prohibiting its widespread use.

Also see backuppc/backuppc#369

Thanks!

@craigbarratt
Copy link

I'm the BackupPC developer. BackupPC 3.x is very old, and it's quite possible it's broken with the latest rsync. BackupPC 4.x uses a completely different integration with rsync, and it much more up-to-date with recent and current rsync versions.

Let's work on this issue using the filed BackupPC issue, and we can close or update this issue based on what we find.

@realtalk69
Copy link

realtalk69 commented Sep 8, 2020 via email

@RsyncProject RsyncProject deleted a comment from realtalk69 Sep 8, 2020
@craigbarratt
Copy link

After checking into this, this change does indeed break BackupPC 3.x. We confirmed that specifying --no-msgs2stderr solves the issue. Unfortunately that option will fail on an older rsync, and any fix to BackupPC 3.x will take a while to propagate to users, if at all (since packaging support for BackupPC 3.x has atrophied). BackupPC 3.x still seems in pretty wide use, even though 4.x has been out for a few years.

BackupPC 3.x only uses rsync protocol 28. Would it be possible to make --no-msgs2stderr the default if the protocol is <= 28? That seems in the spirit of maintaining backward compatibility, and that's so old that it shouldn't affect users who are already using the new options. If you are ok with that, I'm happy to test that approach to make sure it really fixes the issue.

@WayneD
Copy link
Member

WayneD commented Sep 13, 2020

Nobody has specified what the actual rsync issue is. Completely remove all mention of BackupPC and specify what error behavior is happening. Is a v3.2.3 rsync being run as a remote server and the local process doesn't have a stderr open for the remote rsync to be using?

@craigbarratt
Copy link

Yes, rsync <-> rsync works fine. This is due to a long-standing bug in the perl module File::RsyncP that mishandles stderr from rsync, which was exposed by this change in rsync 3.2.3. (BackupPC 3.x via File::RsyncP implements most of rsync protocol 28 and talks directly to rsync.)

I just released a new version 0.76 of File::RsyncP on metacpan that fixes this bug. Any users with this problem on BackupPC 3.x can fix it by upgrading to this new version. See the BackupPC issue 339.

Please go ahead and close this issue.

@haarp
Copy link
Author

haarp commented Sep 26, 2020

IMO, rsync should not enable msgs2stderr by default, at least for rsync protocol 28. Even tho this is not an rsync bug, it will affect users of older File::RsyncP that likely will never see updates to that module again.

It's your call tho, @WayneD

@WayneD
Copy link
Member

WayneD commented Sep 29, 2020

I'm a bit torn on this one. I've currently got a potential change in the rsync-patches repo (stderr-compat.diff) that you could try out if you like.

@craigbarratt
Copy link

I tried the patch and confirmed it works fine with both the old (broken) File::RsyncP 0.74 and the new fixed one (0.76).

It would be great to include this in the next rsync version, since File::RsyncP is so old it's not clear whether or how quickly the fixed version will get packaged and be available in the various distros. But that's also a reason not to contaminate rsync with this legacy bug unrelated to rsync.

It is completely up to you - I'm fine with telling BackupPC 3.x users that encounter this issue to either upgrade to BackupPC 4.x (which does not use File::RsyncP) or upgrade File::RsyncP to 0.76.

Thanks for all you do leading rsync development!

@eLvErDe
Copy link

eLvErDe commented Apr 11, 2021

Hello,

I know nobody is supposed to install unstrusted RPMs from Internet, but in case someone trust me and want to do this anyway, here is a zip (because GitHub) containing RHEL7/CentOS 7 SRPM/RPMS of the fixed perl-File-RsyncP package (for the ones having CentOS 7 BackupPC servers):

perl-File-RsyncP-0.76-1.el7.zip

Just deployed on tested on my servers, confirmed fixing the issue !

Regards, Adam.

@jonglezb
Copy link

FYI, this was reported on Debian against rsync: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=969463

But from what I understand, this is actually a bug in libfile-rsyncp-perl, which is no longer present in Debian testing and stable, so I think it has very little chance of seeing an update: https://packages.debian.org/search?searchon=sourcenames&keywords=libfile-rsyncp-perl

@WayneD
Copy link
Member

WayneD commented Jan 2, 2022

I've gone ahead an merged in the fix that I worked up in rsync-patches.

@WayneD WayneD closed this as completed Jan 2, 2022
WayneD added a commit that referenced this issue Jan 2, 2022
This makes the default for a protocol-28 server process be --stderr=client
instead of --stderr=errors.  See rsync's github issue #95.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants