Merge lp://staging/~gl-az/percona-xtrabackup/bug1273196-2.1 into lp://staging/percona-xtrabackup/2.1

Proposed by George Ormond Lorch III
Status: Merged
Approved by: Sergei Glushchenko
Approved revision: no longer in the source branch.
Merged at revision: 731
Proposed branch: lp://staging/~gl-az/percona-xtrabackup/bug1273196-2.1
Merge into: lp://staging/percona-xtrabackup/2.1
Diff against target: 121 lines (+52/-29)
1 file modified
src/xbcrypt_read.c (+52/-29)
To merge this branch: bzr merge lp://staging/~gl-az/percona-xtrabackup/bug1273196-2.1
Reviewer Review Type Date Requested Status
Sergei Glushchenko (community) g2 Approve
Review via email: mp+206541@code.staging.launchpad.net

Description of the change

bug 1273196 - Streaming issue with xbcrypt

xbcrypt read failures are due to partial reads happening on a stdin file because reads are not using either MY_FULL_IO nor looping and rereading on a partial (> 0) read.

To fix, simple re-read loops are added for each read point to progressively continue to read into the buffer as long as something was read.

jenkins http://jenkins.percona.com/view/PXB%202.1/job/percona-xtrabackup-2.1-param/542/

To post a comment you must log in.
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Hi George,

Looks like crypt->read use my_read to actually read data. Since MY_FULL_IO is not passed to my_read, it can return MY_FILE_ERROR in case of error (which is a big number). I think F_READ doesn't handle this case properly.

review: Needs Fixing (g2)
Revision history for this message
George Ormond Lorch III (gl-az) wrote :

Good catch Sergei, thanks! A simple fix and new jenkins run: http://jenkins.percona.com/job/percona-xtrabackup-2.1-param/547/

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Approve

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

The fix looks correct, but I don't understand the purpose of another wrapper. Why not embed all F_READ() logic into my_xb_crypt_read_callback()?

Revision history for this message
George Ormond Lorch III (gl-az) wrote :

> The fix looks correct, but I don't understand the purpose of another wrapper.
> Why not embed all F_READ() logic into my_xb_crypt_read_callback()?

That is a very good question. Why don't I? I'll get that changed up, jenkinsed and all real quick here.

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

George,

I took care of that in my fix for bug #1273207, because I either had to duplicate your fix, or generalize it.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches