Merge lp:~igor-tverdovskiy/percona-xtrabackup/bug_935847 into lp:percona-xtrabackup/2.0

Proposed by Igor Tverdovskiy
Status: Merged
Approved by: Stewart Smith
Approved revision: no longer in the source branch.
Merged at revision: 429
Proposed branch: lp:~igor-tverdovskiy/percona-xtrabackup/bug_935847
Merge into: lp:percona-xtrabackup/2.0
Diff against target: 12 lines (+1/-1)
1 file modified
innobackupex (+1/-1)
To merge this branch: bzr merge lp:~igor-tverdovskiy/percona-xtrabackup/bug_935847
Reviewer Review Type Date Requested Status
Stewart Smith (community) Approve
Valentine Gostev (community) Needs Fixing
Alexey Kopytov Pending
Review via email: mp+93708@code.launchpad.net

Description of the change

To post a comment you must log in.
Revision history for this message
Valentine Gostev (longbow) wrote :

Rejecting. Please see comment in bug 935847

review: Needs Fixing
Revision history for this message
Stewart Smith (stewart) wrote :

i'll echo Valentine's response on the bug below, but we basically see this as not a bug. Thanks for taking the time and effort to report it and provide a patch.

(from the bug)
It is not very good practice to keep my.cnf and master.info in datadir, much safer to keep it at external location, that's why explicit check for empty directory was added in https://bugs.launchpad.net/percona-xtrabackup/+bug/737569

Anyway, if you need such exceptions current approach is not reasonable, I would recommend to add an option, which could accept a list of files to ignore.

review: Disapprove
Revision history for this message
Alexey Kopytov (akopytov) wrote :

As I wrote in the bug report "datadir is the default location for master.info. I don't see any problems with adding those exceptions, as XtraBackup does not backup those files, so it won't overwrite anything."

So it is a valid bug report and a fix.

Revision history for this message
Stewart Smith (stewart) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'innobackupex'
2--- innobackupex 2012-02-14 04:00:10 +0000
3+++ innobackupex 2012-02-18 23:41:20 +0000
4@@ -564,7 +564,7 @@
5 die "$is_directory_empty_comment directory does not exist!";
6 }
7 opendir (my $dh, $empty_dir) or die "Not a directory";
8- if ( ! scalar( grep { $_ ne "." && $_ ne ".." } readdir($dh)) == 0) {
9+ if ( ! scalar( grep { $_ ne "." && $_ ne ".." && $_ ne "my.cnf" && $_ ne "master.info"} readdir($dh)) == 0) {
10 die "$is_directory_empty_comment directory is not empty!";
11 }
12 closedir($dh);

Subscribers

People subscribed via source and target branches