On Thu, 10 Oct 2019 at 12:44, Max Reitz <mre...@redhat.com> wrote: > > From: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > > Drop write notifiers and use filter node instead.
Hi; after this change Coverity complains about dead code in backup_job_create() (CID 1406402): > @@ -382,6 +353,8 @@ BlockJob *backup_job_create(const char *job_id, > BlockDriverState *bs, > BackupBlockJob *job = NULL; > int64_t cluster_size; > BdrvRequestFlags write_flags; > + BlockDriverState *backup_top = NULL; > + BlockCopyState *bcs = NULL; > > assert(bs); > assert(target); > @@ -463,33 +436,35 @@ BlockJob *backup_job_create(const char *job_id, > BlockDriverState *bs, > write_flags = (bdrv_chain_contains(target, bs) ? BDRV_REQ_SERIALISING : > 0) | > (compress ? BDRV_REQ_WRITE_COMPRESSED : 0), > > + backup_top = bdrv_backup_top_append(bs, target, filter_node_name, > + cluster_size, write_flags, &bcs, > errp); > + if (!backup_top) { > + goto error; > + } > + > /* job->len is fixed, so we can't allow resize */ > - job = block_job_create(job_id, &backup_job_driver, txn, bs, 0, > BLK_PERM_ALL, > + job = block_job_create(job_id, &backup_job_driver, txn, backup_top, > + 0, BLK_PERM_ALL, > speed, creation_flags, cb, opaque, errp); > if (!job) { > goto error; > } > > + job->backup_top = backup_top; > job->source_bs = bs; > job->on_source_error = on_source_error; > job->on_target_error = on_target_error; > job->sync_mode = sync_mode; > job->sync_bitmap = sync_bitmap; > job->bitmap_mode = bitmap_mode; > - > - job->bcs = block_copy_state_new(bs, target, cluster_size, write_flags, > - errp); > - if (!job->bcs) { > - goto error; > - } > - This code deletion has removed the only code path that can reach the 'error' label after the successful creation of the job... > + job->bcs = bcs; > job->cluster_size = cluster_size; > job->len = len; > > - block_copy_set_callbacks(job->bcs, backup_progress_bytes_callback, > + block_copy_set_callbacks(bcs, backup_progress_bytes_callback, > backup_progress_reset_callback, job); > > - /* Required permissions are already taken by block-copy-state target */ > + /* Required permissions are already taken by backup-top target */ > block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL, > &error_abort); > > @@ -502,6 +477,8 @@ BlockJob *backup_job_create(const char *job_id, > BlockDriverState *bs, > if (job) { > backup_clean(&job->common.job); > job_early_fail(&job->common.job); ...so down here in the 'error:' code the "if (job)" condition can never pass, and the code in this part of the if statement is dead. > + } else if (backup_top) { > + bdrv_backup_top_drop(backup_top); > } > > return NULL; > {"return": {}} thanks -- PMM