[libvirt] [PATCH 06/16] util: Add virSysfsDirOpen

Martin Kletzander posted 16 patches 8 years, 10 months ago
[libvirt] [PATCH 06/16] util: Add virSysfsDirOpen
Posted by Martin Kletzander 8 years, 10 months ago
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/libvirt_private.syms |  1 +
 src/util/virsysfs.c      | 17 ++++++++++++++++-
 src/util/virsysfs.h      |  6 ++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9e61accda3fc..3965ef3f89c9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2621,6 +2621,7 @@ virVasprintfInternal;


 # util/virsysfs.h
+virSysfsDirOpen;
 virSysfsGetCpuValueBitmap;
 virSysfsGetCpuValueInt;
 virSysfsGetCpuValueString;
diff --git a/src/util/virsysfs.c b/src/util/virsysfs.c
index c482e188a301..6686d8ddbfcb 100644
--- a/src/util/virsysfs.c
+++ b/src/util/virsysfs.c
@@ -25,7 +25,6 @@
 #include "virsysfspriv.h"

 #include "viralloc.h"
-#include "virfile.h"
 #include "virlog.h"
 #include "virstring.h"

@@ -120,6 +119,22 @@ virSysfsGetValueBitmap(const char *file,
  * Per-CPU getters
  */
 int
+virSysfsDirOpen(const char *file,
+                DIR **dirp)
+{
+    char *path = NULL;
+
+    if (virAsprintf(&path, "%s/%s", sysfs_system_path, file) < 0)
+        return -1;
+
+    if (!virFileIsDir(path))
+        return -2;
+
+    return virDirOpen(dirp, path);
+}
+
+
+int
 virSysfsGetCpuValueInt(unsigned int cpu,
                        const char *file,
                        int *value)
diff --git a/src/util/virsysfs.h b/src/util/virsysfs.h
index 1b24fc193a16..ff5012d62747 100644
--- a/src/util/virsysfs.h
+++ b/src/util/virsysfs.h
@@ -23,6 +23,8 @@

 # include "internal.h"
 # include "virbitmap.h"
+# include "virfile.h"
+

 /*
  * Generic getters
@@ -41,6 +43,10 @@ int
 virSysfsGetValueBitmap(const char *file,
                        virBitmapPtr *value);

+int
+virSysfsDirOpen(const char *file,
+                DIR **dirp);
+

 /*
  * Per-CPU getters
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/16] util: Add virSysfsDirOpen
Posted by Eli Qiao 8 years, 10 months ago

On Thursday, 30 March 2017 at 10:03 PM, Martin Kletzander wrote:

> Signed-off-by: Martin Kletzander <mkletzan@redhat.com (mailto:mkletzan@redhat.com)>
> ---
> src/libvirt_private.syms | 1 +
> src/util/virsysfs.c | 17 ++++++++++++++++-
> src/util/virsysfs.h | 6 ++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9e61accda3fc..3965ef3f89c9 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2621,6 +2621,7 @@ virVasprintfInternal;
> 
> 
> # util/virsysfs.h
> +virSysfsDirOpen;
> virSysfsGetCpuValueBitmap;
> virSysfsGetCpuValueInt;
> virSysfsGetCpuValueString;
> diff --git a/src/util/virsysfs.c b/src/util/virsysfs.c
> index c482e188a301..6686d8ddbfcb 100644
> --- a/src/util/virsysfs.c
> +++ b/src/util/virsysfs.c
> @@ -25,7 +25,6 @@
> #include "virsysfspriv.h"
> 
> #include "viralloc.h"
> -#include "virfile.h"
> #include "virlog.h"
> #include "virstring.h"
> 
> @@ -120,6 +119,22 @@ virSysfsGetValueBitmap(const char *file,
> * Per-CPU getters
> */
> int
> +virSysfsDirOpen(const char *file,
> + DIR **dirp)
> +{
> + char *path = NULL;
> +
> + if (virAsprintf(&path, "%s/%s", sysfs_system_path, file) < 0)
> + return -1;
> +
> + if (!virFileIsDir(path))
> + return -2;
> +
> + return virDirOpen(dirp, path);
> +}
> +
> 
> 

what if I need another util function like:
 virSysfsDirResctrlOpen(const char *file, DIR **dirp)

do another copy and just modify path as:

if (virAsprintf(&path, "%s/%s", sysfs_resctrl_path, file) < 0)

is that okay for you to duplicated code like this?

This is not so common a way to adding utils functions.

If I need another `sysfs_system_path`, for /sys/fs/resctrl
I need to add a new variable sysfs_resctrl_path, then do a copy
of this function with some modification.

Eli.
> + DIR **dirp)

