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

rbd: use concurrent writes for imports #13782

Merged
merged 3 commits into from Mar 8, 2017

Conversation

Projects
None yet
2 participants
@vshankar
Contributor

vshankar commented Mar 3, 2017

No description provided.

@dillaman dillaman changed the title from concurrent image IO during import/import-diff (v2) to rbd: use concurrent writes for imports Mar 7, 2017

@dillaman

This comment has been minimized.

Contributor

dillaman commented Mar 7, 2017

@vshankar nit: in the commit messages, just use "rbd: " as the prefix

@dillaman

lgtm -- would like to clean up the hard-coded byte counts while we are here

size_t size;
utils::ProgressContext pc;
OrderedThrottle throttle;
uint64_t last_offset;

This comment has been minimized.

@dillaman

dillaman Mar 7, 2017

Contributor

Initialize to zero

static int do_image_resize(ImportDiffContext *idiffctx)
{
int r;
char buf[8];

This comment has been minimized.

@dillaman

dillaman Mar 7, 2017

Contributor

Nit: while we are cleaning up -- can we switch to "sizeof(uint64_t)" instead of the magic '8' value (here and below)

if (fd == STDIN_FILENO) {
// read the appending data out to skip this tag.
char buf[4096];
uint64_t len = min(length, uint64_t(4096));

This comment has been minimized.

@dillaman

dillaman Mar 7, 2017

Contributor

Nit: for safety -- sizeof(buf) here and below

int r;
__u8 read_tag;
r = safe_read_exact(fd, &read_tag, 1);

This comment has been minimized.

@dillaman

dillaman Mar 7, 2017

Contributor

Nit: sizeof(read_tag)

{
int r;
char buf[16];
r = safe_read_exact(idiffctx->fd, buf, 16);

This comment has been minimized.

@dillaman

dillaman Mar 7, 2017

Contributor

Nit: sizeof(buf) vs 16

@vshankar

This comment has been minimized.

Contributor

vshankar commented Mar 7, 2017

lgtm -- would like to clean up the hard-coded byte counts while we are here

Done with the updated PR.

@dillaman

This comment has been minimized.

Contributor

dillaman commented Mar 7, 2017

retest this please

vshankar added some commits Mar 1, 2017

rbd: cleanup unused throttle in v2 import
v2 import does not use throttle as of now although v1
import does use it - initialize throttle wherever its
necessary and avoid passing it functions that do not
require it.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
rbd: concurrent v2 image IO during import/import-diff
Fixes: http://tracker.ceph.com/issues/19034
Signed-off-by: Venky Shankar <vshankar@redhat.com>
rbd: refactor header import
and resuse validate_banner() helper routine where
ever necessary.

Signed-off-by: Venky Shankar <vshankar@redhat.com>

@dillaman dillaman merged commit 8349287 into ceph:master Mar 8, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment