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 <jdenemar(a)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