[Openais] proposing patches for AMF
Mathieu Marie
Mathieu.Marie at Sun.COM
Fri Sep 22 06:51:17 PDT 2006
Hi,
This is my first time on this alias.
I just installed openAIS (trunk) on Solaris and tried to play around
with AMF.
I found and fixed some minor problems that are mentioned below,
I added my diffs, in case you're interested in incorporating them.
Here are the problems I found :
1- On Solaris/sparc, the amf crashes with a SIGBUS, because
the serialize/deserialize methods are not aligned for sparc.
(amfutil.c)
I fixed it by aligning everything on 8 on sparc. The current fix
does not allow to communicate between sparc and non-sparc
nodes, because I differentiated the sparc case.
To allow them to communicate, the ALIGN_ADDR macro below
should be the same in sparc and non-sparc cases.
In the same fix, I modified the sa_serialize_SaUint64T to make
it similar to the other methods (that method is not currently used
in trunk).
Index: amfutil.c
===================================================================
--- amfutil.c (revision 1241)
+++ amfutil.c (working copy)
@@ -1067,14 +1067,19 @@
return assignment_state_text[state];
}
-#define ALIGN_ADDR(addr) ((addr) + (4 - ((unsigned long)(addr) % 4)))
+#ifdef __sparc
+#define ALIGN_ADDR(addr) ((addr) + (8 - ((unsigned long)(addr) % 8)))
+#else
+#define ALIGN_ADDR(addr) addr
+#endif
char *amf_serialize_SaNameT (char *buf, int *size, int *offset, SaNameT
*name)
{
char *tmp = buf;
- if ((*size - *offset ) < sizeof (SaNameT)) {
- *size += sizeof (SaNameT);
+ if ((*size - *offset ) < ALIGN_ADDR(sizeof (SaNameT))) {
+ *size += ALIGN_ADDR(sizeof(SaNameT));
+
tmp = realloc (buf, *size);
if (tmp == NULL) {
openais_exit_error (AIS_DONE_OUT_OF_MEMORY);
@@ -1083,7 +1088,7 @@
memcpy (&tmp[*offset], name, sizeof (SaNameT));
- (*offset) += sizeof (SaNameT);
+ (*offset) += ALIGN_ADDR(sizeof (SaNameT));
return tmp;
}
@@ -1098,61 +1103,77 @@
len = 0;
}
- return amf_serialize_opaque (buf, size, offset, str, len);
+ return (amf_serialize_opaque (buf, size, offset, str, len));
}
char *amf_serialize_SaUint32T (char *buf, int *size, int *offset,
SaUint32T num)
{
char *tmp = buf;
+ int lsize = *size;
- if ((*size - *offset ) < sizeof (SaUint32T)) {
- *size += sizeof (SaUint32T);
- tmp = realloc (buf, *size);
+ if ((*size - *offset ) < ALIGN_ADDR(sizeof (SaUint32T))) {
+ lsize += ALIGN_ADDR(sizeof (SaUint32T));
+ tmp = realloc (buf, lsize);
if (tmp == NULL) {
openais_exit_error (AIS_DONE_OUT_OF_MEMORY);
}
}
*((SaUint32T *)&tmp[*offset]) = num;
- (*offset) += sizeof (SaUint32T);
+ (*offset) += ALIGN_ADDR(sizeof (SaUint32T));
+ *size = lsize;
- return tmp;
+ return (tmp);
}
-char *amf_serialize_SaUint64T (char *buf, SaUint64T num)
+char *amf_serialize_SaUint64T (char *buf, int *size, int *offset,
SaUint64T num)
{
- *((SaUint64T *)buf) = num;
- return buf + sizeof (SaUint64T);
+ char *tmp = buf;
+ int lsize = *size;
+
+ if ((*size - *offset ) < ALIGN_ADDR(sizeof (SaUint64T))) {
+ lsize += ALIGN_ADDR(sizeof (SaUint64T));
+ tmp = realloc (buf, lsize);
+ if (tmp == NULL) {
+ openais_exit_error (AIS_DONE_OUT_OF_MEMORY);
+ }
+ }
+
+ *((SaUint64T *)&tmp[*offset]) = num;
+ (*offset) += ALIGN_ADDR(sizeof (SaUint64T));
+ *size = lsize;
+
+ return (tmp);
}
char *amf_serialize_opaque (
char *buf, int *size, int *offset, char *src, int cnt)
{
- unsigned int required_size;
+
char *tmp = buf;
+ int lsize = *size;
- required_size = cnt + sizeof (SaUint32T);
-
- if ((*size - *offset ) < required_size) {
- *size += required_size;
- tmp = realloc (buf, *size);
+ if ((*size - *offset ) < (ALIGN_ADDR(sizeof(SaUint32T)) +
ALIGN_ADDR(cnt))) {
+ lsize += (ALIGN_ADDR(sizeof(SaUint32T)) + ALIGN_ADDR(cnt));
+ tmp = realloc (buf, lsize);
if (tmp == NULL) {
openais_exit_error (AIS_DONE_OUT_OF_MEMORY);
}
}
*((SaUint32T *)&tmp[*offset]) = cnt;
- (*offset) += sizeof (SaUint32T);
+ (*offset) += ALIGN_ADDR(sizeof (SaUint32T));
memcpy (&tmp[*offset], src, cnt);
- (*offset) += cnt;
+ (*offset) += ALIGN_ADDR(cnt);
+ *size = lsize;
- return tmp;
+ return (tmp);
}
char *amf_deserialize_SaNameT (char *buf, SaNameT *name)
{
memcpy (name, buf, sizeof (SaNameT));
- return (buf + sizeof (SaNameT));
+ return (buf + ALIGN_ADDR(sizeof (SaNameT)));
}
char *amf_deserialize_SaStringT (char *buf, SaStringT *str)
@@ -1161,7 +1182,7 @@
char *tmp, *tmp_str;
len = *((SaUint32T *)buf);
- tmp = buf + sizeof (SaUint32T);
+ tmp = buf + ALIGN_ADDR(sizeof (SaUint32T));
if (len > 0) {
tmp_str = amf_malloc (len + 1);
@@ -1172,28 +1193,28 @@
*str = NULL;
}
- tmp += len;
+ tmp += ALIGN_ADDR(len);
- return tmp;
+ return (tmp);
}
char *amf_deserialize_SaUint32T (char *buf, SaUint32T *num)
{
*num = *((SaUint32T *)buf);
- return buf + sizeof (SaUint32T);
+ return (buf + ALIGN_ADDR(sizeof (SaUint32T)));
}
char *amf_deserialize_SaUint64T (char *buf, SaUint64T *num)
{
*num = *((SaUint64T *)buf);
- return buf + sizeof (SaUint64T);
+ return (buf + ALIGN_ADDR(sizeof (SaUint64T)));
}
char *amf_deserialize_opaque (char *buf, char *dst, int *cnt)
{
*cnt = *((SaUint32T *)buf);
- memcpy (dst, buf + sizeof (SaUint32T), *cnt);
- return buf + *cnt + sizeof (SaUint32T);
+ memcpy (dst, buf + ALIGN_ADDR(sizeof (SaUint32T)), *cnt);
+ return (buf + (ALIGN_ADDR(sizeof (SaUint32T)) + ALIGN_ADDR(*cnt)));
}
void *_amf_malloc (size_t size, char *file, unsigned int line)
Index: amf.h
===================================================================
--- amf.h (revision 1241)
+++ amf.h (working copy)
@@ -542,7 +542,8 @@
char *buf, int *size, int *offset, SaStringT str);
extern char *amf_serialize_SaUint32T (
char *buf, int *size, int *offset, SaUint32T num);
-extern char *amf_serialize_SaUint64T (char *buf, SaUint64T num);
+extern char *amf_serialize_SaUint64T (
+ char *buf, int *size, int *offset, SaUint64T num);
extern char *amf_serialize_opaque (
char *buf, int *size, int *offset, char *cp, int cnt);
extern char *amf_deserialize_SaNameT (char *buf, SaNameT *name);
2- On Solaris, the SA components executed have no names.
"ps -p <pid> -o comm" will show that the process has no command name.
This is due to the way the execve() is called in openais-instantiate.c.
On linux, the command name of a component is shown between
square brackets, due to that same problem.
Index: openais-instantiate.c
===================================================================
--- openais-instantiate.c (revision 1241)
+++ openais-instantiate.c (working copy)
@@ -96,7 +96,7 @@
/*
* child process
*/
- res = execve (argv[1], &argv[2], envp);
+ res = execve (argv[1], &argv[1], envp);
if (res == -1) {
return (errno);
}
3- When killing the testamf1 component, it makes the aisexec process
crash on both of my nodes.
This comes from an "==" sign in the clc_cli_script file, instead of
an "-eg".
(Even with this fix, every times clc_cli_script returns a non-zero
value,
the aisexec process will crash. I've seen TODOs in that portion of
the code, so I stopped there).
Index: clc_cli_script
===================================================================
--- clc_cli_script (revision 1240)
+++ clc_cli_script (working copy)
@@ -60,7 +60,7 @@
# `cat $PIDFILEPATH/openais_cleanup_$SA_AMF_COMPONENT_NAME`
kill -9 `cat $PIDFILEPATH/openais_cleanup_$SA_AMF_COMPONENT_NAME`
STATUS=$?
- if [ $STATUS == 1 ]; then
+ if [ $STATUS -eq 1 ]; then
exit 0
else
exit $STATUS
4- the testamf1 component also prints out a trace that it can't set the
scheduling class to RR with a priority of 99, because the max priority
for RR on solaris is 59.
Index: testamf1.c
===================================================================
--- testamf1.c (revision 1240)
+++ testamf1.c (working copy)
@@ -299,11 +299,17 @@
SaVersionT version = { 'B', 1, 1 };
-#if ! defined(TS_CLASS) && (defined(OPENAIS_BSD) ||
defined(OPENAIS_LINUX) || defined(OPENAIS_SOLARIS))
+#if ! defined(TS_CLASS)
+#if (defined(OPENAIS_BSD) || defined(OPENAIS_LINUX))
static struct sched_param sched_param = {
sched_priority: 99
};
+#elif defined(OPENAIS_SOLARIS)
+static struct sched_param sched_param = {
+ sched_priority: 59
+ };
#endif
+#endif
void sigintr_handler (int signum) {
stop = FINALIZE;
@@ -379,7 +385,7 @@
#if ! defined(TS_CLASS) && (defined(OPENAIS_BSD) ||
defined(OPENAIS_LINUX) || defined(OPENAIS_SOLARIS))
result = sched_setscheduler (0, SCHED_RR, &sched_param);
if (result == -1) {
- fprintf (stderr, "%d: couldn't set sched priority\n",
(int)getpid());
+ fprintf (stderr, "%d: couldn't set sched priority
(%s)\n", (int)getpid(), strerror(errno));
}
#endif
Mathieu
More information about the Openais
mailing list