On Fri, Mar 26, 2021 at 10:38:30PM +0100, Klemens Nanni wrote:
> apm(8) never knows gets the result of the requested power action carried
> out by apmd(8), so platforms without suspend/resume support behave like
> this:
>
> $ zzz; echo $?
> Suspending system...
> 0
> $ apm -z; echo $?
> System will enter suspend mode momentarily.
> 0
>
> Prior to the latest apmd commit there wasn't even a syslog message
> reporting any failure whatsoever.
>
>
> This diff adds an extra `int error' field to `struct apm_reply' which
> such that apmd has means to tell apm if something went in simple
> errno(2) fashion.
>
> To do so I need to hoist the power action inside handle_client(),
> obviously before the reply it sent back to apm -- currently apmd replies
> *before* carrying out the requested power actions.
>
>
> On arm64 it looks like this then:
>
> $ zzz; echo $?
> Suspending system...
> zzz: suspend: Operation not supported
> 1
> $ apm -z; echo $?
> System will enter suspend mode momentarily.
> apm: suspend: Operation not supported
> 1
>
> amd64 notebooks for example keep suspending without error messages and
> exit zero as before, i.e. expected behaviour stays unchanged for
> platforms that already work.
>
> Feedback? OK?
Correct diff without fat fingered sleep(3) this time.
Index: usr.sbin/apm/apm.c
===================================================================
RCS file: /cvs/src/usr.sbin/apm/apm.c,v
retrieving revision 1.37
diff -u -p -r1.37 apm.c
--- usr.sbin/apm/apm.c 23 Sep 2020 05:50:26 -0000 1.37
+++ usr.sbin/apm/apm.c 26 Mar 2021 21:48:36 -0000
@@ -97,6 +97,7 @@ do_zzz(int fd, enum apm_action action)
struct apm_command command;
struct apm_reply reply;
char *msg;
+ int ret;
switch (action) {
case NONE:
@@ -117,7 +118,10 @@ do_zzz(int fd, enum apm_action action)
}
printf("%s...\n", msg);
- exit(send_command(fd, &command, &reply));
+ ret = send_command(fd, &command, &reply);
+ if (reply.error)
+ errx(1, "%s: %s", apm_state(reply.newstate),
strerror(reply.error));
+ exit(ret);
}
static int
@@ -418,5 +422,7 @@ balony:
default:
break;
}
+ if (reply.error)
+ errx(1, "%s: %s", apm_state(reply.newstate),
strerror(reply.error));
return (0);
}
Index: usr.sbin/apmd/apm-proto.h
===================================================================
RCS file: /cvs/src/usr.sbin/apmd/apm-proto.h,v
retrieving revision 1.10
diff -u -p -r1.10 apm-proto.h
--- usr.sbin/apmd/apm-proto.h 23 Sep 2020 05:50:26 -0000 1.10
+++ usr.sbin/apmd/apm-proto.h 26 Mar 2021 21:48:36 -0000
@@ -64,6 +64,7 @@ struct apm_reply {
enum apm_perfmode perfmode;
int cpuspeed;
struct apm_power_info batterystate;
+ int error;
};
#define APMD_VNO 3
@@ -71,3 +72,4 @@ struct apm_reply {
extern const char *battstate(int state);
extern const char *ac_state(int state);
extern const char *perf_mode(int mode);
+extern const char *apm_state(int apm_state);
Index: usr.sbin/apmd/apmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.102
diff -u -p -r1.102 apmd.c
--- usr.sbin/apmd/apmd.c 25 Mar 2021 20:46:55 -0000 1.102
+++ usr.sbin/apmd/apmd.c 26 Mar 2021 21:48:37 -0000
@@ -65,9 +65,9 @@ void usage(void);
int power_status(int fd, int force, struct apm_power_info *pinfo);
int bind_socket(const char *sn);
enum apm_state handle_client(int sock_fd, int ctl_fd);
-void suspend(int ctl_fd);
-void stand_by(int ctl_fd);
-void hibernate(int ctl_fd);
+int suspend(int ctl_fd);
+int stand_by(int ctl_fd);
+int hibernate(int ctl_fd);
void resumed(int ctl_fd);
void setperfpolicy(char *policy);
void sigexit(int signo);
@@ -266,16 +266,23 @@ handle_client(int sock_fd, int ctl_fd)
return NORMAL;
}
+ reply.error = 0;
power_status(ctl_fd, 0, &reply.batterystate);
switch (cmd.action) {
case SUSPEND:
reply.newstate = SUSPENDING;
+ if (suspend(ctl_fd) == -1)
+ reply.error = errno;
break;
case STANDBY:
reply.newstate = STANDING_BY;
+ if (stand_by(ctl_fd) == -1)
+ reply.error = errno;
break;
case HIBERNATE:
reply.newstate = HIBERNATING;
+ if (hibernate(ctl_fd) == -1)
+ reply.error = errno;
break;
case SETPERF_LOW:
reply.newstate = NORMAL;
@@ -321,40 +328,49 @@ handle_client(int sock_fd, int ctl_fd)
return reply.newstate;
}
-void
+int
suspend(int ctl_fd)
{
+ int ret;
+
logmsg(LOG_NOTICE, "system suspending");
power_status(ctl_fd, 1, NULL);
do_etc_file(_PATH_APM_ETC_SUSPEND);
sync();
sleep(1);
- if (ioctl(ctl_fd, APM_IOC_SUSPEND, 0) == -1)
+ if ((ret = ioctl(ctl_fd, APM_IOC_SUSPEND, 0)) == -1)
logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
+ return (ret);
}
-void
+int
stand_by(int ctl_fd)
{
+ int ret;
+
logmsg(LOG_NOTICE, "system entering standby");
power_status(ctl_fd, 1, NULL);
do_etc_file(_PATH_APM_ETC_STANDBY);
sync();
sleep(1);
- if (ioctl(ctl_fd, APM_IOC_STANDBY, 0) == -1)
+ if ((ret = ioctl(ctl_fd, APM_IOC_STANDBY, 0)) == -1)
logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
+ return (ret);
}
-void
+int
hibernate(int ctl_fd)
{
+ int ret;
+
logmsg(LOG_NOTICE, "system hibernating");
power_status(ctl_fd, 1, NULL);
do_etc_file(_PATH_APM_ETC_HIBERNATE);
sync();
sleep(1);
- if (ioctl(ctl_fd, APM_IOC_HIBERNATE, 0) == -1)
+ if ((ret = ioctl(ctl_fd, APM_IOC_HIBERNATE, 0)) == -1)
logmsg(LOG_WARNING, "%s: %s", __func__, strerror(errno));
+ return (ret);
}
void
@@ -512,20 +528,12 @@ main(int argc, char *argv[])
break;
if (rv == 1 && ev->ident == sock_fd) {
- switch (handle_client(sock_fd, ctl_fd)) {
- case NORMAL:
- break;
- case SUSPENDING:
- suspend(ctl_fd);
- break;
- case STANDING_BY:
- stand_by(ctl_fd);
- break;
- case HIBERNATING:
- hibernate(ctl_fd);
- break;
- }
- continue;
+ int state;
+
+ if ((state = handle_client(sock_fd, ctl_fd)) == -1)
+ logmsg(LOG_WARNING, "%s: %s", apm_state(state),
strerror(errno));
+ else
+ continue;
}
suspends = standbys = hibernates = resumes = 0;
Index: usr.sbin/apmd/apmsubr.c
===================================================================
RCS file: /cvs/src/usr.sbin/apmd/apmsubr.c,v
retrieving revision 1.9
diff -u -p -r1.9 apmsubr.c
--- usr.sbin/apmd/apmsubr.c 23 Sep 2020 05:50:26 -0000 1.9
+++ usr.sbin/apmd/apmsubr.c 26 Mar 2021 21:48:37 -0000
@@ -83,3 +83,20 @@ perf_mode(int mode)
return "invalid";
}
}
+
+const char *
+apm_state(int apm_state)
+{
+ switch (apm_state) {
+ case NORMAL:
+ return "normal";
+ case SUSPENDING:
+ return "suspend";
+ case STANDING_BY:
+ return "standby";
+ case HIBERNATING:
+ return "hibenate";
+ default:
+ return "unknown";
+}
+}