> +
> +int
> virSysfsGetCpuValueInt(unsigned int cpu,
> const char *file,
> int *value)
> diff --git a/src/util/virsysfs.h b/src/util/virsysfs.h
> index 1b24fc193a16..ff5012d62747 100644
> --- a/src/util/virsysfs.h
> +++ b/src/util/virsysfs.h
> @@ -23,6 +23,8 @@
> 
> # include "internal.h"
> # include "virbitmap.h"
> +# include "virfile.h"
> +
> 
> /*
> * Generic getters
> @@ -41,6 +43,10 @@ int
> virSysfsGetValueBitmap(const char *file,
> virBitmapPtr *value);
> 
> +int
> +virSysfsDirOpen(const char *file,
> + DIR **dirp);
> +
> 
> /*
> * Per-CPU getters
> -- 
> 2.12.2
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com (mailto:libvir-list@redhat.com)
> https://www.redhat.com/mailman/listinfo/libvir-list
> 
> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/16] util: Add virSysfsDirOpen
Posted by Martin Kletzander 8 years, 10 months ago
On Fri, Mar 31, 2017 at 09:32:06AM +0800, Eli Qiao wrote:
>
>
>On Thursday, 30 March 2017 at 10:03 PM, Martin Kletzander wrote:
>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com (mailto:mkletzan@redhat.com)>
>> ---
>> src/libvirt_private.syms | 1 +
>> src/util/virsysfs.c | 17 ++++++++++++++++-
>> src/util/virsysfs.h | 6 ++++++
>> 3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 9e61accda3fc..3965ef3f89c9 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -2621,6 +2621,7 @@ virVasprintfInternal;
>>
>>
>> # util/virsysfs.h
>> +virSysfsDirOpen;
>> virSysfsGetCpuValueBitmap;
>> virSysfsGetCpuValueInt;
>> virSysfsGetCpuValueString;
>> diff --git a/src/util/virsysfs.c b/src/util/virsysfs.c
>> index c482e188a301..6686d8ddbfcb 100644
>> --- a/src/util/virsysfs.c
>> +++ b/src/util/virsysfs.c
>> @@ -25,7 +25,6 @@
>> #include "virsysfspriv.h"
>>
>> #include "viralloc.h"
>> -#include "virfile.h"
>> #include "virlog.h"
>> #include "virstring.h"
>>
>> @@ -120,6 +119,22 @@ virSysfsGetValueBitmap(const char *file,
>> * Per-CPU getters
>> */
>> int
>> +virSysfsDirOpen(const char *file,
>> + DIR **dirp)
>> +{
>> + char *path = NULL;
>> +
>> + if (virAsprintf(&path, "%s/%s", sysfs_system_path, file) < 0)
>> + return -1;
>> +
>> + if (!virFileIsDir(path))
>> + return -2;
>> +
>> + return virDirOpen(dirp, path);
>> +}
>> +
>>
>>
>
>what if I need another util function like:
> virSysfsDirResctrlOpen(const char *file, DIR **dirp)
>
>do another copy and just modify path as:
>
>if (virAsprintf(&path, "%s/%s", sysfs_resctrl_path, file) < 0)
>
>is that okay for you to duplicated code like this?
>
>This is not so common a way to adding utils functions.
>
>If I need another `sysfs_system_path`, for /sys/fs/resctrl
>I need to add a new variable sysfs_resctrl_path, then do a copy
>of this function with some modification.
>

Yeah, well, it's not nice to duplicate that much stuff.  But for now I
can't think of any better way.  Anyway, Erik started the discussion in
another thread, so we can think of something there.

Do you have a better idea?

Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list