+# util/virresctrl.h
+virResCtrlAvailable;
+virResCtrlInit;
+

This has nothing to do in the patch

Okay, this should be involved by mistake while rebasing.

# util/virrotatingfile.h
virRotatingFileReaderConsume;
virRotatingFileReaderFree;
@@ -2626,13 +2630,18 @@ virSysfsGetCpuValueString;
virSysfsGetCpuValueUint;
virSysfsGetNodeValueBitmap;
virSysfsGetNodeValueString;
+virSysfsGetResctrlInfoString;
+virSysfsGetResctrlInfoUint;
+virSysfsGetResctrlPath;
+virSysfsGetResctrlString;
+virSysfsGetResctrlUint;
virSysfsGetSystemPath;
virSysfsGetValueBitmap;
virSysfsGetValueInt;
virSysfsGetValueString;
+virSysfsSetResctrlPath;
virSysfsSetSystemPath;

-

Don't remove the line. Every time you are doing a change in the code

(and I mean any code), try being consistent. If the code follows some

style, don't fight it, but go with it. Not everything can be written in

the coding style. As you can see here, all files are separated by two

lines. When you remove this, you make the file inconsistent.

Also, I mentioned many times before that you should run make check and

make syntax-check between patches, and I'm sure our contribution

guidelines mention that make check must pass between patches. This

change should not pass the checks. It's fine every now and then, we all

forget, but I'm trying to run make check and make syntax-check after

each patch before sending it. Useful command to use from your topic

branch is something along the lines of:

git rebase -ix 'make -j9 check && make -j9 syntax-check' master

(written by hand from memory, there might be typo, I have a script and

bunch of aliases for that)

Sure, Daniel had reminded me before, I missed it this time, will keep in mind next path.

# util/virsysinfo.h
virSysinfoBaseBoardDefClear;
virSysinfoBIOSDefFree;
diff --git a/src/util/virsysfs.c b/src/util/virsysfs.c
index 7a98b48..97808be 100644
--- a/src/util/virsysfs.c
+++ b/src/util/virsysfs.c

[...]

@@ -227,3 +242,88 @@ virSysfsGetNodeValueBitmap(unsigned int node,
VIR_FREE(path);
return ret;
}
+
+int
+virSysfsGetResctrlString(const char* file,

'char *file' instead of 'char* file'

+ char **value)
+{
+ char *path = NULL;
+ int ret = -1;
+ if (virAsprintf(&path, "%s/%s", sysfs_resctrl_path, file) < 0)
+ return -1;
+
+ if (!virFileExists(path)) {
+ ret = -2;
+ goto cleanup;
+ }
+

Now the question is; is it possible for some file to be missing there?

I mean some file we'd expect? For the system path, the -2 return value

is there because we need to work on older systems with older kernels and

the files were being added in multiple releases. If there is no need

for distinguishing that, then there is no point in explicitly checking

for the file to be existing.

No, we won’t expect file doesn’t existed. I will remove this checking

And that leads me to another point. Since this patch is by itself, it's

not visible how it is used. It looks good (not considering the things

pointed out above), but there is no point in merging it until there is a

value added. It's just added dead code. But it's something that we'll

use, surely.

Okay, I can add this patch to CAT supporting patch set as the first patch.

Martin

P.S.: I just sent [1] the next couple of patches (again, first ones are

just fixing some bullocks I found out that were left in the code

-- that just happens when you're working on a codebase with lots

of legacy code), but it adds host cache information to the

capabilities. If you have a minute or two, feel free to check it

out and let me know what you think.

Sure, I see you have added fake cache id, that’s good, I can drop it.

I have one propose, not sure it’s a good or bad one:

I would like to suggest that let the developer keep focus on adding features,

then refactor some uitls functions later, developers won’t have all knowledge

for what’s the function should be go into which utils, and this will bring time

wasting on keeping rebasing rebasing ……