Merge into trunk : handle-tar-creator-edge-cases : Code : Keeper

Status:	Superseded
Proposed branch:	lp://qastaging/~charlesk/keeper/handle-tar-creator-edge-cases
Merge into:	lp://qastaging/keeper
Diff against target:	7720 lines (+3998/-1884) 89 files modified CMakeLists.txt (+18/-3) debian/changelog (+1/-1) debian/control (+10/-0) debian/qml-keeper.install (+1/-0) debian/rules (+1/-0) include/client/client.h (+37/-9) include/helper/backup-helper.h (+11/-7) include/helper/helper.h (+21/-7) include/helper/metadata.h (+1/-1) src/cli/CMakeLists.txt (+3/-0) src/cli/main.cpp (+1/-2) src/client/CMakeLists.txt (+5/-14) src/client/client.cpp (+180/-28) src/client/qml-plugin/CMakeLists.txt (+42/-0) src/client/qml-plugin/plugin.cpp (+37/-0) src/client/qml-plugin/plugin.h (+33/-0) src/client/qml-plugin/qmldir (+2/-0) src/helper/CMakeLists.txt (+2/-0) src/helper/backup-helper.cpp (+144/-161) src/helper/helper.cpp (+199/-19) src/helper/metadata.cpp (+1/-1) src/service/CMakeLists.txt (+31/-6) src/service/app-const.h (+0/-3) src/service/backup-choices.cpp (+2/-2) src/service/backup-choices.h (+2/-2) src/service/keeper-task-backup.cpp (+105/-0) src/service/keeper-task-backup.h (+46/-0) src/service/keeper-task.cpp (+203/-0) src/service/keeper-task.h (+70/-0) src/service/keeper-user.cpp (+11/-4) src/service/keeper-user.h (+1/-1) src/service/keeper.cpp (+72/-331) src/service/keeper.h (+6/-3) src/service/metadata-provider.h (+2/-2) src/service/private/keeper-task_p.h (+56/-0) src/service/restore-choices.cpp (+8/-3) src/service/restore-choices.h (+2/-2) src/service/task-manager.cpp (+323/-0) src/service/task-manager.h (+67/-0) src/storage-framework/CMakeLists.txt (+3/-1) src/storage-framework/sf-uploader.cpp (+55/-0) src/storage-framework/sf-uploader.h (+45/-0) src/storage-framework/storage_framework_client.cpp (+114/-90) src/storage-framework/storage_framework_client.h (+26/-25) src/storage-framework/uploader.h (+44/-0) src/tar/tar-creator.cpp (+97/-40) src/tar/tar-creator.h (+1/-1) src/util/CMakeLists.txt (+28/-9) src/util/attributes.h (+26/-0) src/util/connection-helper.h (+120/-0) src/util/dbus-utils.cpp (+6/-1) src/util/unix-signal-handler.cpp (+4/-4) tests/CMakeLists.txt (+28/-1) tests/dbusmock/CMakeLists.txt (+12/-13) tests/fakes/CMakeLists.txt (+3/-0) tests/fakes/fake-backup-helper.cpp (+5/-4) tests/fakes/inactive_helper.sh (+22/-0) tests/fakes/upstart/CMakeLists.txt (+68/-0) tests/fakes/upstart/com.ubuntu.Upstart0_6.Instance.xml (+7/-0) tests/fakes/upstart/com.ubuntu.Upstart0_6.Job.xml (+19/-0) tests/fakes/upstart/com.ubuntu.Upstart0_6.xml (+18/-0) tests/fakes/upstart/main.cpp (+83/-0) tests/fakes/upstart/upstart-defs.h (+35/-0) tests/fakes/upstart/upstart-instance-mock.cpp (+37/-0) tests/fakes/upstart/upstart-instance-mock.h (+47/-0) tests/fakes/upstart/upstart-job-mock.cpp (+187/-0) tests/fakes/upstart/upstart-job-mock.h (+56/-0) tests/fakes/upstart/upstart-mock.cpp (+50/-0) tests/fakes/upstart/upstart-mock.h (+46/-0) tests/integration/CMakeLists.txt (+3/-1) tests/integration/helpers/CMakeLists.txt (+9/-5) tests/integration/helpers/helpers-test-failure.cpp (+34/-20) tests/integration/helpers/helpers-test.cc (+91/-228) tests/integration/helpers/mir-mock.cpp (+0/-124) tests/integration/helpers/mir-mock.h (+0/-31) tests/integration/helpers/test-helpers-base.cpp (+508/-431) tests/integration/helpers/test-helpers-base.h (+47/-66) tests/qdbus-stubs/CMakeLists.txt (+53/-0) tests/qdbus-stubs/org.freedesktop.DBus.Properties.xml (+27/-0) tests/unit/CMakeLists.txt (+3/-1) tests/unit/helper/CMakeLists.txt (+4/-4) tests/unit/helper/fake-helper.h (+1/-1) tests/unit/helper/speed-test.cpp (+9/-2) tests/unit/tar/keeper-tar-create-test.cpp (+0/-1) tests/unit/tar/tar-creator-libarchive-failure-test.cpp (+15/-13) tests/unit/tar/tar-creator-test.cpp (+1/-2) tests/utils/file-utils.cpp (+139/-137) tests/utils/file-utils.h (+3/-14) tests/utils/keeper-dbusmock-fixture.h (+2/-2)
To merge this branch:	bzr merge lp://qastaging/~charlesk/keeper/handle-tar-creator-edge-cases
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Xavi Garcia		2016-09-13	Pending
Review via email: mp+305670@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2016-09-13.

