[guest-efi] Add varstored-watch #69
Conversation
varstored-watch/Makefile
Outdated
| varstored-watch: varstored-watch.o | ||
| $(CC) $(CFLAGS) $(LDFLAGS) $^ -o $@ | ||
|
|
||
| %.o: %.c | ||
| $(CC) -c $(CFLAGS) $< -o $@ | ||
|
|
There was a problem hiding this comment.
Delete all this and let the implicit rules do the work for you.
varstored-watch/Makefile
Outdated
| INCDIR ?= /usr/include | ||
| LIBDIR ?= /usr/lib |
varstored-watch/Makefile
Outdated
| INCDIR ?= /usr/include | ||
| LIBDIR ?= /usr/lib | ||
|
|
||
| LIBFLAGS = -lxenctrl |
There was a problem hiding this comment.
change this to
LDLIBS = -lxenctrl
varstored-watch/Makefile
Outdated
| LIBDIR ?= /usr/lib | ||
|
|
||
| LIBFLAGS = -lxenctrl | ||
| CFLAGS = -I${INCDIR} -g -Wall -Werror |
There was a problem hiding this comment.
CFLAGS += -g -Wall -Werror
varstored-watch/Makefile
Outdated
| rm -f varstored-watch *.o | ||
|
|
||
| install: | ||
| install -m 0755 varstored-watch /usr/sbin |
There was a problem hiding this comment.
This could take a D/DESTDIR prefix and then you could do make install for the OE recipe.
varstored-watch/varstored-watch.c
Outdated
| goto err; | ||
| } | ||
| if (dominfo.shutdown == true) { | ||
| syslog(LOG_ERR, "Domid %u is shutting down.\n", domid); |
There was a problem hiding this comment.
It'd be nicer to avoid having to open xc_interface to avoid the need for privcmd permissions. Would checking /local/domain/$domid be sufficient as an existence check? libxl will kill this process, so it doesn't need to exit itself on domain shutdown.
varstored-watch/Makefile
Outdated
| @@ -0,0 +1,15 @@ | |||
| CFLAGS += -g -Wall -Werror | |||
| LDLIBS = -lxenctrl -lxenstore | |||
| LDFLAGS = -L${LIBDIR} ${LDLIBS} | |||
There was a problem hiding this comment.
LIBDIR is no longer defined. You can drop more like so:
diff --git a/varstored-watch/Makefile b/varstored-watch/Makefile
index 76574d5..4498895 100644
--- a/varstored-watch/Makefile
+++ b/varstored-watch/Makefile
@@ -1,12 +1,8 @@
CFLAGS += -g -Wall -Werror
LDLIBS = -lxenctrl -lxenstore
-LDFLAGS = -L${LIBDIR} ${LDLIBS}
all: varstored-watch
-varstored-watch: varstored-watch.o
- $(CC) $(CFLAGS) $(LDFLAGS) $^ -o $@
-
clean:
rm -f varstored-watch *.o
varstored-watch/varstored-watch.c
Outdated
| } else if (WIFSIGNALED(rc)) { | ||
| syslog(LOG_ERR, "Varstored killed by signal %d, restarting.\n", WTERMSIG(rc)); | ||
| varstored_pid = start_varstored(xsh, domid_str, uuid); | ||
| } else if (WIFSTOPPED(rc)) { |
There was a problem hiding this comment.
WIFSTOPPED(wstatus)
returns true if the child process was stopped by delivery of a
signal; this is possible only if the call was done using WUN‐
TRACED or when the child is being traced (see ptrace(2)).
The process hasn't died - it has just been stopped. SIGCONT would resume it, I think? Therefore starting a new varstored doesn't seem right. WUNTRACED is not set in waitpid options through, so it would only happen under ptrace. In case someone does ptrace varstored, I'd just remove the WIFSTOPPED case.
There was a problem hiding this comment.
Yeah good point. Didn't catch the nuance there, removing.
varstored-watch/varstored-watch.c
Outdated
| ret = -1; | ||
| done: | ||
| if (xsh) | ||
| xs_close(xsh); |
There was a problem hiding this comment.
xsh is open for the lifetime of the program, but it's only used in xs_write_pid. The handle's fd probably leaks into the child varstored? Why don't you move xs_open/xs_close into xs_write_pid to keep it all together and avoid that.
varstored-watch is a helper binary that starts and monitors an instance of varstored. OXT-1826 Signed-off-by: Nicholas Tsirakis <[email protected]>
varstored-watch is a helper binary that starts and
monitors an instance of varstored.
OXT-1826
Signed-off-by: Nicholas Tsirakis [email protected]