Hi Daniel,
On 2020/7/17 22:33, Daniel Henrique Barboza wrote:
>
>
> On 7/15/20 3:18 AM, Keqian Zhu wrote:
>> For that Qemu supports returning incoming migration info since its commit
>> 65ace0604551 (migration: add postcopy total blocktime into query-migrate),
>
>
> It is worth saying that this QEMU commit that decoupled the RAM
> status from the active migration status is live since early 2018:
>
> $ git show 65ace0604551
> commit 65ace060455122a461cdc9302238b914084bcd42
> Author: Alexey Perevalov <a.perevalov@xxxxxxxxxxx>
> Date: Thu Mar 22 21:17:27 2018 +0300
>
> migration: add postcopy total blocktime into query-migrate
>
> $ git describe 65ace0604551
> v2.12.0-6-g65ace06045
>
>
> I am not sure if we care about removing a migration failure check for
> QEMU 2.12 when we're waiting for 5.1 to come out. My guess is that we
> do care, but not enough to demand a "if (QEMU <= 2.12)" in this logic.
> I'll also assume that the existing failure check is doing more harm than
> good nowadays, so:
>
Thanks for your review.
Thanks,
Keqian
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
>
>
>> which may contains active status, but without RAM info. Drop this binding
>> relationship check in libvirt.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@xxxxxxxxxx>
>> ---
>
>
>
>
>
>> 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",
>>
> .
>
