On Thu, Jul 01, 2021 at 14:55:56 +0800, [email protected] wrote: > From: Chongyun Wu <[email protected]> > > pointer disk might be null in some special cases or new > usage scenarios, therefore code protection is needed to > prevent segment faults.
Please elaborate on when that happens. Generally the legacy block job handler should no longer be in action with new versions, elaborate please also where you are seeing this. > > Signed-off-by: Chongyun Wu <[email protected]> > --- > src/qemu/qemu_blockjob.c | 43 ++++++++++++++++++++++++------------------- > 1 file changed, 24 insertions(+), 19 deletions(-) > > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c > index faf9a9f..00506b9 100644 > --- a/src/qemu/qemu_blockjob.c > +++ b/src/qemu/qemu_blockjob.c > @@ -781,12 +781,13 @@ qemuBlockJobEventProcessLegacy(virQEMUDriver *driver, > { > virDomainDiskDef *disk = job->disk; > > - VIR_DEBUG("disk=%s, mirrorState=%s, type=%d, state=%d, newstate=%d", > - disk->dst, > - > NULLSTR(virDomainDiskMirrorStateTypeToString(disk->mirrorState)), > - job->type, > - job->state, > - job->newstate); > + if (disk) > + VIR_DEBUG("disk=%s, mirrorState=%s, type=%d, state=%d, newstate=%d", > + disk->dst, > + > NULLSTR(virDomainDiskMirrorStateTypeToString(disk->mirrorState)), > + job->type, > + job->state, > + job->newstate); The legacy block job handler makes no real sense if disk is NULL ... > > if (job->newstate == -1) > return; > @@ -804,26 +805,30 @@ qemuBlockJobEventProcessLegacy(virQEMUDriver *driver, > break; > > case VIR_DOMAIN_BLOCK_JOB_READY: > - disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; > - qemuDomainSaveStatus(vm); > + if (disk) { > + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; > + qemuDomainSaveStatus(vm); ... so the whole function can be skipped ... > + } > break; > > case VIR_DOMAIN_BLOCK_JOB_FAILED: > case VIR_DOMAIN_BLOCK_JOB_CANCELED: > - if (disk->mirror) { > - virDomainLockImageDetach(driver->lockManager, vm, disk->mirror); > + if (disk) { > + if (disk->mirror) { > + virDomainLockImageDetach(driver->lockManager, vm, > disk->mirror); ... rather than special casing everything. > > - /* Ideally, we would restore seclabels on the backing chain here > - * but we don't know if somebody else is not using parts of it. > - * Remove security driver metadata so that they are not leaked. > */ > - qemuBlockRemoveImageMetadata(driver, vm, disk->dst, > disk->mirror); > + /* Ideally, we would restore seclabels on the backing chain > here > + * but we don't know if somebody else is not using parts of > it. > + * Remove security driver metadata so that they are not > leaked. */ > + qemuBlockRemoveImageMetadata(driver, vm, disk->dst, > disk->mirror); > > - virObjectUnref(disk->mirror); > - disk->mirror = NULL; > + virObjectUnref(disk->mirror); > + disk->mirror = NULL; > + } > + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; > + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; > + qemuBlockJobUnregister(job, vm); > } > - disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; > - disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; > - qemuBlockJobUnregister(job, vm); > break; > > case VIR_DOMAIN_BLOCK_JOB_LAST: > -- > 1.8.3.1 >