Commit message

Handle possible edge cases in our use of libarchive.

Description of the change

Handle possible edge cases in our use of libarchive.

1. Wrap the archive_write_header(), archive_write_data(), and archive_write_close() functions in helper functions that check libarchive's return values. All three follow a similar pattern: return on OK, try again on RETRY, gWarning() on WARN, and warn + throw an exception on FATAL.

2. Fix a subtle error in TarCreator::Impl::step() that caused step_buf_ to grow larger than intended by not breaking out of the outer loop after our read(BUFSIZE) call.

3. Fix a memory error in TarCreator::Impl::~Impl() where the step_archive_ smart pointer's deleter called archive_free(), which flushed libarchive's internal buffer to our step_buf_ via our append_bytes_write_cb() callback. This is all fine except for step_buf_ being declared after step_archive_ s.t. it was also destroyed before that final append_bytes_write_cb() call. Fixed by reordering the fields.

4. Disable libarchive's internal buffering by calling archive_write_set_bytes_per_block(0). libarchive's buffering is unnecessary because all our writes already route through step_buf_.

lp://qastaging/~charlesk/keeper/handle-tar-creator-edge-cases updated on 2016-09-15

114. By Charles Kerr on 2016-09-13: copyediting: consistent use of 'auto const', whitespace
115. By Charles Kerr on 2016-09-14: add extra logging messages, re-enable libarchive buffering
116. By Charles Kerr on 2016-09-14: in tar-creator, call archive_write_new() in a consistent way each time
117. By Charles Kerr on 2016-09-14: add slightly more test failure information to tar-creator-test.cpp
118. By Charles Kerr on 2016-09-14: in tar-creator's append_bytes_write_cb(), don't use references for storing the result of static_cast calls
119. By Charles Kerr on 2016-09-14: in tar-creator, remove unnecessary size_t cast
120. By Charles Kerr on 2016-09-14: in tar-creator, use QStringLiteral where possible
121. By Charles Kerr on 2016-09-14: in tar-creator-test, split the compressed and uncompressed tests out into separate pieces
122. By Charles Kerr on 2016-09-15: since the tar-creator-test failure is only happening in compress, try reimplementing the read loop in calculate_compressed_size()
123. By Charles Kerr on 2016-09-15: add some new sanity tests to tar-creator-test's compression tsts
124. By Charles Kerr on 2016-09-15: change how compression is invoked

Unmerged revisions

124. By Charles Kerr on 2016-09-15: change how compression is invoked
123. By Charles Kerr on 2016-09-15: add some new sanity tests to tar-creator-test's compression tsts
122. By Charles Kerr on 2016-09-15: since the tar-creator-test failure is only happening in compress, try reimplementing the read loop in calculate_compressed_size()
121. By Charles Kerr on 2016-09-14: in tar-creator-test, split the compressed and uncompressed tests out into separate pieces
120. By Charles Kerr on 2016-09-14: in tar-creator, use QStringLiteral where possible
119. By Charles Kerr on 2016-09-14: in tar-creator, remove unnecessary size_t cast
118. By Charles Kerr on 2016-09-14: in tar-creator's append_bytes_write_cb(), don't use references for storing the result of static_cast calls
117. By Charles Kerr on 2016-09-14: add slightly more test failure information to tar-creator-test.cpp
116. By Charles Kerr on 2016-09-14: in tar-creator, call archive_write_new() in a consistent way each time
115. By Charles Kerr on 2016-09-14: add extra logging messages, re-enable libarchive buffering

Keeper

Merge lp://qastaging/~charlesk/keeper/handle-tar-creator-edge-cases into lp://qastaging/keeper

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers