On Fri, Sep 05, 2014 at 14:42:00 -0400, John Ferlan wrote:
On 09/01/2014 11:05 AM, Jiri Denemark wrote:
virDomainGetJobStats gains new VIR_DOMAIN_JOB_STATS_COMPLETED flag that can be used to fetch statistics of a completed job rather than a currently running job.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/libvirt.h.in | 11 +++++++++++ src/libvirt.c | 8 +++++++- src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 25 +++++++++++++++++++------ src/qemu/qemu_migration.c | 6 ++++++ src/qemu/qemu_monitor_json.c | 3 ++- 7 files changed, 47 insertions(+), 8 deletions(-)
This seems AOK - see my note at the end about qemu_monitor_json.c though.
...
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 62e7d5d..263fa8b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2478,7 +2478,8 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, if (rc == 0) status->downtime_set = true;
- if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE) { + if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE || + status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { virJSONValuePtr ram = virJSONValueObjectGet(ret, "ram"); if (!ram) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
While you're in this module - the following 3 calls don't check status (and were flagged by my new coverity):
virJSONValueObjectGetNumberUlong(ret, "total-time", &status->total_time);
virJSONValueObjectGetNumberUlong(ram, "normal", &status->ram_normal);
virJSONValueObjectGetNumberUlong(ram, "normal-bytes", &status->ram_normal_bytes);
QEMU does not always report all values so we just ignore failures to get some of them. I'll look at them and use ignore_value() when appropriate.
I know "somewhat" outside the bounds of these changes but since they're in qemuMonitorJSONGetMigrationStatusReply() and used by the changes here - there's enough of a reason to adjust that I think.
I don't see where "total_time" is ever used though...
Right, we compute total time ourselves. This is just for completeness to parse all fields QEMU supports in case we want to use them sometime somewhere :-) Jirka