cancel virtual backups if none of the jobids contains files#1070
cancel virtual backups if none of the jobids contains files#1070
Conversation
36414eb to
9c9b4b6
Compare
arogge
left a comment
There was a problem hiding this comment.
I think it is OK to consolidate a job with no files. The problem is consolidating a job with files that were purged.
So we should fail if one or more of the jobs had its files purged.
core/src/dird/vbackup.cc
Outdated
|
|
||
| // Find oldest Jobid, get the db record and find its level | ||
| p = strchr(jobids, ','); /* find oldest jobid */ | ||
| p = strchr(const_cast<char*>(jobids.data()), ','); /* find oldest jobid */ |
There was a problem hiding this comment.
data() does not guarantee a null-terminated string. You'll need c_str() for that.
The const_cast<> is also not needed, as strchr is defined to be char* strchr(const char *s, int c); so it should happily accept the const char* that c_str() will return.
| p = strchr(const_cast<char*>(jobids.data()), ','); /* find oldest jobid */ | |
| p = strchr(jobids.c_str(), ','); /* find oldest jobid */ |
There was a problem hiding this comment.
at second thought: all of this pointer trickery is used, because we get a comma-separated list of jobids in a string and we need the first and the last of these.
I propose to have a function vector<std::string> split_string(const std::string& str, const std:string& delim) that splits str at delim and puts the pieces into a vector it returns.
We can then simply do:
auto jobid_list = split_string(jobids, ',');
jcr->impl->previous_jr = JobDbRecord{};
jcr->impl->previous_jr.JobId = str_to_int64(jobid_list.front.c_str());
and further below
jcr->impl->previous_jr = JobDbRecord{};
jcr->impl->previous_jr.JobId = str_to_int64(jobid_list.back.c_str());
9c9b4b6 to
c363cad
Compare
|
I updated the PR branch with my personal version of choice. We now detect if one of the jobs that should be consolidated had its files pruned or was removed from the catalog (by means of |
c1f4c16 to
92a25ed
Compare
92a25ed to
b5f5ade
Compare
95abc2e to
8868d7b
Compare
Run three virtual backups that are supposed to fail: 1. no files in any of the jobs 2. one jobid that doesn't exist in the catalog anymore 3. one job that had its files pruned
You can now construct a PoolMem from a std::string if needed. We also mark the single-argument constructors explicit, so you can't accidentially convert an integer oder char* into a PoolMem.
add a templated function so that you can use an object or a lambda as your query handler.
Add a function to split std::string into std::vector<std::string>.
This patch adds a check that makes sure that the input jobids for the virtual job are valid (i.e. exist in the database) and did't have their files pruned.
8868d7b to
27ad8df
Compare
Virtual backups consolidating jobids that contain no files failed when creating the bootstrap file.
This PR now checks for the number of files before creating the bootstrap file and cancels the consolidation if no files exist.
Please check
If you have any questions or problems, please give a comment in the PR.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
General
Source code quality
bareos-check-sources --since-mergedoes not report any problemsgit statusshould not report modifications in the source tree after building and testingTests