subject:"\[PATCH v4 4\/7\] migration\/dirtyrate\: Implement qemuDomainQueryDirtyRate"

Re: [PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

2020-11-16 Thread Hao Wang

Thanks for these reviews too. I'll apply all of them.

On 2020/11/11 4:11, Michal Privoznik wrote:
> On 11/7/20 10:41 AM, Hao Wang wrote:
>> Implement qemuDomainQueryDirtyRate which query domain's memory
>> dirty rate calling qmp "query-dirty-rate".
>>
>> Signed-off-by: Hao Wang 
>> Signed-off-by: Zhou Yimin 
>> Reviewed-by: Chuan Zheng 
>> ---
>>   src/qemu/qemu_migration.c    | 31 +++
>>   src/qemu/qemu_migration.h    |  5 +
>>   src/qemu/qemu_monitor.c  | 12 
>>   src/qemu/qemu_monitor.h  |  4 
>>   src/qemu/qemu_monitor_json.c | 22 ++
>>   src/qemu/qemu_monitor_json.h |  4 
>>   6 files changed, 78 insertions(+)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index 8029e24415..3d07ba3ac4 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -5864,3 +5864,34 @@ qemuDomainCalculateDirtyRate(virDomainPtr dom,
>>     return ret;
>>   }
>> +
>> +
>> +int
>> +qemuDomainQueryDirtyRate(virDomainPtr dom,
>> + virDomainObjPtr vm,
>> + virDomainDirtyRateInfoPtr info)
>> +{
>> +    virQEMUDriverPtr driver = dom->conn->privateData;
> 
> Again, driver is accessible from the caller, just pass it directly.
> 
>> +    qemuDomainObjPrivatePtr priv;
>> +    int ret = -1;
>> +
>> +    if (!virDomainObjIsActive(vm)) {
>> +    virReportError(VIR_ERR_OPERATION_INVALID,
>> +   "%s", _("domain is not running"));
>> +    return ret;
>> +    }
>> +
>> +    priv = vm->privateData;
>> +
>> +    qemuDomainObjEnterMonitor(driver, vm);
>> +
>> +    ret = qemuMonitorQueryDirtyRate(priv->mon, info);
>> +    if (ret < 0) {
>> +    virReportError(VIR_ERR_OPERATION_FAILED,
>> +   "%s", _("get vm's dirty rate failed."));
> 
> No, this is not correct, qemuMonitorQueryDirtyRate() will report an error if 
> something goes wrong. And with pretty accurate error message. This overwrites 
> it with more generic one.
> 
>> +    }
>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +    ret = -1;
>> +
>> +    return ret;
>> +}
>> diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
>> index 0522b375c0..8baae512b7 100644
>> --- a/src/qemu/qemu_migration.h
>> +++ b/src/qemu/qemu_migration.h
>> @@ -263,3 +263,8 @@ int
>>   qemuDomainCalculateDirtyRate(virDomainPtr dom,
>>    virDomainObjPtr vm,
>>    long long sec);
>> +
>> +int
>> +qemuDomainQueryDirtyRate(virDomainPtr dom,
>> + virDomainObjPtr vm,
>> + virDomainDirtyRateInfoPtr info);
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index 06603b8691..2fa6879467 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -4781,3 +4781,15 @@ qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
>>     return qemuMonitorJSONCalculateDirtyRate(mon, sec);
>>   }
>> +
>> +
>> +int
>> +qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
>> +  virDomainDirtyRateInfoPtr info)
>> +{
>> +    VIR_DEBUG("info=%p", info);
>> +
>> +    QEMU_CHECK_MONITOR(mon);
>> +
>> +    return qemuMonitorJSONQueryDirtyRate(mon, info);
>> +}
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index afb97780cf..25105c3ad9 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1531,3 +1531,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
>>   int
>>   qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
>>     long long sec);
>> +
>> +int
>> +qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
>> +  virDomainDirtyRateInfoPtr info);
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 65691522fb..1924c7229b 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -9657,3 +9657,25 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
>>     return 0;
>>   }
>> +
>> +
>> +int
>> +qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
>> +  virDomainDirtyRateInfoPtr info)
>> +{
>> +    g_autoptr(virJSONValue) cmd = NULL;
>> +    g_autoptr(virJSONValue) reply = NULL;
>> +
>> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
>> +    return -1;
>> +
>> +    if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
>> +    return -1;
>> +
>> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>> +    return -1;
>> +
>> +    /* TODO: extract dirtyrate data from reply and store in 
>> virDomainDirtyRateInfoPtr */
> 
> How about instead of noting down to finish this, squash 5/7 here? It's not 
> like somebody could backport only this or that patch.
> 
>> +    info->status = 0;
>> +    return 0;
>> +}
>> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
>> index c9556fc19a..487f2e6e58 100644
>>

