[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 09/12] [v2] virNodeGetMemoryStats: Implement public API



On 06/14/2011 03:17 AM, Daniel P. Berrange wrote:
> On Tue, Jun 07, 2011 at 10:04:54AM +0900, Minoru Usui wrote:
>> virNodeGetMemoryStats: Implement public API
>>
>> Signed-off-by: Minoru Usui <usui mxm nes nec co jp>
>> ---
>>  include/libvirt/libvirt.h.in |    2 +-
>>  src/libvirt.c                |   87 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 88 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>> index 885db25..b7772ba 100644
>> --- a/include/libvirt/libvirt.h.in
>> +++ b/include/libvirt/libvirt.h.in
>> @@ -824,7 +824,7 @@ int                     virNodeGetCPUStats (virConnectPtr conn,
>>  
>>  int                     virNodeGetMemoryStats (virConnectPtr conn,
>>                                                 int cellNum,
>> -                                               virCPUStatsPtr params,
>> +                                               virMemoryStatsPtr params,
>>                                                 int *nparams,
>>                                                 unsigned int flags);
> 
> 
> Opps, I think this chunk should be in your first patch.

Aargh - too late (I already pushed the broken version).  Oh well, it
goes to prove that we have ABI compatible structs, and that perhaps a
consolidated common struct definition before 0.9.3 would be wise.

>> + * Here is the sample code snippet:
>> + *
>> + * if ((virNodeGetMemoryStats(conn, cellNum, NULL, &nparams, 0) == 0) &&
>> + *     (nparams != 0)) {
>> + *     params = vshMalloc(ctl, sizeof(virMemoryStats) * nparams);
>> + *     memset(params, cellNum, 0, sizeof(virMemoryStats) * nparams);
>> + *     if (virNodeGetMemoryStats(conn, params, &nparams, 0)) {
>> + *         vshError(ctl, "%s", _("Unable to get node memory stats"));
>> + *         goto error;
>> + *     }
>> + * }
> 
> 
> Same comment as the earlier patch - use malloc() and fprintf(stderr)
> here.
> 
>> + *
>> + * This function doesn't requires privileged access to the hypervisor.

and same typo here.

> 
> ACK, if those 2 points above are addressed

Pushed with this squashed in.

diff --git i/src/libvirt.c w/src/libvirt.c
index 54d7699..d455e1e 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -5400,15 +5400,14 @@ error:
  *
  * if ((virNodeGetMemoryStats(conn, cellNum, NULL, &nparams, 0) == 0) &&
  *     (nparams != 0)) {
- *     params = vshMalloc(ctl, sizeof(virMemoryStats) * nparams);
+ *     if ((params = malloc(sizeof(virMemoryStats) * nparams)) == NULL)
+ *         goto error;
  *     memset(params, cellNum, 0, sizeof(virMemoryStats) * nparams);
- *     if (virNodeGetMemoryStats(conn, params, &nparams, 0)) {
- *         vshError(ctl, "%s", _("Unable to get node memory stats"));
+ *     if (virNodeGetMemoryStats(conn, params, &nparams, 0))
  *         goto error;
- *     }
  * }
  *
- * This function doesn't requires privileged access to the hypervisor.
+ * This function doesn't require privileged access to the hypervisor.
  * This function expects the caller to allocate the @params.
  *
  * Memory Stats:

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]