On 01/31/2018 10:41 AM, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> src/util/virresctrl.c | 79 +++++++++++++++++++++++++++++++++------------------
> 1 file changed, 52 insertions(+), 27 deletions(-)
>
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 42876c6e2b9b..fa7c28e9b45e 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -1041,6 +1041,53 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl,
> }
>
>
> +static int
> +virResctrlAllocGetGroup(virResctrlInfoPtr resctrl,
> + const char *groupname,
> + virResctrlAllocPtr *alloc)
> +{
> + char *schemata = NULL;
> + int rv = virFileReadValueString(&schemata,
> + SYSFS_RESCTRL_PATH
> + "/%s/schemata",
> + groupname);
> +
> + if (rv < 0)
> + return rv;
I guess we need to set *alloc = NULL before returning. Because with this
patch the function may or may not do that when returning an error. But
it's just cosmetics.
> +
> + *alloc = virResctrlAllocNew();
> + if (!*alloc)
> + goto error;
> +
> + if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0)
> + goto error;
> +
> + VIR_FREE(schemata);
> + return 0;
> +
> + error:
> + VIR_FREE(schemata);
> + virObjectUnref(*alloc);
> + *alloc = NULL;
> + return -1;
> +}
> +
> +
> +static virResctrlAllocPtr
> +virResctrlAllocGetDefault(virResctrlInfoPtr resctrl)
> +{
> + virResctrlAllocPtr ret = NULL;
> +
> + if (virResctrlAllocGetGroup(resctrl, ".", &ret) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not read schemata file for the default group"));
> + return NULL;
No need to report an error. virResctrlAllocGetGroup() calls functions
which report error on failure. But this is more user friendly error
message than 'unable to open /sys/blah: Bad address'. Your call.
> + }
> +
> + return ret;
> +}
> +
> +
> static void
> virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr dst,
> virResctrlAllocPerTypePtr src)
> @@ -1141,7 +1188,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
> virResctrlAllocPtr alloc = NULL;
> struct dirent *ent = NULL;
> DIR *dirp = NULL;
> - char *schemata = NULL;
> int rv = -1;
>
> if (virResctrlInfoIsEmpty(resctrl)) {
> @@ -1154,22 +1200,12 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
> if (!ret)
> return NULL;
>
> - if (virFileReadValueString(&schemata,
> - SYSFS_RESCTRL_PATH
> - "/schemata") < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Could not read schemata file for the default group"));
> - goto error;
> - }
> -
> - alloc = virResctrlAllocNew();
> + alloc = virResctrlAllocGetDefault(resctrl);
> if (!alloc)
> goto error;
>
> - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0)
> - goto error;
> -
> virResctrlAllocSubtract(ret, alloc);
> + virObjectUnref(alloc);
>
> if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0)
> goto error;
> @@ -1178,11 +1214,7 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
> if (STREQ(ent->d_name, "info"))
> continue;
>
> - VIR_FREE(schemata);
> - rv = virFileReadValueString(&schemata,
> - SYSFS_RESCTRL_PATH
> - "/%s/schemata",
> - ent->d_name);
> + rv = virResctrlAllocGetGroup(resctrl, ent->d_name, &alloc);
> if (rv == -2)
> continue;
Unfortunately, not visible in the context but the same 'error message
overwrite' problem.
>
> @@ -1193,15 +1225,9 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
> goto error;
> }
>
> - virObjectUnref(alloc);
> - alloc = virResctrlAllocNew();
> - if (!alloc)
> - goto error;
> -
> - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0)
> - goto error;
> -
> virResctrlAllocSubtract(ret, alloc);
> + virObjectUnref(alloc);
> + alloc = NULL;
> }
> if (rv < 0)
> goto error;
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
