* [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager
@ 2026-05-28 19:04 Sudheendra Sampath
2026-05-28 21:10 ` Stephen Hemminger
2026-05-29 8:01 ` Bruce Richardson
0 siblings, 2 replies; 5+ messages in thread
From: Sudheendra Sampath @ 2026-05-28 19:04 UTC (permalink / raw)
To: dev; +Cc: Sudheendra Sampath, Anatoly Burakov, Sivaprasad Tummala
This patch for bug 1832 will do the following:
1. If /run/dpdk is not present, it will create it first with and
then create powermanager directory underneath it.
2. If /run/dpdk is present, it will verify it is actually a directory
before creating subdirectory, powermanager.
All directory permissions are 0700.
Signed-off-by: Sudheendra Sampath <giveback4fun@gmail.com>
---
examples/vm_power_manager/channel_manager.c | 27 ++++++++++++++++++++-
examples/vm_power_manager/channel_manager.h | 2 +-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index b69449c61d..60d767ea98 100644
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -420,8 +420,33 @@ add_all_channels(const char *vm_name)
if (d == NULL) {
RTE_LOG(ERR, CHANNEL_MANAGER, "Error opening directory '%s': %s\n",
CHANNEL_MGR_SOCKET_PATH, strerror(errno));
- return -1;
+
+ const char *run_dpdk = "/run/dpdk";
+ struct stat path_stat;
+ int ret;
+
+ if (stat(run_dpdk, &path_stat) != 0) {
+ ret = mkdir(run_dpdk, 0700);
+ if (ret < 0 && errno != EEXIST) {
+ RTE_LOG(ERR, CHANNEL_MANAGER, "Error creating '%s': %s",
+ run_dpdk, strerror(errno));
+ return -1;
+ }
+ }
+
+ /* Make sure /run/dpdk is a directory */
+ if (!S_ISDIR(path_stat.st_mode)) {
+ return -1;
+ }
+
+ ret = mkdir(CHANNEL_MGR_SOCKET_PATH, 0700);
+ if (ret < 0 && errno != EEXIST) {
+ RTE_LOG(ERR, CHANNEL_MANAGER, "Error creating '%s': %s",
+ CHANNEL_MGR_SOCKET_PATH, strerror(errno));
+ return -1;
+ }
}
+
while ((dir = readdir(d)) != NULL) {
if (!strncmp(dir->d_name, ".", 1) ||
!strncmp(dir->d_name, "..", 2))
diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index 6f70539815..5fc93ae0be 100644
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -22,7 +22,7 @@ extern "C" {
#define CHANNEL_MGR_DEFAULT_HV_PATH "qemu:///system"
/* File socket directory */
-#define CHANNEL_MGR_SOCKET_PATH "/tmp/powermonitor/"
+#define CHANNEL_MGR_SOCKET_PATH "/run/dpdk/powermonitor/"
/* FIFO file name template */
#define CHANNEL_MGR_FIFO_PATTERN_NAME "fifo"
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager
2026-05-28 19:04 [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager Sudheendra Sampath
@ 2026-05-28 21:10 ` Stephen Hemminger
2026-05-29 8:01 ` Bruce Richardson
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-05-28 21:10 UTC (permalink / raw)
To: Sudheendra Sampath; +Cc: dev, Anatoly Burakov, Sivaprasad Tummala
On Thu, 28 May 2026 19:04:48 +0000
Sudheendra Sampath <giveback4fun@gmail.com> wrote:
> This patch for bug 1832 will do the following:
> 1. If /run/dpdk is not present, it will create it first with and
> then create powermanager directory underneath it.
> 2. If /run/dpdk is present, it will verify it is actually a directory
> before creating subdirectory, powermanager.
>
> All directory permissions are 0700.
>
> Signed-off-by: Sudheendra Sampath <giveback4fun@gmail.com>
> ---
Lots of issues with hardcoding the path here.
Longer explanation from AI review.
Thanks for the patch. A few issues to address before this can be merged:
Two correctness bugs in the new recovery block:
- After creating the directories, d is still NULL but the code falls through to readdir(d). The recovery path must call opendir() again (and return -1 if that also fails) before the readdir loop.
- path_stat is only populated when stat() succeeds. The S_ISDIR(path_stat.st_mode) check runs on uninitialised memory when stat() returned an error and mkdir() was used. Restructure as if (stat(...) == 0) { check S_ISDIR } else { mkdir }.
Other items:
- doc/guides/sample_app_ug/vm_power_management.rst still references /tmp/powermonitor/ in several places; code and docs need to change together.
- Hardcoding /run/dpdk does not follow the DPDK runtime-dir convention and breaks non-root usage.
Please consider using rte_eal_get_runtime_dir() (or at least the same getuid() / RUNTIME_DIRECTORY logic as eal_create_runtime_dir()) instead.
- Subject says "powermanager" but the path is "powermonitor".
- The two new RTE_LOG messages are missing trailing "\n".
- Please add a proper Fixes: tag (12-char hash) for the commit that introduced /tmp/powermonitor.
Stephen
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager
2026-05-28 19:04 [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager Sudheendra Sampath
2026-05-28 21:10 ` Stephen Hemminger
@ 2026-05-29 8:01 ` Bruce Richardson
2026-05-29 15:23 ` Stephen Hemminger
1 sibling, 1 reply; 5+ messages in thread
From: Bruce Richardson @ 2026-05-29 8:01 UTC (permalink / raw)
To: Sudheendra Sampath; +Cc: dev, Anatoly Burakov, Sivaprasad Tummala
On Thu, May 28, 2026 at 07:04:48PM +0000, Sudheendra Sampath wrote:
> This patch for bug 1832 will do the following:
> 1. If /run/dpdk is not present, it will create it first with and
> then create powermanager directory underneath it.
> 2. If /run/dpdk is present, it will verify it is actually a directory
> before creating subdirectory, powermanager.
>
I would suggest using $XDG_RUNTIME_DIR for the directory path, rather than
hardcoding it by default. If XDG_RUNTIME_DIR is not set, then maybe
consider using /run/dpdk. However, rather than /run/dpdk, I'd suggest using
the normal runtime dir path on most distros as the default:
/run/user/<uid>.
/Bruce
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager
2026-05-29 8:01 ` Bruce Richardson
@ 2026-05-29 15:23 ` Stephen Hemminger
2026-05-29 16:01 ` Bruce Richardson
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2026-05-29 15:23 UTC (permalink / raw)
To: Bruce Richardson
Cc: Sudheendra Sampath, dev, Anatoly Burakov, Sivaprasad Tummala
On Fri, 29 May 2026 09:01:34 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:
> On Thu, May 28, 2026 at 07:04:48PM +0000, Sudheendra Sampath wrote:
> > This patch for bug 1832 will do the following:
> > 1. If /run/dpdk is not present, it will create it first with and
> > then create powermanager directory underneath it.
> > 2. If /run/dpdk is present, it will verify it is actually a directory
> > before creating subdirectory, powermanager.
> >
> I would suggest using $XDG_RUNTIME_DIR for the directory path, rather than
> hardcoding it by default. If XDG_RUNTIME_DIR is not set, then maybe
> consider using /run/dpdk. However, rather than /run/dpdk, I'd suggest using
> the normal runtime dir path on most distros as the default:
> /run/user/<uid>.
>
> /Bruce
The login in EAL is a little more detailed.
The choice is from systemd conventions which follows filesystem hierarchy.
int eal_create_runtime_dir(void)
{
const char *directory;
char run_dir[PATH_MAX];
char tmp[PATH_MAX];
int ret;
/* from RuntimeDirectory= see systemd.exec */
directory = getenv("RUNTIME_DIRECTORY");
if (directory == NULL) {
/*
* Used standard convention defined in
* XDG Base Directory Specification and
* Filesystem Hierarchy Standard.
*/
if (getuid() == 0)
directory = "/var/run";
else
directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
}
/* create DPDK subdirectory under runtime dir */
ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
if (ret < 0 || ret == sizeof(tmp)) {
EAL_LOG(ERR, "Error creating DPDK runtime path name");
return -1;
}
/* create prefix-specific subdirectory under DPDK runtime dir */
ret = snprintf(run_dir, sizeof(run_dir), "%s/%s",
tmp, eal_get_hugefile_prefix());
if (ret < 0 || ret == sizeof(run_dir)) {
EAL_LOG(ERR, "Error creating prefix-specific runtime path name");
return -1;
}
/* create the path if it doesn't exist. no "mkdir -p" here, so do it
* step by step.
*/
ret = mkdir(tmp, 0700);
if (ret < 0 && errno != EEXIST) {
EAL_LOG(ERR, "Error creating '%s': %s",
tmp, strerror(errno));
return -1;
}
ret = mkdir(run_dir, 0700);
if (ret < 0 && errno != EEXIST) {
EAL_LOG(ERR, "Error creating '%s': %s",
run_dir, strerror(errno));
return -1;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager
2026-05-29 15:23 ` Stephen Hemminger
@ 2026-05-29 16:01 ` Bruce Richardson
0 siblings, 0 replies; 5+ messages in thread
From: Bruce Richardson @ 2026-05-29 16:01 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Sudheendra Sampath, dev, Anatoly Burakov, Sivaprasad Tummala
On Fri, May 29, 2026 at 08:23:24AM -0700, Stephen Hemminger wrote:
> On Fri, 29 May 2026 09:01:34 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > On Thu, May 28, 2026 at 07:04:48PM +0000, Sudheendra Sampath wrote:
> > > This patch for bug 1832 will do the following:
> > > 1. If /run/dpdk is not present, it will create it first with and
> > > then create powermanager directory underneath it.
> > > 2. If /run/dpdk is present, it will verify it is actually a directory
> > > before creating subdirectory, powermanager.
> > >
> > I would suggest using $XDG_RUNTIME_DIR for the directory path, rather than
> > hardcoding it by default. If XDG_RUNTIME_DIR is not set, then maybe
> > consider using /run/dpdk. However, rather than /run/dpdk, I'd suggest using
> > the normal runtime dir path on most distros as the default:
> > /run/user/<uid>.
> >
> > /Bruce
>
> The login in EAL is a little more detailed.
> The choice is from systemd conventions which follows filesystem hierarchy.
>
>
> int eal_create_runtime_dir(void)
> {
> const char *directory;
> char run_dir[PATH_MAX];
> char tmp[PATH_MAX];
> int ret;
>
> /* from RuntimeDirectory= see systemd.exec */
> directory = getenv("RUNTIME_DIRECTORY");
> if (directory == NULL) {
> /*
> * Used standard convention defined in
> * XDG Base Directory Specification and
> * Filesystem Hierarchy Standard.
> */
> if (getuid() == 0)
> directory = "/var/run";
> else
> directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
> }
>
> /* create DPDK subdirectory under runtime dir */
> ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
> if (ret < 0 || ret == sizeof(tmp)) {
> EAL_LOG(ERR, "Error creating DPDK runtime path name");
> return -1;
> }
>
> /* create prefix-specific subdirectory under DPDK runtime dir */
> ret = snprintf(run_dir, sizeof(run_dir), "%s/%s",
> tmp, eal_get_hugefile_prefix());
> if (ret < 0 || ret == sizeof(run_dir)) {
> EAL_LOG(ERR, "Error creating prefix-specific runtime path name");
> return -1;
> }
>
> /* create the path if it doesn't exist. no "mkdir -p" here, so do it
> * step by step.
> */
> ret = mkdir(tmp, 0700);
> if (ret < 0 && errno != EEXIST) {
> EAL_LOG(ERR, "Error creating '%s': %s",
> tmp, strerror(errno));
> return -1;
> }
>
> ret = mkdir(run_dir, 0700);
> if (ret < 0 && errno != EEXIST) {
> EAL_LOG(ERR, "Error creating '%s': %s",
> run_dir, strerror(errno));
> return -1;
> }
Yes. Can the power manager call the rte_eal_get_runtime_dir() API and use
that as a basis for its working directory? Save duplicating all this logic.
/Bruce
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-29 16:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 19:04 [PATCH] examples: Fix vm_power_manager scratch area to /run/dpdk/powermanager Sudheendra Sampath
2026-05-28 21:10 ` Stephen Hemminger
2026-05-29 8:01 ` Bruce Richardson
2026-05-29 15:23 ` Stephen Hemminger
2026-05-29 16:01 ` Bruce Richardson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox