Skip to content

Add process iterator test #131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion init.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ net_up(void)
err(1, "socket");

bzero(&ifr, sizeof(ifr));
strlcpy(ifr.ifr_name, "lo", sizeof(ifr.ifr_name));
strncpy(ifr.ifr_name, "lo", sizeof(ifr.ifr_name));
if (ioctl(fd, SIOCGIFFLAGS, &ifr) == -1)
err(1, "SIOCGIFFLAGS");
ifr.ifr_flags |= IFF_UP;
Expand Down
98 changes: 98 additions & 0 deletions quark-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,103 @@ t_fork_exec_exit(const struct test *t, struct quark_queue_attr *qa)
return (0);
}

static int
t_process_iterator(const struct test *t, struct quark_queue_attr *qa)
{
struct quark_queue qq;
struct quark_process_iter qi;
const struct quark_process *qp;
int n_children = 3;
pid_t children[n_children];
Comment on lines +445 to +446
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int n_children = 3;
pid_t children[n_children];
pid_t children[3];

See the nitems() comment below, also .

struct args *args;
size_t expected_len;
int i;
int j;
char cwd[PATH_MAX];

if (quark_queue_open(&qq, qa) != 0)
err(1, "quark_queue_open");

quark_process_iter_init(&qi, &qq);

Comment on lines +456 to +457
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you initialize the iterator before drain_for_pid, so the iterator can be invalidated, just move this to before the main loop. I realize this is not ideal, maybe I should change iter_init to point to NULL, instead of pointing to RB_ROOT.

for (i = 0; i < n_children; i++) {
children[i] = fork_exec_nop();
drain_for_pid(&qq, children[i]);
}
Comment on lines +458 to +461
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (i = 0; i < n_children; i++) {
children[i] = fork_exec_nop();
drain_for_pid(&qq, children[i]);
}
for (i = 0; i < (int)nitems(children); i++)
children[i] = fork_exec_nop();
for (i = 0; i < (int)nitems(children); i++)
children[i] = drain_for_pid(&qq, children[i]);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small trick, use nitems() so you can cut n_children out.
Because of how aging works in quark, it makes more sense to fork all + drain all, otherwise each drain will have to age the event for 25ms, so this cuts 75ms->25ms


for (i = 0; i < n_children; i++) {
do {
qp = quark_process_iter_next(&qi);
} while (qp != NULL && (pid_t)qp->pid != children[i]);
Comment on lines +463 to +466
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outer loop should be the iterator, and then look for the children.
This is only working now because of how the internal tree is sorted, if that changes, you would be skipping elements.

assert(qp->flags & QUARK_F_EXIT);
assert(qp->flags & QUARK_F_COMM);
assert(qp->flags & QUARK_F_FILENAME);
assert(qp->flags & QUARK_F_CMDLINE);
assert(qp->flags & QUARK_F_CWD);
assert((pid_t)qp->proc_ppid == getpid());
assert(qp->proc_time_boot > 0); /* XXX: improve */
assert(qp->proc_uid == getuid());
assert(qp->proc_gid == getgid());
assert(qp->proc_suid == geteuid());
assert(qp->proc_sgid == getegid());
assert(qp->proc_euid == geteuid());
assert(qp->proc_egid == getegid());
assert((pid_t)qp->proc_pgid == getpgid(0));
assert((pid_t)qp->proc_sid == getsid(0));
/* check capabilities */
/* XXX assumes we're root */
assert(qp->proc_cap_inheritable == 0);
/*
* We don't know the exact set since it varies from kernel,
* improve this in the future.
*/
assert(qp->proc_cap_effective != 0);
assert(qp->proc_cap_permitted != 0);
/* check entry leader */
/*
* XXX TODO This depends how we're running the test, if we're over ssh
* it will show ssh, if not it will show init and whatnot, for now
* assert that it is not unknown at least.
*/
assert(qp->proc_entry_leader != 0);
assert(qp->proc_entry_leader_type != QUARK_ELT_UNKNOWN);
/* XXX TODO check tty_major and tty_minor for self in the future */
#if 0
assert(qp->proc_tty_major != QUARK_TTY_UNKNOWN);
assert(qp->proc_tty_minor != 0);
#endif
/* check strings */
assert(!strcmp(qp->comm, program_invocation_short_name));
assert(!strcmp(qp->filename, binpath()));
/* check args */
args = args_make(qp);
assert(args != NULL);
assert(args->argc == 5);
assert(!strcmp(args->argv[0], binpath()));
assert(!strcmp(args->argv[1], "-N"));
assert(!strcmp(args->argv[2], "this"));
assert(!strcmp(args->argv[3], "is"));
assert(!strcmp(args->argv[4], "nop!"));
/*
* Expected len is the length of the arguments summed up, plus one byte
* for each argument(the NUL after each argument, including the last
* one), so we just start at 'argc' bytes.
*/
for (expected_len = args->argc, j = 0; j < args->argc; j++)
expected_len += strlen(args->argv[j]);
args_free(args);
assert(qp->cmdline_len == expected_len);

if (getcwd(cwd, sizeof(cwd)) == NULL)
err(1, "getcwd");
assert(!strcmp(cwd, qp->cwd));

Comment on lines +467 to +529
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(qp->flags & QUARK_F_EXIT);
assert(qp->flags & QUARK_F_COMM);
assert(qp->flags & QUARK_F_FILENAME);
assert(qp->flags & QUARK_F_CMDLINE);
assert(qp->flags & QUARK_F_CWD);
assert((pid_t)qp->proc_ppid == getpid());
assert(qp->proc_time_boot > 0); /* XXX: improve */
assert(qp->proc_uid == getuid());
assert(qp->proc_gid == getgid());
assert(qp->proc_suid == geteuid());
assert(qp->proc_sgid == getegid());
assert(qp->proc_euid == geteuid());
assert(qp->proc_egid == getegid());
assert((pid_t)qp->proc_pgid == getpgid(0));
assert((pid_t)qp->proc_sid == getsid(0));
/* check capabilities */
/* XXX assumes we're root */
assert(qp->proc_cap_inheritable == 0);
/*
* We don't know the exact set since it varies from kernel,
* improve this in the future.
*/
assert(qp->proc_cap_effective != 0);
assert(qp->proc_cap_permitted != 0);
/* check entry leader */
/*
* XXX TODO This depends how we're running the test, if we're over ssh
* it will show ssh, if not it will show init and whatnot, for now
* assert that it is not unknown at least.
*/
assert(qp->proc_entry_leader != 0);
assert(qp->proc_entry_leader_type != QUARK_ELT_UNKNOWN);
/* XXX TODO check tty_major and tty_minor for self in the future */
#if 0
assert(qp->proc_tty_major != QUARK_TTY_UNKNOWN);
assert(qp->proc_tty_minor != 0);
#endif
/* check strings */
assert(!strcmp(qp->comm, program_invocation_short_name));
assert(!strcmp(qp->filename, binpath()));
/* check args */
args = args_make(qp);
assert(args != NULL);
assert(args->argc == 5);
assert(!strcmp(args->argv[0], binpath()));
assert(!strcmp(args->argv[1], "-N"));
assert(!strcmp(args->argv[2], "this"));
assert(!strcmp(args->argv[3], "is"));
assert(!strcmp(args->argv[4], "nop!"));
/*
* Expected len is the length of the arguments summed up, plus one byte
* for each argument(the NUL after each argument, including the last
* one), so we just start at 'argc' bytes.
*/
for (expected_len = args->argc, j = 0; j < args->argc; j++)
expected_len += strlen(args->argv[j]);
args_free(args);
assert(qp->cmdline_len == expected_len);
if (getcwd(cwd, sizeof(cwd)) == NULL)
err(1, "getcwd");
assert(!strcmp(cwd, qp->cwd));

I think we can cut this, this is already tested multiple times in other places, if we find the processes through iteration I'm happy.

}
quark_queue_close(&qq);

return (0);
}

/* Make sure an exit comes from tgid, not tid */
static int
t_exit_tgid(const struct test *t, struct quark_queue_attr *qa)
Expand Down Expand Up @@ -633,6 +730,7 @@ t_stats(const struct test *t, struct quark_queue_attr *qa)
const struct test all_tests[] = {
T(t_probe),
T(t_fork_exec_exit),
T(t_process_iterator),
T(t_exit_tgid),
T(t_namespace),
T(t_cache_grace),
Expand Down