Skip to content

Commit

Permalink
Copy job definition requires unnecessary settings
Browse files Browse the repository at this point in the history 
When configuring a Migration or Copy Job you need to specify a Client
and Fileset although those items are never used in the Job as it selects
via a certain selection criterium the Jobs it needs to Migrate or Copy
and uses the Client and Fileset of that original Job.

We relaxed the configuration engine to no longer make the Client and
Fileset keyword required keywords. In the check_resources() function
we now check after the required keyword check what type of Job is
configured and when its not a Migration or Copy Job we still fail the
configuration check as for any Job not being a Migration or Copy Job we
still need the Client and Fileset.

We also needed to check in a lot of places if we have a Client or
Fileset for the current running Job as a lot of code assumes that there
always is a Client or Fileset. This is of course a bad assumption and
hopefully we found all places where these wrong assumptions are made
and the Jobs Client or Fileset pointer is dereferenced without a check.

This new code passed a full regression test both with the old configs
for Migrate and Copy with a dummy Client and Fileset and without.

Fixes #220: Copy job definition requires unnecessary settings
  • Loading branch information
Marco van Wieringen committed Feb 17, 2015
1 parent 8085e55 commit a744832
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 149 deletions.
4 changes: 2 additions & 2 deletions src/dird/backup.c
View file
Expand Up @@ -324,12 +324,12 @@ bool do_native_backup(JCR *jcr)
return false;
}

