Re: [libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo
Joao Martins wrote: > On 02/08/2017 04:17 PM, Jim Fehlig wrote: >> Joao Martins wrote: >>> On 02/02/2017 10:39 PM, Jim Fehlig wrote: The typical pattern when calling libxl functions that populate a structure is libxl_foo foo; libxl_foo_init(&foo); libxl_get_foo(ctx, &foo); ... libxl_foo_dispose(&foo); Fix several instances of libxl_physinfo missing the init and dispose calls. >>> Indeed, >>> Signed-off-by: Jim Fehlig >>> Reviewed-by: Joao Martins >>> >>> See also one comment/nit below, perhaps one libxl_physinfo_init could be >>> moved >>> slightly up.. >>> [...] diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3a69720..8951bef 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4286,6 +4286,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn) if (virNodeGetFreeMemoryEnsureACL(conn) < 0) goto cleanup; +libxl_physinfo_init(&phy_info); >>> .. namely here? That is before virNodeGetFreeMemoryEnsureACL. >> Nice catch. Moved as suggested in my local branch. >> >> Any other comments on this small series? Would be nice to get these bug fixes >> committed :-). >> > Nope, looks all good to me: > > Acked-by: Joao Martins Thanks, I've pushed these fixes now. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo
On 02/08/2017 04:17 PM, Jim Fehlig wrote:
> Joao Martins wrote:
>> On 02/02/2017 10:39 PM, Jim Fehlig wrote:
>>> The typical pattern when calling libxl functions that populate a
>>> structure is
>>>
>>> libxl_foo foo;
>>> libxl_foo_init(&foo);
>>> libxl_get_foo(ctx, &foo);
>>> ...
>>> libxl_foo_dispose(&foo);
>>>
>>> Fix several instances of libxl_physinfo missing the init and
>>> dispose calls.
>> Indeed,
>>
>>> Signed-off-by: Jim Fehlig
>>
>> Reviewed-by: Joao Martins
>>
>> See also one comment/nit below, perhaps one libxl_physinfo_init could be
>> moved
>> slightly up..
>>
>>> [...]
>>
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index 3a69720..8951bef 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -4286,6 +4286,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
>>> if (virNodeGetFreeMemoryEnsureACL(conn) < 0)
>>> goto cleanup;
>>>
>>> +libxl_physinfo_init(&phy_info);
>>
>> .. namely here? That is before virNodeGetFreeMemoryEnsureACL.
>
> Nice catch. Moved as suggested in my local branch.
>
> Any other comments on this small series? Would be nice to get these bug fixes
> committed :-).
>
Nope, looks all good to me:
Acked-by: Joao Martins
> Regards,
> Jim
>
>>
>> Not that it matters much, as init/dispose in this case just zeroes out
>> phy_info
>> region. But just perhaps consistency as you would end disposing an non
>> initialized object.
>>
>>> if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
>>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> _("libxl_get_physinfo_info failed"));
>>> @@ -4295,6 +4296,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
>>> ret = phy_info.free_pages * cfg->verInfo->pagesize;
>>>
>>> cleanup:
>>> +libxl_physinfo_dispose(&phy_info);
>>> virObjectUnref(cfg);
>>> return ret;
>>> }
>>>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo
Joao Martins wrote:
> On 02/02/2017 10:39 PM, Jim Fehlig wrote:
>> The typical pattern when calling libxl functions that populate a
>> structure is
>>
>> libxl_foo foo;
>> libxl_foo_init(&foo);
>> libxl_get_foo(ctx, &foo);
>> ...
>> libxl_foo_dispose(&foo);
>>
>> Fix several instances of libxl_physinfo missing the init and
>> dispose calls.
> Indeed,
>
>> Signed-off-by: Jim Fehlig
>
> Reviewed-by: Joao Martins
>
> See also one comment/nit below, perhaps one libxl_physinfo_init could be moved
> slightly up..
>
>> [...]
>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 3a69720..8951bef 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -4286,6 +4286,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
>> if (virNodeGetFreeMemoryEnsureACL(conn) < 0)
>> goto cleanup;
>>
>> +libxl_physinfo_init(&phy_info);
>
> .. namely here? That is before virNodeGetFreeMemoryEnsureACL.
Nice catch. Moved as suggested in my local branch.
Any other comments on this small series? Would be nice to get these bug fixes
committed :-).
Regards,
Jim
>
> Not that it matters much, as init/dispose in this case just zeroes out
> phy_info
> region. But just perhaps consistency as you would end disposing an non
> initialized object.
>
>> if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("libxl_get_physinfo_info failed"));
>> @@ -4295,6 +4296,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
>> ret = phy_info.free_pages * cfg->verInfo->pagesize;
>>
>> cleanup:
>> +libxl_physinfo_dispose(&phy_info);
>> virObjectUnref(cfg);
>> return ret;
>> }
>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo
On 02/02/2017 10:39 PM, Jim Fehlig wrote:
> The typical pattern when calling libxl functions that populate a
> structure is
>
> libxl_foo foo;
> libxl_foo_init(&foo);
> libxl_get_foo(ctx, &foo);
> ...
> libxl_foo_dispose(&foo);
>
> Fix several instances of libxl_physinfo missing the init and
> dispose calls.
Indeed,
>
> Signed-off-by: Jim Fehlig
Reviewed-by: Joao Martins
See also one comment/nit below, perhaps one libxl_physinfo_init could be moved
slightly up..
> [...]
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 3a69720..8951bef 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -4286,6 +4286,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
> if (virNodeGetFreeMemoryEnsureACL(conn) < 0)
> goto cleanup;
>
> +libxl_physinfo_init(&phy_info);
.. namely here? That is before virNodeGetFreeMemoryEnsureACL.
Not that it matters much, as init/dispose in this case just zeroes out phy_info
region. But just perhaps consistency as you would end disposing an non
initialized object.
> if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("libxl_get_physinfo_info failed"));
> @@ -4295,6 +4296,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
> ret = phy_info.free_pages * cfg->verInfo->pagesize;
>
> cleanup:
> +libxl_physinfo_dispose(&phy_info);
> virObjectUnref(cfg);
> return ret;
> }
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo
The typical pattern when calling libxl functions that populate a
structure is
libxl_foo foo;
libxl_foo_init(&foo);
libxl_get_foo(ctx, &foo);
...
libxl_foo_dispose(&foo);
Fix several instances of libxl_physinfo missing the init and
dispose calls.
Signed-off-by: Jim Fehlig
---
src/libxl/libxl_capabilities.c | 16 +++-
src/libxl/libxl_conf.c | 2 ++
src/libxl/libxl_driver.c | 2 ++
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index a1c0a87..2bbd2d1 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -211,27 +211,33 @@ libxlCapsInitHost(libxl_ctx *ctx, virCapsPtr caps)
const libxl_version_info *ver_info;
enum libxlHwcapVersion version;
libxl_physinfo phy_info;
+int ret = -1;
+libxl_physinfo_init(&phy_info);
if (libxl_get_physinfo(ctx, &phy_info) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to get node physical info from libxenlight"));
-return -1;
+goto cleanup;
}
if ((ver_info = libxl_get_version_info(ctx)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to get version info from libxenlight"));
-return -1;
+goto cleanup;
}
version = (ver_info->xen_version_minor >= 7);
if (libxlCapsInitCPU(caps, &phy_info, version) < 0)
-return -1;
+goto cleanup;
if (virCapabilitiesSetNetPrefix(caps, LIBXL_GENERATED_PREFIX_XEN) < 0)
-return -1;
+goto cleanup;
-return 0;
+ret = 0;
+
+ cleanup:
+libxl_physinfo_dispose(&phy_info);
+return ret;
}
static int
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index ee1e639..28b05e1 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1943,6 +1943,7 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver,
virNodeInfoPtr info)
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
int ret = -1;
+libxl_physinfo_init(&phy_info);
if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("libxl_get_physinfo_info failed"));
@@ -1967,6 +1968,7 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver,
virNodeInfoPtr info)
ret = 0;
cleanup:
+libxl_physinfo_dispose(&phy_info);
virObjectUnref(cfg);
return ret;
}
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3a69720..8951bef 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4286,6 +4286,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
if (virNodeGetFreeMemoryEnsureACL(conn) < 0)
goto cleanup;
+libxl_physinfo_init(&phy_info);
if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("libxl_get_physinfo_info failed"));
@@ -4295,6 +4296,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
ret = phy_info.free_pages * cfg->verInfo->pagesize;
cleanup:
+libxl_physinfo_dispose(&phy_info);
virObjectUnref(cfg);
return ret;
}
--
2.9.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
