Skip to content

Improve glusterfs backend#455

Merged
arogge merged 3 commits intomasterfrom
dev/arogge/master/TT4200617
Apr 8, 2020
Merged

Improve glusterfs backend#455
arogge merged 3 commits intomasterfrom
dev/arogge/master/TT4200617

Conversation

@arogge
Copy link
Member

@arogge arogge commented Mar 24, 2020

  • add logging options to glusterfs backend
  • add a systemtest to test the glusterfs backend

arogge added 2 commits March 19, 2020 21:04
This patch adds a copy of backup-bareos-test that will write to and read
from a glusterfs filesystem.
This patch adds two new device options logfile and loglevel to the gfapi
backend. These can be used to set a logfile and loglevel for gfapi. By
configuring this you can make the glusterfs api write a logfile.
@arogge arogge requested review from franku and pstorz March 24, 2020 14:53
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the one question this PR looks good.

Well done!

done = true;
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this block intentionally removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look closely, you'll find it in the switch()'s default-case

Copy link
Contributor

@franku franku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gfapi_logfile_ should be set to zero, one not handled error could bei silently overseen.
Otherwise very good.

gfapi_logfile_ = bp + device_options[i].compare_size;
break;
case argument_loglevel:
gfapi_loglevel_ =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of error gfapi_loglevel_ will be zero.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is expected and not a problem.

private:
char* gfapi_configstring_;
char* gfapi_uri_;
char* gfapi_logfile_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
char* gfapi_logfile_;
char* gfapi_logfile_{};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should double check if the constructor is really called. Ohterwise this has no effect.

@arogge arogge merged commit acfa5c9 into master Apr 8, 2020
@arogge arogge deleted the dev/arogge/master/TT4200617 branch April 14, 2020 17:28
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

Successfully merging this pull request may close these issues.

3 participants