[PATCH] Qemu: migration: Not bind RAM info with active migration status

[zhukeqian]

On 2020/7/28 18:32, Daniel P. Berrangé wrote:
> On Wed, Jul 15, 2020 at 02:18:01PM +0800, Keqian Zhu wrote:
>> For that Qemu supports returning incoming migration info since its commit
>> 65ace0604551 (migration: add postcopy total blocktime into query-migrate),
>> which may contains active status, but without RAM info. Drop this binding
>> relationship check in libvirt.
> 
> I'm not clear from this description what the actual bug in libvirt
> is ?   What currently fails in libvirt that this patch fixes ?
> 
When query migration status, libvirt assumes that when migration status is active,
then RAM info must be set. However qemu has changed this assumption, so libvirt will
report "migration was active, but no RAM info was set" when query migration info.

Thanks,
Keqian
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1 at huawei.com>
>> ---
>>  src/qemu/qemu_monitor_json.c | 88 +++++++++++++++++-------------------
>>  1 file changed, 42 insertions(+), 46 deletions(-)
>>
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index d808c4b55b..ba8e340742 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -3547,56 +3547,52 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply,
>>      case QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER:
>>      case QEMU_MONITOR_MIGRATION_STATUS_DEVICE:
>>          ram = virJSONValueObjectGetObject(ret, "ram");
>> -        if (!ram) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                           _("migration was active, but no RAM info was set"));
>> -            return -1;
>> -        }
>> +        if (ram) {
>> +            if (virJSONValueObjectGetNumberUlong(ram, "transferred",
>> +                                                 &stats->ram_transferred) < 0) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("migration was active, but RAM 'transferred' "
>> +                                 "data was missing"));
>> +                return -1;
>> +            }
>> +            if (virJSONValueObjectGetNumberUlong(ram, "remaining",
>> +                                                 &stats->ram_remaining) < 0) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("migration was active, but RAM 'remaining' "
>> +                                 "data was missing"));
>> +                return -1;
>> +            }
>> +            if (virJSONValueObjectGetNumberUlong(ram, "total",
>> +                                                 &stats->ram_total) < 0) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("migration was active, but RAM 'total' "
>> +                                 "data was missing"));
>> +                return -1;
>> +            }
>>  
>> -        if (virJSONValueObjectGetNumberUlong(ram, "transferred",
>> -                                             &stats->ram_transferred) < 0) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                           _("migration was active, but RAM 'transferred' "
>> -                             "data was missing"));
>> -            return -1;
>> -        }
>> -        if (virJSONValueObjectGetNumberUlong(ram, "remaining",
>> -                                             &stats->ram_remaining) < 0) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                           _("migration was active, but RAM 'remaining' "
>> -                             "data was missing"));
>> -            return -1;
>> -        }
>> -        if (virJSONValueObjectGetNumberUlong(ram, "total",
>> -                                             &stats->ram_total) < 0) {
>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                           _("migration was active, but RAM 'total' "
>> -                             "data was missing"));
>> -            return -1;
>> -        }
>> +            if (virJSONValueObjectGetNumberDouble(ram, "mbps", &mbps) == 0 &&
>> +                mbps > 0) {
>> +                /* mpbs from QEMU reports Mbits/s (M as in 10^6 not Mi as 2^20) */
>> +                stats->ram_bps = mbps * (1000 * 1000 / 8);
>> +            }
>>  
>> -        if (virJSONValueObjectGetNumberDouble(ram, "mbps", &mbps) == 0 &&
>> -            mbps > 0) {
>> -            /* mpbs from QEMU reports Mbits/s (M as in 10^6 not Mi as 2^20) */
>> -            stats->ram_bps = mbps * (1000 * 1000 / 8);
>> +            if (virJSONValueObjectGetNumberUlong(ram, "duplicate",
>> +                                                 &stats->ram_duplicate) == 0)
>> +                stats->ram_duplicate_set = true;
>> +            ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal",
>> +                                                          &stats->ram_normal));
>> +            ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal-bytes",
>> +                                                          &stats->ram_normal_bytes));
>> +            ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-pages-rate",
>> +                                                          &stats->ram_dirty_rate));
>> +            ignore_value(virJSONValueObjectGetNumberUlong(ram, "page-size",
>> +                                                          &stats->ram_page_size));
>> +            ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-sync-count",
>> +                                                          &stats->ram_iteration));
>> +            ignore_value(virJSONValueObjectGetNumberUlong(ram, "postcopy-requests",
>> +                                                          &stats->ram_postcopy_reqs));
>>          }
>>  
>> -        if (virJSONValueObjectGetNumberUlong(ram, "duplicate",
>> -                                             &stats->ram_duplicate) == 0)
>> -            stats->ram_duplicate_set = true;
>> -        ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal",
>> -                                                      &stats->ram_normal));
>> -        ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal-bytes",
>> -                                                      &stats->ram_normal_bytes));
>> -        ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-pages-rate",
>> -                                                      &stats->ram_dirty_rate));
>> -        ignore_value(virJSONValueObjectGetNumberUlong(ram, "page-size",
>> -                                                      &stats->ram_page_size));
>> -        ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-sync-count",
>> -                                                      &stats->ram_iteration));
>> -        ignore_value(virJSONValueObjectGetNumberUlong(ram, "postcopy-requests",
>> -                                                      &stats->ram_postcopy_reqs));
>> -
>>          disk = virJSONValueObjectGetObject(ret, "disk");
>>          if (disk) {
>>              rc = virJSONValueObjectGetNumberUlong(disk, "transferred",
>> -- 
>> 2.19.1
>>
> 
> Regards,
> Daniel
>