Re: [libvirt] [REPOST PATCH v2 02/12] qemu: Split qemuDomainGetIOThreadsLive

2018-11-15 Thread John Ferlan 



On 11/15/18 4:55 AM, Michal Privoznik wrote:
> On 11/05/2018 01:58 PM, John Ferlan wrote:
>> Separate out the fetch of the IOThread monitor call into a separate
>> helper so that a subsequent domain statistics change can fetch the raw
>> IOThread data and parse it as it sees fit.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/qemu/qemu_driver.c | 48 ++
>>  1 file changed, 30 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index a52e2495d5..e13633c1e0 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -5486,39 +5486,52 @@ qemuDomainGetMaxVcpus(virDomainPtr dom)
>>   VIR_DOMAIN_VCPU_MAXIMUM));
>>  }
>>  
>> +
>>  static int
>> -qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
>> -   virDomainObjPtr vm,
>> -   virDomainIOThreadInfoPtr **info)
>> +qemuDomainGetIOThreadsMon(virQEMUDriverPtr driver,
>> +  virDomainObjPtr vm,
>> +  qemuMonitorIOThreadInfoPtr **iothreads)
>>  {
>>  qemuDomainObjPrivatePtr priv;
>> -qemuMonitorIOThreadInfoPtr *iothreads = NULL;
>> -virDomainIOThreadInfoPtr *info_ret = NULL;
>>  int niothreads = 0;
>> -size_t i;
>> -int ret = -1;
>> -
>> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
>> -goto cleanup;
>>  
>>  if (!virDomainObjIsActive(vm)) {
>>  virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("cannot list IOThreads for an inactive domain"));
> 
> I wonder if this check should go into qemuDomainGetIOThreadsLive() right
> after BeginJob(). Rationale behind is that in the next patch, the
> qemuDomainGetStatsIOThread() does the same check already and then it
> calls this function which does the check again. Not crucial though.
> 

Good point - I'll move it (and change return -1 to goto endjob ;-))

Tks

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [REPOST PATCH v2 02/12] qemu: Split qemuDomainGetIOThreadsLive

2018-11-15 Thread Michal Privoznik 
On 11/05/2018 01:58 PM, John Ferlan wrote:
> Separate out the fetch of the IOThread monitor call into a separate
> helper so that a subsequent domain statistics change can fetch the raw
> IOThread data and parse it as it sees fit.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_driver.c | 48 ++
>  1 file changed, 30 insertions(+), 18 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a52e2495d5..e13633c1e0 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5486,39 +5486,52 @@ qemuDomainGetMaxVcpus(virDomainPtr dom)
>   VIR_DOMAIN_VCPU_MAXIMUM));
>  }
>  
> +
>  static int
> -qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
> -   virDomainObjPtr vm,
> -   virDomainIOThreadInfoPtr **info)
> +qemuDomainGetIOThreadsMon(virQEMUDriverPtr driver,
> +  virDomainObjPtr vm,
> +  qemuMonitorIOThreadInfoPtr **iothreads)
>  {
>  qemuDomainObjPrivatePtr priv;
> -qemuMonitorIOThreadInfoPtr *iothreads = NULL;
> -virDomainIOThreadInfoPtr *info_ret = NULL;
>  int niothreads = 0;
> -size_t i;
> -int ret = -1;
> -
> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> -goto cleanup;
>  
>  if (!virDomainObjIsActive(vm)) {
>  virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("cannot list IOThreads for an inactive domain"));

I wonder if this check should go into qemuDomainGetIOThreadsLive() right
after BeginJob(). Rationale behind is that in the next patch, the
qemuDomainGetStatsIOThread() does the same check already and then it
calls this function which does the check again. Not crucial though.

> -goto endjob;
> +return -1;
>  }
>  
>  priv = vm->privateData;
>  if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("IOThreads not supported with this binary"));
> -goto endjob;
> +return -1;
>  }
>  
>  qemuDomainObjEnterMonitor(driver, vm);
> -niothreads = qemuMonitorGetIOThreads(priv->mon, );
> -if (qemuDomainObjExitMonitor(driver, vm) < 0)
> -goto endjob;
> -if (niothreads < 0)
> +niothreads = qemuMonitorGetIOThreads(priv->mon, iothreads);
> +if (qemuDomainObjExitMonitor(driver, vm) < 0 || niothreads < 0)
> +return -1;
> +
> +return niothreads;
> +}
> +
> +
> +static int
> +qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
> +   virDomainObjPtr vm,
> +   virDomainIOThreadInfoPtr **info)
> +{
> +qemuMonitorIOThreadInfoPtr *iothreads = NULL;
> +virDomainIOThreadInfoPtr *info_ret = NULL;
> +int niothreads = 0;
> +size_t i;
> +int ret = -1;
> +
> +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +goto cleanup;
> +
> +if ((niothreads = qemuDomainGetIOThreadsMon(driver, vm, )) < 0)
>  goto endjob;
>  
>  /* Nothing to do */
> @@ -5548,8 +5561,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
>  virBitmapFree(map);
>  }
>  
> -*info = info_ret;
> -info_ret = NULL;
> +VIR_STEAL_PTR(*info, info_ret);
>  ret = niothreads;
>  
>   endjob:
> 

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [REPOST PATCH v2 02/12] qemu: Split qemuDomainGetIOThreadsLive

2018-11-05 Thread John Ferlan 
Separate out the fetch of the IOThread monitor call into a separate
helper so that a subsequent domain statistics change can fetch the raw
IOThread data and parse it as it sees fit.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c | 48 ++
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a52e2495d5..e13633c1e0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5486,39 +5486,52 @@ qemuDomainGetMaxVcpus(virDomainPtr dom)
  VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
+
 static int
-qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
-   virDomainObjPtr vm,
-   virDomainIOThreadInfoPtr **info)
+qemuDomainGetIOThreadsMon(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  qemuMonitorIOThreadInfoPtr **iothreads)
 {
 qemuDomainObjPrivatePtr priv;
-qemuMonitorIOThreadInfoPtr *iothreads = NULL;
-virDomainIOThreadInfoPtr *info_ret = NULL;
 int niothreads = 0;
-size_t i;
-int ret = -1;
-
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
-goto cleanup;
 
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot list IOThreads for an inactive domain"));
-goto endjob;
+return -1;
 }
 
 priv = vm->privateData;
 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("IOThreads not supported with this binary"));
-goto endjob;
+return -1;
 }
 
 qemuDomainObjEnterMonitor(driver, vm);
-niothreads = qemuMonitorGetIOThreads(priv->mon, );
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
-goto endjob;
-if (niothreads < 0)
+niothreads = qemuMonitorGetIOThreads(priv->mon, iothreads);
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || niothreads < 0)
+return -1;
+
+return niothreads;
+}
+
+
+static int
+qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   virDomainIOThreadInfoPtr **info)
+{
+qemuMonitorIOThreadInfoPtr *iothreads = NULL;
+virDomainIOThreadInfoPtr *info_ret = NULL;
+int niothreads = 0;
+size_t i;
+int ret = -1;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
+if ((niothreads = qemuDomainGetIOThreadsMon(driver, vm, )) < 0)
 goto endjob;
 
 /* Nothing to do */
@@ -5548,8 +5561,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
 virBitmapFree(map);
 }
 
-*info = info_ret;
-info_ret = NULL;
+VIR_STEAL_PTR(*info, info_ret);
 ret = niothreads;
 
  endjob:
-- 
2.17.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list