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

[zhukeqian]

Hi Daniel,

On 2020/12/4 22:42, Daniel Henrique Barboza wrote:
> 
> 
> On 12/4/20 5:12 AM, zhukeqian wrote:
>> Hi folks,
>>
>> Kindly ping. I found that this patch is not applied.
>> Has reviewed by Daniel Henrique Barboza and Daniel P. Berrangé.
> 
> 
> It has my ACK, but looking into the messages I see that Daniel was
> inquiring about this being a bug fix or an enhancement (he didn't
> provide an ACK). Not sure if he wants some changes in the commit
> message or if he has any other reservations about it.
> 
I see, thanks. I will ask for his thoughts.

Cheers,
Keqian
> 
> 
> 
> Thanks,
> 
> 
> DHB
> 
> 
>>
>> Cheers,
>> Keqian
>>
>> On 2020/7/15 14:18, 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.
>>>
>>> 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",
>>>
> 
> .
>