if (quota_check_hardquotas(jcr)) {
if (check_hardquotas(jcr)) {
Jmsg(jcr, M_FATAL, 0, _("Quota Exceeded. Job terminated.\n"));
return false;
}

if (quota_check_softquotas(jcr)) {
if (check_softquotas(jcr)) {
Dmsg0(10, "Quota exceeded\n");
Jmsg(jcr, M_FATAL, 0, _("Soft Quota Exceeded / Grace Time expired. Job terminated.\n"));
return false;
Expand Down
40 changes: 33 additions & 7 deletions src/dird/dird.c
View file
Expand Up @@ -693,8 +693,6 @@ static bool check_resources()
}

foreach_res(job, R_JOB) {
int i;

if (job->jobdefs) {
/*
* Handle Storage alists specifically
Expand Down Expand Up @@ -727,7 +725,7 @@ static bool check_resources()
/*
* Transfer default items from JobDefs Resource
*/
for (i=0; job_items[i].name; i++) {
for (int i = 0; job_items[i].name; i++) {
char **def_svalue, **svalue; /* string value */
uint32_t *def_ivalue, *ivalue; /* integer value */
bool *def_bvalue, *bvalue; /* bool value */
Expand All @@ -742,7 +740,7 @@ static bool check_resources()
if (!bit_is_set(i, job->hdr.item_present) &&
bit_is_set(i, job->jobdefs->hdr.item_present)) {
Dmsg2(400, "Job \"%s\", field \"%s\": getting default.\n",
job->name(), job_items[i].name);
job->name(), job_items[i].name);
offset = (char *)(job_items[i].value) - (char *)&res_all;
if (job_items[i].handler == store_str ||
job_items[i].handler == store_dir) {
Expand Down Expand Up @@ -828,11 +826,12 @@ static bool check_resources()
/*
* Ensure that all required items are present
*/
for (i=0; job_items[i].name; i++) {
for (int i = 0; job_items[i].name; i++) {
if (job_items[i].flags & ITEM_REQUIRED) {
if (!bit_is_set(i, job->hdr.item_present)) {
Jmsg(NULL, M_ERROR_TERM, 0, _("\"%s\" directive in Job \"%s\" resource is required, but not found.\n"),
job_items[i].name, job->name());
Jmsg(NULL, M_ERROR_TERM, 0,
_("\"%s\" directive in Job \"%s\" resource is required, but not found.\n"),
job_items[i].name, job->name());
OK = false;
}
}
Expand All @@ -845,6 +844,33 @@ static bool check_resources()
}
}

/*
* For Copy and Migrate we can have Jobs without a client or fileset.
*/
switch (job->JobType) {
case JT_COPY:
case JT_MIGRATE:
break;
default:
/*
* All others must have a client and fileset.
*/
if (!job->client) {
Jmsg(NULL, M_ERROR_TERM, 0,
_("\"client\" directive in Job \"%s\" resource is required, but not found.\n"),
job->name());
OK = false;
}

if (!job->fileset) {
Jmsg(NULL, M_ERROR_TERM, 0,
_("\"fileset\" directive in Job \"%s\" resource is required, but not found.\n"),
job->name());
OK = false;
}
break;
}

if (!job->storage && !job->pool->storage) {
Jmsg(NULL, M_FATAL, 0, _("No storage specified in Job \"%s\" nor in Pool.\n"),
job->name());
Expand Down
4 changes: 2 additions & 2 deletions src/dird/dird_conf.c
View file
Expand Up @@ -319,8 +319,8 @@ RES_ITEM job_items[] = {
{ "incrementalbackuppool", store_res, ITEM(res_job.inc_pool), R_POOL, 0, NULL },
{ "differentialbackuppool", store_res, ITEM(res_job.diff_pool), R_POOL, 0, NULL },
{ "nextpool", store_res, ITEM(res_job.next_pool), R_POOL, 0, NULL },
{ "client", store_res, ITEM(res_job.client), R_CLIENT, ITEM_REQUIRED, NULL },
{ "fileset", store_res, ITEM(res_job.fileset), R_FILESET, ITEM_REQUIRED, NULL },
{ "client", store_res, ITEM(res_job.client), R_CLIENT, 0, NULL },
{ "fileset", store_res, ITEM(res_job.fileset), R_FILESET, 0, NULL },
{ "schedule", store_res, ITEM(res_job.schedule), R_SCHEDULE, 0, NULL },
{ "verifyjob", store_res, ITEM(res_job.verify_job), R_JOB, 0, NULL },
{ "jobtoverify", store_res, ITEM(res_job.verify_job), R_JOB, 0, NULL },
Expand Down
17 changes: 12 additions & 5 deletions src/dird/job.c
View file
Expand Up @@ -177,8 +177,11 @@ bool setup_job(JCR *jcr, bool suppress_output)
* Create Job record
*/
init_jcr_job_record(jcr);
if (!get_or_create_client_record(jcr)) {
goto bail_out;

if (jcr->res.client) {
if (!get_or_create_client_record(jcr)) {
goto bail_out;
}
}

if (!db_create_job_record(jcr, jcr->db, &jcr->jr)) {
Expand Down Expand Up @@ -1463,10 +1466,14 @@ void set_jcr_defaults(JCR *jcr, JOBRES *job)
copy_rwstorage(jcr, job->pool->storage, _("Pool resource"));
}
jcr->res.client = job->client;
if (!jcr->client_name) {
jcr->client_name = get_pool_memory(PM_NAME);

if (jcr->res.client) {
if (!jcr->client_name) {
jcr->client_name = get_pool_memory(PM_NAME);
}
pm_strcpy(jcr->client_name, jcr->res.client->hdr.name);
}
pm_strcpy(jcr->client_name, jcr->res.client->hdr.name);

pm_strcpy(jcr->res.pool_source, _("Job resource"));
jcr->res.pool = job->pool;
jcr->res.full_pool = job->full_pool;
Expand Down
24 changes: 15 additions & 9 deletions src/dird/jobq.c
View file
Expand Up @@ -462,7 +462,9 @@ void *jobq_server(void *arg)
if (jcr->acquired_resource_locks) {
dec_read_store(jcr);
dec_write_store(jcr);
jcr->res.client->NumConcurrentJobs--;
if (jcr->res.client) {
jcr->res.client->NumConcurrentJobs--;
}
jcr->res.job->NumConcurrentJobs--;
jcr->acquired_resource_locks = false;
}
Expand Down Expand Up @@ -768,20 +770,24 @@ static bool acquire_resources(JCR *jcr)
skip_this_jcr = true;
}
}

if (skip_this_jcr) {
jcr->setJobStatus(JS_WaitStoreRes);
return false;
}

if (jcr->res.client->NumConcurrentJobs < jcr->res.client->MaxConcurrentJobs) {
jcr->res.client->NumConcurrentJobs++;
} else {
/* Back out previous locks */
dec_write_store(jcr);
dec_read_store(jcr);
jcr->setJobStatus(JS_WaitClientRes);
return false;
if (jcr->res.client) {
if (jcr->res.client->NumConcurrentJobs < jcr->res.client->MaxConcurrentJobs) {
jcr->res.client->NumConcurrentJobs++;
} else {
/* Back out previous locks */
dec_write_store(jcr);
dec_read_store(jcr);
jcr->setJobStatus(JS_WaitClientRes);
return false;
}
}

if (jcr->res.job->NumConcurrentJobs < jcr->res.job->MaxConcurrentJobs) {
jcr->res.job->NumConcurrentJobs++;
} else {
Expand Down
78 changes: 35 additions & 43 deletions src/dird/migrate.c
View file
Expand Up @@ -127,29 +127,25 @@ bool do_migration_init(JCR *jcr)
Jmsg(jcr, M_FATAL, 0, _("Could not get or create a Pool record.\n"));
return false;
}

/*
* Note, at this point, pool is the pool for this job. We
* transfer it to rpool (read pool), and a bit later,
* Note, at this point, pool is the pool for this job.
* We transfer it to rpool (read pool), and a bit later,
* pool will be changed to point to the write pool,
* which comes from pool->NextPool.
*/
jcr->res.rpool = jcr->res.pool; /* save read pool */
pm_strcpy(jcr->res.rpool_source, jcr->res.pool_source);
Dmsg2(dbglevel, "Read pool=%s (From %s)\n", jcr->res.rpool->name(), jcr->res.rpool_source);

if (!get_or_create_fileset_record(jcr)) {
Dmsg1(dbglevel, "JobId=%d no FileSet\n", (int)jcr->JobId);
Jmsg(jcr, M_FATAL, 0, _("Could not get or create the FileSet record.\n"));
return false;
}

/*
* If we find a job or jobs to migrate it is previous_jr.JobId
*/
count = getJob_to_migrate(jcr);
if (count < 0) {
return false;
}

if (count == 0) {
set_migration_next_pool(jcr, &pool);
return true; /* no work */
Expand Down Expand Up @@ -190,10 +186,12 @@ bool do_migration_init(JCR *jcr)
job = (JOBRES *)GetResWithName(R_JOB, jcr->jr.Name);
prev_job = (JOBRES *)GetResWithName(R_JOB, jcr->previous_jr.Name);
UnlockRes();

if (!job) {
Jmsg(jcr, M_FATAL, 0, _("Job resource not found for \"%s\".\n"), jcr->jr.Name);
return false;
}

if (!prev_job) {
Jmsg(jcr, M_FATAL, 0, _("Previous Job resource not found for \"%s\".\n"),
jcr->previous_jr.Name);
Expand Down Expand Up @@ -252,8 +250,8 @@ bool do_migration_init(JCR *jcr)
mig_jcr->res.job->IgnoreDuplicateJobChecking = true;

Dmsg4(dbglevel, "mig_jcr: Name=%s JobId=%d Type=%c Level=%c\n",
mig_jcr->jr.Name, (int)mig_jcr->jr.JobId,
mig_jcr->jr.JobType, mig_jcr->jr.JobLevel);
mig_jcr->jr.Name, (int)mig_jcr->jr.JobId,
mig_jcr->jr.JobType, mig_jcr->jr.JobLevel);

if (set_migration_next_pool(jcr, &pool)) {
/*
Expand Down Expand Up @@ -301,7 +299,7 @@ static bool set_migration_next_pool(JCR *jcr, POOLRES **retpool)
pr.PoolId = jcr->jr.PoolId;
if (!db_get_pool_record(jcr, jcr->db, &pr)) {
Jmsg(jcr, M_FATAL, 0, _("Pool for JobId %s not in database. ERR=%s\n"),
edit_int64(pr.PoolId, ed1), db_strerror(jcr->db));
edit_int64(pr.PoolId, ed1), db_strerror(jcr->db));
return false;
}

Expand Down Expand Up @@ -1645,45 +1643,39 @@ void migration_cleanup(JCR *jcr, int TermCode)
jobstatus_to_ascii(jcr->SDJobStatus, sd_term_msg, sizeof(sd_term_msg));

Jmsg(jcr, msg_type, 0, _("%s %s %s (%s):\n"
" Build OS: %s %s %s\n"
" Prev Backup JobId: %s\n"
" Prev Backup Job: %s\n"
" New Backup JobId: %s\n"
" Current JobId: %s\n"
" Current Job: %s\n"
" Backup Level: %s%s\n"
" Client: %s\n"
" FileSet: \"%s\" %s\n"
" Read Pool: \"%s\" (From %s)\n"
" Read Storage: \"%s\" (From %s)\n"
" Write Pool: \"%s\" (From %s)\n"
" Write Storage: \"%s\" (From %s)\n"
" Next Pool: \"%s\" (From %s)\n"
" Catalog: \"%s\" (From %s)\n"
" Start time: %s\n"
" End time: %s\n"
" Elapsed time: %s\n"
" Priority: %d\n"
" SD Files Written: %s\n"
" SD Bytes Written: %s (%sB)\n"
" Rate: %.1f KB/s\n"
" Volume name(s): %s\n"
" Volume Session Id: %d\n"
" Volume Session Time: %d\n"
" Last Volume Bytes: %s (%sB)\n"
" SD Errors: %d\n"
" SD termination status: %s\n"
" Termination: %s\n\n"),
" Build OS: %s %s %s\n"
" Prev Backup JobId: %s\n"
" Prev Backup Job: %s\n"
" New Backup JobId: %s\n"
" Current JobId: %s\n"
" Current Job: %s\n"
" Read Pool: \"%s\" (From %s)\n"
" Read Storage: \"%s\" (From %s)\n"
" Write Pool: \"%s\" (From %s)\n"
" Write Storage: \"%s\" (From %s)\n"
" Next Pool: \"%s\" (From %s)\n"
" Catalog: \"%s\" (From %s)\n"
" Start time: %s\n"
" End time: %s\n"
" Elapsed time: %s\n"
" Priority: %d\n"
" SD Files Written: %s\n"
" SD Bytes Written: %s (%sB)\n"
" Rate: %.1f KB/s\n"
" Volume name(s): %s\n"
" Volume Session Id: %d\n"
" Volume Session Time: %d\n"
" Last Volume Bytes: %s (%sB)\n"
" SD Errors: %d\n"
" SD termination status: %s\n"
" Termination: %s\n\n"),
BAREOS, my_name, VERSION, LSMDATE,
HOST_OS, DISTNAME, DISTVER,
edit_uint64(jcr->previous_jr.JobId, ec6),
jcr->previous_jr.Job,
mig_jcr ? edit_uint64(mig_jcr->jr.JobId, ec7) : "0",
edit_uint64(jcr->jr.JobId, ec8),
jcr->jr.Job,
level_to_str(jcr->getJobLevel()), jcr->since,
jcr->res.client->name(),
jcr->res.fileset->name(), jcr->FSCreateTime,
jcr->res.rpool->name(), jcr->res.rpool_source,
jcr->res.rstore ? jcr->res.rstore->name() : _("*None*"),
NPRT(jcr->res.rstore_source),
Expand Down
27 changes: 22 additions & 5 deletions src/dird/msgchan.c
View file
Expand Up @@ -143,6 +143,7 @@ bool start_storage_daemon_job(JCR *jcr, alist *rstore, alist *wstore, bool send_
bool ok = true;
STORERES *storage;
char auth_key[100];
const char *fileset_md5;
POOL_MEM store_name, device_name, pool_name, pool_type, media_type, backup_format;
POOL_MEM job_name, client_name, fileset_name;
int copy = 0;
Expand All @@ -156,15 +157,31 @@ bool start_storage_daemon_job(JCR *jcr, alist *rstore, alist *wstore, bool send_
*/
pm_strcpy(job_name, jcr->res.job->name());
bash_spaces(job_name);
pm_strcpy(client_name, jcr->res.client->name());

if (jcr->res.client) {
pm_strcpy(client_name, jcr->res.client->name());
} else {
pm_strcpy(client_name, "**None**");
}
bash_spaces(client_name);
pm_strcpy(fileset_name, jcr->res.fileset->name());

if (jcr->res.fileset) {
pm_strcpy(fileset_name, jcr->res.fileset->name());
} else {
pm_strcpy(fileset_name, "**None**");
}
bash_spaces(fileset_name);

pm_strcpy(backup_format, jcr->backup_format);
bash_spaces(backup_format);

if (jcr->res.fileset->MD5[0] == 0) {
if (jcr->res.fileset && jcr->res.fileset->MD5[0] == 0) {
bstrncpy(jcr->res.fileset->MD5, "**Dummy**", sizeof(jcr->res.fileset->MD5));
fileset_md5 = jcr->res.fileset->MD5;
} else if (jcr->res.fileset) {
fileset_md5 = jcr->res.fileset->MD5;
} else {
fileset_md5 = "**Dummy**";
}

/*
Expand All @@ -183,14 +200,14 @@ bool start_storage_daemon_job(JCR *jcr, alist *rstore, alist *wstore, bool send_
/*
* Retrieve available quota 0 bytes means dont perform the check
*/
remainingquota = quota_fetch_remaining_quota(jcr);
remainingquota = fetch_remaining_quotas(jcr);
Dmsg1(50,"Remainingquota: %llu\n", remainingquota);

sd->fsend(jobcmd, edit_int64(jcr->JobId, ed1), jcr->Job,
job_name.c_str(), client_name.c_str(),
jcr->getJobType(), jcr->getJobLevel(),
fileset_name.c_str(), !jcr->res.pool->catalog_files,
jcr->res.job->SpoolAttributes, jcr->res.fileset->MD5, jcr->spool_data,
jcr->res.job->SpoolAttributes, fileset_md5, jcr->spool_data,
jcr->res.job->PreferMountedVolumes, edit_int64(jcr->spool_size, ed2),
jcr->rerunning, jcr->VolSessionId, jcr->VolSessionTime, remainingquota,
jcr->getJobProtocol(), backup_format.c_str(), jcr->DumpLevel);
Expand Down
4 changes: 2 additions & 2 deletions src/dird/ndmp_dma.c
View file
Expand Up @@ -1689,12 +1689,12 @@ bool do_ndmp_backup(JCR *jcr)
return false;
}

if (quota_check_hardquotas(jcr)) {
if (check_hardquotas(jcr)) {
Jmsg(jcr, M_FATAL, 0, "Quota Exceeded. Job terminated.");
return false;
}

if (quota_check_softquotas(jcr)) {
if (check_softquotas(jcr)) {
Dmsg0(10, "Quota exceeded\n");
Jmsg(jcr, M_FATAL, 0, "Soft Quota Exceeded / Grace Time expired. Job terminated.");
return false;
Expand Down

0 comments on commit a744832

Please sign in to comment.