On Tue, Jun 09, 2015 at 16:56:34 +0200, Peter Krempa wrote: > On Tue, Jun 02, 2015 at 14:34:07 +0200, Jiri Denemark wrote: > > The wrapper is useful for calling qemuBlockJobEventProcess with the > > event details stored in disk's privateData, which is the most likely > > usage of qemuBlockJobEventProcess. > > > > Signed-off-by: Jiri Denemark <jdene...@redhat.com> > > --- > > > > Notes: > > Already ACKed in version 1. > > > > Version 2: > > - no changes > > > > src/libvirt_private.syms | 2 ++ > > src/qemu/qemu_blockjob.c | 37 +++++++++++++++++++++++++++++-------- > > src/qemu/qemu_blockjob.h | 3 +++ > > 3 files changed, 34 insertions(+), 8 deletions(-) > > > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > > index 9076135..8846dea 100644 > > --- a/src/libvirt_private.syms > > +++ b/src/libvirt_private.syms > > @@ -265,6 +265,8 @@ virDomainDiskInsert; > > virDomainDiskInsertPreAlloced; > > virDomainDiskIoTypeFromString; > > virDomainDiskIoTypeToString; > > +virDomainDiskMirrorStateTypeFromString; > > +virDomainDiskMirrorStateTypeToString; > > virDomainDiskPathByName; > > virDomainDiskRemove; > > virDomainDiskRemoveByName; > > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c > > index 098a43a..605c2a5 100644 > > --- a/src/qemu/qemu_blockjob.c > > +++ b/src/qemu/qemu_blockjob.c > > @@ -38,6 +38,27 @@ > > > > VIR_LOG_INIT("qemu.qemu_blockjob"); > > > > + > > +int > > +qemuBlockJobUpdate(virQEMUDriverPtr driver, > > + virDomainObjPtr vm, > > + virDomainDiskDefPtr disk) > > +{ > > + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); > > + int ret; > > + > > + if ((ret = diskPriv->blockJobStatus) == -1) > > + return -1; > > + > > + qemuBlockJobEventProcess(driver, vm, disk, > > + diskPriv->blockJobType, > > + diskPriv->blockJobStatus); > > + diskPriv->blockJobStatus = -1; > > + > > + return ret; > > +} > > While reading this function the second time I realized that the control > flow looks weird. > > How about: > > > int > qemuBlockJobUpdate(virQEMUDriverPtr driver, > virDomainObjPtr vm, > virDomainDiskDefPtr disk) > { > qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); > int ret = diskPriv->blockJobStatus; > > if (diskPriv->blockJobStatus != -1) { > qemuBlockJobEventProcess(driver, vm, disk, > diskPriv->blockJobType, > diskPriv->blockJobStatus); > diskPriv->blockJobStatus = -1; > } > > return ret; > }
Yeah, although I will also rename "ret" to "status" since the name implicitly suggests semantics of -1... anyone seeing ret = -1 would consider it a failure. But this function does not fail, it just returns the original value stored in blockJobStatus. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list