On 08/31/14 06:02, Eric Blake wrote:
> Another layer of overly-multiplexed code that deserves to be
> split into obviously separate paths for query vs. modify.
> This continues the cleanup started in the previous patch.
>
> In the process, make some tweaks to simplify the logic when
> parsing the JSON reply. There should be no user-visible
> semantic changes.
>
> * src/qemu/qemu_monitor.h (qemuMonitorBlockJob): Drop parameter.
> (qemuMonitorBlockJobInfo): New prototype.
> (BLOCK_JOB_INFO): Drop enum.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockJob)
> (qemuMonitorJSONBlockJobInfo): Likewise.
> * src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Split...
> (qemuMonitorBlockJobInfo): ...into second function.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Move
> block info portions...
> (qemuMonitorJSONGetBlockJobInfo): ...here, and rename...
> (qemuMonitorJSONBlockJobInfo): ...and export.
> (qemuMonitorJSONGetBlockJobInfoOne): Alter return semantics.
> * src/qemu/qemu_driver.c (qemuDomainBlockPivot)
> (qemuDomainBlockJobImpl, qemuDomainGetBlockJobInfo): Adjust
> callers.
> * src/qemu/qemu_migration.c (qemuMigrationDriveMirror)
> (qemuMigrationCancelDriveMirror): Likewise.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 11 +++-----
> src/qemu/qemu_migration.c | 7 +++--
> src/qemu/qemu_monitor.c | 41 ++++++++++++++++++++--------
> src/qemu/qemu_monitor.h | 7 +++--
> src/qemu/qemu_monitor_json.c | 63 ++++++++++++++++++++++++--------------------
> src/qemu/qemu_monitor_json.h | 6 ++++-
> 6 files changed, 81 insertions(+), 54 deletions(-)
>
...
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 4fd6f01..947ca34 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -695,11 +694,15 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
> const char *base,
> const char *backingName,
> unsigned long bandwidth,
> - virDomainBlockJobInfoPtr info,
> qemuMonitorBlockJobCmd mode,
> bool modern)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> +int qemuMonitorBlockJobInfo(qemuMonitorPtr mon,
> + const char *device,
> + virDomainBlockJobInfoPtr info)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
Implementation of this function doesn't check for presence of the @info
structure and dereferences it always. You should mark argument 3 nonnull
too.
> +
> int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
> const char *protocol,
> int fd,
...
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 62e7d5d..68ba084 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3733,54 +3735,66 @@ static int qemuMonitorJSONGetBlockJobInfoOne(virJSONValuePtr entry,
> _("entry was missing 'len'"));
> return -1;
> }
> - return 0;
> + return 1;
> }
>
> -/** qemuMonitorJSONGetBlockJobInfo:
> - * Parse Block Job information.
> - * The reply is a JSON array of objects, one per active job.
> +/**
> + * qemuMonitorJSONBlockJobInfo:
> + * Parse Block Job information, and populate info for the named device.
> + * Return 1 if info available, 0 if device has no block job, and -1 on error.
> */
> -static int qemuMonitorJSONGetBlockJobInfo(virJSONValuePtr reply,
> - const char *device,
> - virDomainBlockJobInfoPtr info)
> +int
> +qemuMonitorJSONBlockJobInfo(qemuMonitorPtr mon,
> + const char *device,
> + virDomainBlockJobInfoPtr info)
> {
> + virJSONValuePtr cmd = NULL;
> + virJSONValuePtr reply = NULL;
> virJSONValuePtr data;
> int nr_results;
> size_t i;
> + int ret;
>
> - if (!info)
> + cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL);
> + if (!cmd)
> return -1;
> + ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> + if (ret < 0)
> + goto cleanup;
>
> if ((data = virJSONValueObjectGet(reply, "return")) == NULL) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("reply was missing return data"));
> - return -1;
> + goto cleanup;
> }
>
> if (data->type != VIR_JSON_TYPE_ARRAY) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("unrecognized format of block job information"));
> - return -1;
> + goto cleanup;
> }
>
> if ((nr_results = virJSONValueArraySize(data)) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("unable to determine array size"));
> - return -1;
> + goto cleanup;
> }
>
> - for (i = 0; i < nr_results; i++) {
> + for (i = 0, ret = 0; i < nr_results && ret == 0; i++) {
> virJSONValuePtr entry = virJSONValueArrayGet(data, i);
> if (!entry) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("missing array element"));
> - return -1;
> + ret = -1;
> + goto cleanup;
> }
> - if (qemuMonitorJSONGetBlockJobInfoOne(entry, device, info) == 0)
> - return 1;
> + ret = qemuMonitorJSONGetBlockJobInfoOne(entry, device, info);
This doesn't break the loop, thus if the device info isn't last in the
structure you'd overwrite the return code. I presume the clients don't
check that far but you've documented that semantics.
> }
>
> - return 0;
> + cleanup:
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> }
>
>
...
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index d8c9308..d3f6f37 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -285,11 +285,15 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> const char *base,
> const char *backingName,
> unsigned long long speed,
> - virDomainBlockJobInfoPtr info,
> qemuMonitorBlockJobCmd mode,
> bool modern)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> +int qemuMonitorJSONBlockJobInfo(qemuMonitorPtr mon,
> + const char *device,
> + virDomainBlockJobInfoPtr info)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
Again, @info is required, thus should be marked ATTRIBUTE_NONNULL.
> +
> int qemuMonitorJSONSetLink(qemuMonitorPtr mon,
> const char *name,
> virDomainNetInterfaceLinkState state);
>
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list