Re: [libvirt PATCH 1/3] vircgroup: introduce virCgroupGetInode function
On 8/4/21 10:22 AM, Pavel Hrdina wrote:
On Wed, Aug 04, 2021 at 10:06:17AM +0200, Michal Prívozník wrote:
> On 8/3/21 4:29 PM, Pavel Hrdina wrote:
>> For new feature Fibre Channel VMID we will need to get inode of the
>> VM root cgroup as it is used in the new kernel API together with VMID.
>>
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/libvirt_private.syms | 1 +
>> src/util/vircgroup.c | 37 +++++++++++++++++++++++++++++++++++++
>> src/util/vircgroup.h | 2 ++
>> 3 files changed, 40 insertions(+)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 6961cdb137..d8451fcfff 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1920,6 +1920,7 @@ virCgroupGetCpuShares;
>> virCgroupGetDevicePermsString;
>> virCgroupGetDomainTotalCpuStats;
>> virCgroupGetFreezerState;
>> +virCgroupGetInode;
>> virCgroupGetMemoryHardLimit;
>> virCgroupGetMemorySoftLimit;
>> virCgroupGetMemoryStat;
>> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
>> index 1b3b28342e..ea702e7b63 100644
>> --- a/src/util/vircgroup.c
>> +++ b/src/util/vircgroup.c
>> @@ -3973,3 +3973,40 @@ virCgroupGetCpuPeriodQuota(virCgroup *cgroup, unsigned
long long *period,
>>
>> return 0;
>> }
>> +
>> +
>> +/**
>> + * virCgroupGetInode:
>> + *
>> + * @cgroup: the cgroup to get inode for
>> + *
>> + * Get the @cgroup inode and return its value to the caller.
>> + *
>> + * Returns inode on success, -1 on error with error message reported.
>> + */
>> +int
>> +virCgroupGetInode(virCgroup *cgroup)
>> +{
>> + VIR_AUTOCLOSE fd = 0;
>> + struct stat st;
>> + int controller = virCgroupGetAnyController(cgroup);
>> + g_autofree char *path = NULL;
>> +
>> + if (controller < 0)
>> + return -1;
>> +
>> + if (virCgroupPathOfController(cgroup, controller, "", &path)
< 0)
>> + return -1;
>> +
>> + if ((fd = open(path, O_RDONLY | O_NONBLOCK)) < 0) {
>> + virReportSystemError(errno, _("failed to open cgroup
'%s'"), path);
>> + return -1;
>> + }
>
> Is the open() necessary? Why isn't plain stat() enough?
Good question :) I was lazy so I looked for first similar code in
libvirt codebase and copied it. You are right, stat should be enough
as according to man page stat and fstat are identical except or the
fd vs path parameter. I'll fix it before pushing or should I send a v2?
No need, just drop open() and switch to stat() before pushing. My
Reviewed-by still applies.
Michal