Re: [PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

2020-11-10 Thread Michal Privoznik


On 11/7/20 10:41 AM, Hao Wang wrote:

Implement qemuDomainQueryDirtyRate which query domain's memory
dirty rate calling qmp "query-dirty-rate".

Signed-off-by: Hao Wang 
Signed-off-by: Zhou Yimin 
Reviewed-by: Chuan Zheng 
---
  src/qemu/qemu_migration.c| 31 +++
  src/qemu/qemu_migration.h|  5 +
  src/qemu/qemu_monitor.c  | 12 
  src/qemu/qemu_monitor.h  |  4 
  src/qemu/qemu_monitor_json.c | 22 ++
  src/qemu/qemu_monitor_json.h |  4 
  6 files changed, 78 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8029e24415..3d07ba3ac4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5864,3 +5864,34 @@ qemuDomainCalculateDirtyRate(virDomainPtr dom,
  
  return ret;

  }
+
+
+int
+qemuDomainQueryDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainDirtyRateInfoPtr info)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;


Again, driver is accessible from the caller, just pass it directly.


+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+return ret;
+}
+
+priv = vm->privateData;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorQueryDirtyRate(priv->mon, info);
+if (ret < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   "%s", _("get vm's dirty rate failed."));


No, this is not correct, qemuMonitorQueryDirtyRate() will report an 
error if something goes wrong. And with pretty accurate error message. 
This overwrites it with more generic one.



+}
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 0522b375c0..8baae512b7 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -263,3 +263,8 @@ int
  qemuDomainCalculateDirtyRate(virDomainPtr dom,
   virDomainObjPtr vm,
   long long sec);
+
+int
+qemuDomainQueryDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 06603b8691..2fa6879467 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4781,3 +4781,15 @@ qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
  
  return qemuMonitorJSONCalculateDirtyRate(mon, sec);

  }
+
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info)
+{
+VIR_DEBUG("info=%p", info);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONQueryDirtyRate(mon, info);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index afb97780cf..25105c3ad9 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1531,3 +1531,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
  int
  qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
long long sec);
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 65691522fb..1924c7229b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9657,3 +9657,25 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
  
  return 0;

  }
+
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+/* TODO: extract dirtyrate data from reply and store in 
virDomainDirtyRateInfoPtr */


How about instead of noting down to finish this, squash 5/7 here? It's 
not like somebody could backport only this or that patch.



+info->status = 0;
+return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index c9556fc19a..487f2e6e58 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -714,3 +714,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
  int
  qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
long long sec);
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info);



I think the following should be squashed in:

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index

[PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

2020-11-07 Thread Hao Wang

Implement qemuDomainQueryDirtyRate which query domain's memory
dirty rate calling qmp "query-dirty-rate".

Signed-off-by: Hao Wang 
Signed-off-by: Zhou Yimin 
Reviewed-by: Chuan Zheng 
---
 src/qemu/qemu_migration.c| 31 +++
 src/qemu/qemu_migration.h|  5 +
 src/qemu/qemu_monitor.c  | 12 
 src/qemu/qemu_monitor.h  |  4 
 src/qemu/qemu_monitor_json.c | 22 ++
 src/qemu/qemu_monitor_json.h |  4 
 6 files changed, 78 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8029e24415..3d07ba3ac4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5864,3 +5864,34 @@ qemuDomainCalculateDirtyRate(virDomainPtr dom,
 
 return ret;
 }
+
+
+int
+qemuDomainQueryDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainDirtyRateInfoPtr info)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+return ret;
+}
+
+priv = vm->privateData;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorQueryDirtyRate(priv->mon, info);
+if (ret < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   "%s", _("get vm's dirty rate failed."));
+}
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 0522b375c0..8baae512b7 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -263,3 +263,8 @@ int
 qemuDomainCalculateDirtyRate(virDomainPtr dom,
  virDomainObjPtr vm,
  long long sec);
+
+int
+qemuDomainQueryDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 06603b8691..2fa6879467 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4781,3 +4781,15 @@ qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
 
 return qemuMonitorJSONCalculateDirtyRate(mon, sec);
 }
+
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info)
+{
+VIR_DEBUG("info=%p", info);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONQueryDirtyRate(mon, info);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index afb97780cf..25105c3ad9 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1531,3 +1531,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
 int
 qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
   long long sec);
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 65691522fb..1924c7229b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9657,3 +9657,25 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
 
 return 0;
 }
+
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+/* TODO: extract dirtyrate data from reply and store in 
virDomainDirtyRateInfoPtr */
+info->status = 0;
+return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index c9556fc19a..487f2e6e58 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -714,3 +714,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
 int
 qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
   long long sec);
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+  virDomainDirtyRateInfoPtr info);
-- 
2.23.0

Re: [PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

Re: [PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

[PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

3 matches

Site Navigation

Mail list logo

Footer information