Re: [PATCH v4 3/7] migration/dirtyrate: Implement qemuDomainCalculateDirtyRate

2020-11-16 Thread Hao Wang 
Great thanks for these helpful suggestions. I'll introduce them into my next 
version.

On 2020/11/11 4:11, Michal Privoznik wrote:
> On 11/7/20 10:41 AM, Hao Wang wrote:
>> Implement qemuDomainCalculateDirtyRate which calculates domain's memory
>> dirty rate calling qmp "calc-dirty-rate".
>>
>> Signed-off-by: Hao Wang 
>> Signed-off-by: Zhou Yimin 
>> Reviewed-by: Chuan Zheng 
>> ---
>>   src/qemu/qemu_migration.c    | 28 
>>   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, 75 insertions(+)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index 6f764b0c73..8029e24415 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -5836,3 +5836,31 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr 
>> driver,
>>   virHashFree(blockinfo);
>>   return 0;
>>   }
>> +
>> +
>> +int
>> +qemuDomainCalculateDirtyRate(virDomainPtr dom,
> 
> The caller (implemented later in the series) has pointer to the driver. Might 
> as well pass it here. Alternativelly, there is a piggy back pointer stored 
> inside @vm's private data: QEMU_DOMAIN_PRIVATE(vm)->driver
> 
>> + virDomainObjPtr vm,
>> + long long sec)
>> +{
>> +    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;
> 
> It's okay to initialize priv when declaring it.
> 
>> +
>> +    VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);
>> +    qemuDomainObjEnterMonitor(driver, vm);
>> +
>> +    ret = qemuMonitorCalculateDirtyRate(priv->mon, sec);
>> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +    ret = -1;
>> +
>> +    return ret;
>> +}
>> diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
>> index fd9eb7cab0..0522b375c0 100644
>> --- a/src/qemu/qemu_migration.h
>> +++ b/src/qemu/qemu_migration.h
>> @@ -258,3 +258,8 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
>>    virDomainObjPtr vm,
>>    qemuDomainAsyncJob asyncJob,
>>    qemuDomainJobInfoPtr jobInfo);
>> +
>> +int
>> +qemuDomainCalculateDirtyRate(virDomainPtr dom,
>> + virDomainObjPtr vm,
>> + long long sec);
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index 5481bd99a0..06603b8691 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -4769,3 +4769,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
>>   return qemuMonitorJSONTransactionBackup(actions, device, jobname, 
>> target,
>>   bitmap, syncmode);
>>   }
>> +
>> +
>> +int
>> +qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
>> +  long long sec)
>> +{
>> +    VIR_DEBUG("seconds=%lld", sec);
>> +
>> +    QEMU_CHECK_MONITOR(mon);
>> +
>> +    return qemuMonitorJSONCalculateDirtyRate(mon, sec);
>> +}
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index 54fbb41ef7..afb97780cf 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
>>    const char *target,
>>    const char *bitmap,
>>    qemuMonitorTransactionBackupSyncMode 
>> syncmode);
>> +
>> +int
>> +qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
>> +  long long sec);
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 843a555952..65691522fb 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -9635,3 +9635,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
>>   return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
>>     migratable);
>>   }
>> +
>> +
>> +int
>> +qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
>> +  long long sec)
>> +{
>> +    g_autoptr(virJSONValue) cmd = NULL;
>> +    g_autoptr(virJSONValue) reply = NULL;
>> +
>> +    if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
>> +   "I:calc-time", (long)sec,
> 
> I'm not convinced on this typecast. qemuMonitorJSONMakeCommand() will under 
> the hood pop long long. But anyway, @sec shouldn't be long long in the first 
> place.
> 
>> +  

Re: [PATCH v4 3/7] migration/dirtyrate: Implement qemuDomainCalculateDirtyRate

2020-11-10 Thread Michal Privoznik 

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

Implement qemuDomainCalculateDirtyRate which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".

Signed-off-by: Hao Wang 
Signed-off-by: Zhou Yimin 
Reviewed-by: Chuan Zheng 
---
  src/qemu/qemu_migration.c| 28 
  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, 75 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f764b0c73..8029e24415 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5836,3 +5836,31 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
  virHashFree(blockinfo);
  return 0;
  }
+
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,


The caller (implemented later in the series) has pointer to the driver. 
Might as well pass it here. Alternativelly, there is a piggy back 
pointer stored inside @vm's private data: QEMU_DOMAIN_PRIVATE(vm)->driver



+ virDomainObjPtr vm,
+ long long sec)
+{
+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;


It's okay to initialize priv when declaring it.


+
+VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorCalculateDirtyRate(priv->mon, sec);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index fd9eb7cab0..0522b375c0 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -258,3 +258,8 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   qemuDomainAsyncJob asyncJob,
   qemuDomainJobInfoPtr jobInfo);
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ long long sec);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5481bd99a0..06603b8691 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4769,3 +4769,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
  return qemuMonitorJSONTransactionBackup(actions, device, jobname, target,
  bitmap, syncmode);
  }
+
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec)
+{
+VIR_DEBUG("seconds=%lld", sec);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONCalculateDirtyRate(mon, sec);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 54fbb41ef7..afb97780cf 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
   const char *target,
   const char *bitmap,
   qemuMonitorTransactionBackupSyncMode syncmode);
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 843a555952..65691522fb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9635,3 +9635,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
  return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
migratable);
  }
+
+
+int
+qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
+   "I:calc-time", (long)sec,


I'm not convinced on this typecast. qemuMonitorJSONMakeCommand() will 
under the hood pop long long. But anyway, @sec shouldn't be long long in 
the first place.



+   NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 38e23ef3c5..c9556fc19a 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -710,3 +710,7 @@ int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon,
  int
  q

[PATCH v4 3/7] migration/dirtyrate: Implement qemuDomainCalculateDirtyRate

2020-11-07 Thread Hao Wang 
Implement qemuDomainCalculateDirtyRate which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".

Signed-off-by: Hao Wang 
Signed-off-by: Zhou Yimin 
Reviewed-by: Chuan Zheng 
---
 src/qemu/qemu_migration.c| 28 
 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, 75 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f764b0c73..8029e24415 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5836,3 +5836,31 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
 virHashFree(blockinfo);
 return 0;
 }
+
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ long long sec)
+{
+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;
+
+VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorCalculateDirtyRate(priv->mon, sec);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index fd9eb7cab0..0522b375c0 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -258,3 +258,8 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  qemuDomainAsyncJob asyncJob,
  qemuDomainJobInfoPtr jobInfo);
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ long long sec);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5481bd99a0..06603b8691 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4769,3 +4769,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
 return qemuMonitorJSONTransactionBackup(actions, device, jobname, target,
 bitmap, syncmode);
 }
+
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec)
+{
+VIR_DEBUG("seconds=%lld", sec);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONCalculateDirtyRate(mon, sec);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 54fbb41ef7..afb97780cf 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
  const char *target,
  const char *bitmap,
  qemuMonitorTransactionBackupSyncMode syncmode);
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 843a555952..65691522fb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9635,3 +9635,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
 return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
   migratable);
 }
+
+
+int
+qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
+   "I:calc-time", (long)sec,
+   NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 38e23ef3c5..c9556fc19a 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -710,3 +710,7 @@ int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon,
 int
 qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
 bool *migratable);
+
+int
+qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
+  long long sec);
-- 
2.23.0