On Fri, Mar 26, 2021 at 10:49:53PM +0100, Klemens Nanni wrote:
> 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.
Anyone interested in this attempt to improve user experience on
platforms without proper power management support?
suspend/resume is just a special case; in general I'm fond of programs
telling me when things like ioctls go wrong.
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 2 Apr 2021 16:07: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 2 Apr 2021 16:07:37 -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 2 Apr 2021 16:07: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 2 Apr 2021 16:07:41 -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";
+}
+}
