Re: [libvirt] [PATCH v2 08/12] qemu: drop unused code in qemuDomainGetJobStatsInternal
On Fri, Feb 17, 2017 at 17:06:17 +0300, Nikolay Shirokovskiy wrote:
On 16.02.2017 18:45, Jiri Denemark wrote:
> On Wed, Dec 28, 2016 at 17:39:17 +0300, Nikolay Shirokovskiy wrote:
>> Destination migration never fetch migration stats thus due
>
> This becomes true only after patch 10/12.
>
>> to the check above fetch flag can not be set.
>> ---
>> src/qemu/qemu_driver.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 913f8f3..84db59d 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -13037,12 +13037,6 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
>> fetch = false;
>>
>> if (fetch) {
>> - if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) {
>> - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>> - _("migration statistics are available only on
"
>> - "the source host"));
>> - return -1;
>> - }
>
> With a combined knowledge of this patch and 10/12 I think it's better to
> report this error on the destination host rather then successfully
> returning statistics with everything but elapsed time equal to zero.
>
Looks like this place is contradictionary. On dest side it depends on
stats.status whether we fail or return elapsed time. So you suggest
always fail?
Yes.
Won't it break anyone? Probably it depends upon how often one
will
receive fails/time during migration i guess.
I think it should be OK, because it would only return elapsed time for a
very short time between starting QEMU process on the destination host
and actually starting the migration. Well, the time could be longer if
storage migration is involved, but there is still a significant part of
migration for which getting the statistics on the destination host
fails. So anyone doing that should already be able to cope with the
failure :-)
Jirka