[pve-devel] [PATCH qemu-server v3 1/3] add qmeventd

Wolfgang Bumiller w.bumiller at proxmox.com
Fri Nov 2 11:12:38 CET 2018


On Tue, Oct 30, 2018 at 04:06:56PM +0100, Dominik Csapak wrote:
> this adds a program that can listen to qemu qmp events on a given socket
> and if a shutdown event followed by a disconnected socket occurs,
> executes qm cleanup with arguments that indicate if the
> vm was closed gracefully and whether the guest initiated it
> 
> this is useful if we want to cleanup after the qemu process exited,
> e.g. tap devices, vgpus, etc.
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  Makefile         |  21 ++-
>  debian/control   |   1 +
>  debian/rules     |   2 +-
>  qmeventd.c       | 386 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmeventd.h       |  45 +++++++
>  qmeventd.rst     |  38 ++++++
>  qmeventd.service |  10 ++
>  7 files changed, 499 insertions(+), 4 deletions(-)
>  create mode 100644 qmeventd.c
>  create mode 100644 qmeventd.h
>  create mode 100644 qmeventd.rst
>  create mode 100644 qmeventd.service
> 
> diff --git a/Makefile b/Makefile
> index c531d04..6c6f165 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,10 @@ VERSION=5.0
>  PACKAGE=qemu-server
>  PKGREL=38
>  
> -CFLAGS= -O2 -Werror -Wall -Wtype-limits -Wl,-z,relro 
> +CFLAGS=-O2 -Werror -Wall -Wextra -Wpedantic -Wtype-limits -Wl,-z,relro
> +JSON_CFLAGS=$(shell pkg-config --cflags json-c)
> +JSON_LIBS=$(shell pkg-config --libs json-c)
> +RST2MAN=/usr/bin/rst2man
>  
>  DESTDIR=
>  PREFIX=/usr
> @@ -10,6 +13,7 @@ BINDIR=${PREFIX}/bin
>  SBINDIR=${PREFIX}/sbin
>  BINDIR=${PREFIX}/bin
>  LIBDIR=${PREFIX}/lib/${PACKAGE}
> +SERVICEDIR=/lib/systemd/system
>  VARLIBDIR=/var/lib/${PACKAGE}
>  MANDIR=${PREFIX}/share/man
>  DOCDIR=${PREFIX}/share/doc
> @@ -36,6 +40,12 @@ all:
>  dinstall: deb
>  	dpkg -i ${DEB}
>  
> +qmeventd: qmeventd.c
> +	$(CC) $(CFLAGS) ${JSON_CFLAGS} -o $@ $< ${JSON_LIBS} 
> +
> +qmeventd.1: qmeventd.rst
> +	${RST2MAN} $< $@
> +
>  qm.bash-completion:
>  	PVE_GENERATING_DOCS=1 perl -I. -T -e "use PVE::CLI::qm; PVE::CLI::qm->generate_bash_completions();" >$@.tmp
>  	mv $@.tmp $@
> @@ -44,13 +54,14 @@ qmrestore.bash-completion:
>  	PVE_GENERATING_DOCS=1 perl -I. -T -e "use PVE::CLI::qmrestore; PVE::CLI::qmrestore->generate_bash_completions();" >$@.tmp
>  	mv $@.tmp $@
>  
> -PKGSOURCES=qm qm.1 qmrestore qmrestore.1 qmextract qm.conf.5 qm.bash-completion qmrestore.bash-completion
> +PKGSOURCES=qm qm.1 qmrestore qmrestore.1 qmextract qm.conf.5 qm.bash-completion qmrestore.bash-completion qmeventd qmeventd.1
>  
>  .PHONY: install
>  install: ${PKGSOURCES}
>  	install -d ${DESTDIR}/${SBINDIR}
>  	install -d ${DESTDIR}${LIBDIR}
>  	install -d ${DESTDIR}${VARLIBDIR}
> +	install -d ${DESTDIR}${SERVICEDIR}
>  	install -d ${DESTDIR}/${MAN1DIR}
>  	install -d ${DESTDIR}/${MAN5DIR}
>  	install -d ${DESTDIR}/usr/share/man/man5
> @@ -63,6 +74,8 @@ install: ${PKGSOURCES}
>  	make -C PVE install
>  	install -m 0755 qm ${DESTDIR}${SBINDIR}
>  	install -m 0755 qmrestore ${DESTDIR}${SBINDIR}
> +	install -m 0755 qmeventd ${DESTDIR}${SBINDIR}
> +	install -m 0644 qmeventd.service ${DESTDIR}${SERVICEDIR}
>  	install -m 0755 pve-bridge ${DESTDIR}${VARLIBDIR}/pve-bridge
>  	install -m 0755 pve-bridge-hotplug ${DESTDIR}${VARLIBDIR}/pve-bridge-hotplug
>  	install -m 0755 pve-bridgedown ${DESTDIR}${VARLIBDIR}/pve-bridgedown
> @@ -70,6 +83,8 @@ install: ${PKGSOURCES}
>  	install -m 0755 qmextract ${DESTDIR}${LIBDIR}
>  	install -m 0644 qm.1 ${DESTDIR}/${MAN1DIR}
>  	gzip -9 -n -f ${DESTDIR}/${MAN1DIR}/qm.1
> +	install -m 0644 qmeventd.1 ${DESTDIR}/${MAN1DIR}
> +	gzip -9 -n -f ${DESTDIR}/${MAN1DIR}/qmeventd.1
>  	install -m 0644 qmrestore.1 ${DESTDIR}/${MAN1DIR}
>  	gzip -9 -n -f ${DESTDIR}/${MAN1DIR}/qmrestore.1
>  	install -m 0644 qm.conf.5 ${DESTDIR}/${MAN5DIR}
> @@ -97,7 +112,7 @@ upload: ${DEB}
>  .PHONY: clean
>  clean:
>  	make cleanup-docgen
> -	rm -rf build *.deb *.buildinfo *.changes
> +	rm -rf build *.deb *.buildinfo *.changes qmeventd
>  	find . -name '*~' -exec rm {} ';'
>  
>  
> diff --git a/debian/control b/debian/control
> index f3b9ca0..5b3cb1f 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -3,6 +3,7 @@ Section: admin
>  Priority: optional
>  Maintainer: Proxmox Support Team <support at proxmox.com>
>  Build-Depends: debhelper (>= 7.0.50~),
> +               docutils,
>                 libio-multiplex-perl,
>                 libpve-common-perl,
>                 libpve-guest-common-perl (>= 2.0-18),
> diff --git a/debian/rules b/debian/rules
> index 955dd78..97112b4 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -10,4 +10,4 @@
>  #export DH_VERBOSE=1
>  
>  %:
> -	dh $@
> +	dh $@ --with systemd
> diff --git a/qmeventd.c b/qmeventd.c
> new file mode 100644
> index 0000000..9498cd0
> --- /dev/null
> +++ b/qmeventd.c
> @@ -0,0 +1,386 @@
> +/*
> +
> +    Copyright (C) 2018 Proxmox Server Solutions GmbH
> +
> +    Copyright: qemumonitor is under GNU GPL, the GNU General Public License.
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; version 2 dated June, 1991.
> +
> +    This program is distributed in the hope that it will be useful,
> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +    GNU General Public License for more details.
> +
> +    You should have received a copy of the GNU General Public License
> +    along with this program; if not, write to the Free Software
> +    Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> +    02111-1307, USA.
> +
> +    Author: Dominik Csapak <d.csapak at proxmox.com>
> +
> +    qmevend listens on a given socket, and waits for qemu processes
> +    to connect
> +
> +    it then waits for shutdown events followed by the closing of the socket,
> +    it then calls /usr/sbin/qm cleanup with following arguments
> +
> +    /usr/sbin/qm cleanup VMID <graceful> <guest>
> +
> +    parameter explanation:
> +
> +    graceful:
> +    1|0 depending if it saw a shutdown event before the socket closed
> +
> +    guest:
> +    1|0 depending if the shutdown was requested from the guest
> +
> +*/
> +
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
> +#include <errno.h>
> +#include <json.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/epoll.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +#include <sys/un.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +
> +#include "qmeventd.h"
> +
> +static int verbose = 0;
> +static int epoll_fd = 0;
> +/*
> + * Helper functions
> + */
> +
> +static void
> +usage (char *argv[])

remove the space before (

> +{
> +    fprintf(stderr, "Usage: %s [-f] [-v] PATH\n", argv[0]);
> +    fprintf(stderr, "  -f       run in foreground (default: false)\n");
> +    fprintf(stderr, "  -v       verbose (default: false)\n");
> +    fprintf(stderr, "  PATH     use PATH for socket\n");
> +}
> +
> +static pid_t
> +get_pid_from_fd (int fd)

remove the space before (

> +{
> +    struct ucred credentials;
> +    socklen_t len = sizeof(struct ucred);
> +    PERR_NEG(getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &credentials, &len), "getsockopt");
> +    return credentials.pid;
> +}
> +
> +/*
> + * reads the vmid from /proc/<pid>/cmdline
> + * after the '-id' argument
> + */
> +static unsigned int
> +get_vmid_from_pid (pid_t pid)

remove the space before (

> +{
> +    unsigned int vmid = 0;
> +    int ch;
> +    FILE *fp;
> +    char filename[32] = { 0 };
> +    char buf[5] = { 0 };
> +    sprintf(filename, "/proc/%d/cmdline", pid);
> +    fp = fopen(filename, "r");

Use "re" please (even though here it's not strictly necessary).

> +    if (fp == NULL) {
> +	perror("fopen cmdline");
> +	goto ret;

At this point you can still just `return 0;` as there's nothing to clean
up.

> +    }
> +
> +    while(fgets(buf, 5, fp) != NULL) {

You can use getdelim() with a literal 0 as delimiter to go through
parameters here. Alternatively you add a slurping `read_whole_file`
helper and iterate via strlen()+1 through the entries, looking for
an exact "-id" string via strcmp().
Either way will be shorter than this mess ;-)

> +	if (strcmp(buf, "-id") == 0) {
> +	    int i = 0;
> +	    while ((ch = fgetc(fp)) != EOF) {
> +		if (ch <= '9' && ch >= '0') {
> +		    if (i >= VMID_LEN - 1) {
> +			fprintf(stderr, "vmid too long\n");
> +			vmid = 0;
> +			goto ret;
> +		    }
> +		    vmid *= 10;
> +		    vmid += (unsigned int)(ch - '0');
> +		    i++;
> +		} else if (ch == '\0') {
> +		    goto ret;
> +		} else {
> +		    fprintf(stderr, "invalid id\n");
> +		    vmid = 0;
> +		    goto ret;
> +		}
> +	    }
> +	} else {
> +	    while((ch = fgetc(fp)) != EOF && ch != '\0');
> +	}
> +    }
> +
> +ret:
> +    fclose(fp);
> +    return vmid;
> +}
> +
> +/*
> + * qmp handling functions
> + */
> +
> +void
> +handle_qmp_handshake(struct Client *client)
> +{
> +    VERBOSE_PRINT("%s: got QMP handshake\n", client->vmid);
> +    ssize_t wlen;
> +    do {
> +	wlen = write(client->fd, QMP_ANSWER, strlen(QMP_ANSWER) - 1);
> +    } while (wlen != strlen(QMP_ANSWER) - 1 && errno == EINTR);

While this only appears once, it would probably still look better with a
`bool must_write(fd, buf, len)` doing the EINTR loop returning true on
success.

> +    if (wlen != strlen(QMP_ANSWER) - 1) {

Would save you the extra strlen(QMP_ANSWER) duplicate here:
    if (!must_write(fd, QMP_ANSWER, sizeof(QMP_ANSWER)-1))

Also, why strlen()-1? Should be sizeof()-1 or strlen() without the -1
AFAICT?

> +	fprintf(stderr, "%s: can not complete handshake\n", client->vmid);
> +	cleanup_client(client);
> +    }
> +}
> +
> +void
> +handle_qmp_event(struct Client *client, struct json_object *obj)
> +{
> +    struct json_object *event;
> +    if (!json_object_object_get_ex(obj, "event", &event)) {
> +	return;
> +    }
> +    VERBOSE_PRINT("%s: got QMP event: %s\n", client->vmid,
> +		  json_object_get_string(event));
> +    // event, check if shutdown and get guest parameter
> +    if (!strcmp(json_object_get_string(event), "SHUTDOWN")) {
> +	client->graceful = 1;
> +	struct json_object *data;
> +	struct json_object *guest;
> +	if (json_object_object_get_ex(obj, "data", &data) &&
> +	    json_object_object_get_ex(data, "guest", &guest)) {

With the condition spanning multiple lines I'd really prefer the '{' on
a separate line, especially with a 4-space indentation producing the
exact same alignment as the condition itself.

> +	    client->guest = (unsigned short)json_object_get_boolean(guest);
> +	}
> +    }
> +}
> +
> +/*
> + * client management functions
> + */
> +
> +void
> +add_new_client(int client_fd)
> +{
> +    struct Client *client = calloc(sizeof(struct Client), 1);
> +    client->fd = client_fd;
> +    client->pid = get_pid_from_fd(client_fd);
> +    unsigned int vmid = get_vmid_from_pid(client->pid);
> +    if (vmid == 0) {
> +	fprintf(stderr, "invalid client\n");
> +	PERR_NEG(close(client_fd), "close invalid client");

This should not be a fatal error.

> +	free(client);
> +	return;
> +    }
> +    snprintf(client->vmid, VMID_LEN, "%d", vmid);

You should check that this does not return -1 or >= VMID_LEN (in which
case it would be truncated). snprintf() returns the number of bytes
(excluding the 0 terminator) it _would_ require to print the string, so
this can in theory return even more than VMID_LEN.

> +
> +    struct epoll_event ev;
> +    ev.events = EPOLLIN;
> +    ev.data.ptr = client;
> +    PERR_NEG(epoll_ctl(epoll_fd, EPOLL_CTL_ADD, client_fd, &ev),
> +	     "epoll_ctl client");

This should not be a fatal error.

> +    VERBOSE_PRINT("added new client, pid: %d, vmid: %s\n", client->pid,
> +		client->vmid);
> +}
> +
> +void
> +cleanup_client(struct Client *client)
> +{
> +    VERBOSE_PRINT("%s: client exited, status: graceful: %d, guest: %d\n",
> +		  client->vmid, client->graceful, client->guest);
> +    PERR_NEG(epoll_ctl(epoll_fd, EPOLL_CTL_DEL, client->fd, NULL), "epoll del");
> +    PERR_NEG(close(client->fd), "close client");

No error in this function should be considered fatal. There's no reason
to expect close() to fail, and a failed close() doesn't mean the file
descriptor's lingering, it only means a previous operation on the file
descriptor couldn't be completed, so in this function I'd just
    (void)epoll_ctl(epoll_fd, EPOLL_CTL_DEL, client->fd, NULL);
    (void)close(client->fd);
(Or at most log an error from a failing epoll_ctl())

> +    unsigned short graceful = client->graceful;
> +    unsigned short guest = client->guest;
> +    char vmid[VMID_LEN + 1];
> +    strncpy(vmid, client->vmid, VMID_LEN + 1);

Please don't copy size expressions, even when they're using a macro
already. Use sizeof(vmid) here.
Also, considering client->vmid is always null terminated, the +1
shouldn't be necessary, otherwise we shouldn't even get here as the
snprintf() producing the string should have cancelled the client when it
connected.

> +    free(client);
> +    VERBOSE_PRINT("%s: executing cleanup\n", vmid);
> +
> +    int pid = fork();
> +    if (pid < 0) {
> +	fprintf(stderr, "fork failed: %s\n", strerror(errno));
> +	return;
> +    }
> +    if (pid == 0) {
> +	char *script = "/usr/sbin/qm";
> +	char **args = malloc((size_t)(6) * sizeof(*args));
> +
> +	args[0] = script;
> +	args[1] = "cleanup";
> +	args[2] = vmid;
> +
> +	char shutdown_args[4] = {
> +	    graceful ? '1' : '0', 0,
> +	    guest    ? '1' : '0', 0,
> +	};
> +
> +	args[3] = &shutdown_args[0];
> +	args[4] = &shutdown_args[2];
> +	args[5] = NULL;
> +
> +	execvp(script, args);
> +	exit(1);

Use `_exit` here, and please log an error before exiting.

> +    }

If we aren't wait()-ing for the child here, we should setup a SIGCLD
handler to reap child processes one way or another.

> +}
> +
> +void
> +handle_client(struct Client *client)
> +{
> +    VERBOSE_PRINT("%s: entering handle\n", client->vmid);
> +    ssize_t len;
> +    len = read(client->fd, (client->buf+client->buflen), BUF_SIZE - client->buflen);

Misses the -1 error case.

> +    VERBOSE_PRINT("%s: read %ld bytes\n", client->vmid, len);
> +    if (len == 0) {
> +	cleanup_client(client);
> +	return;
> +    }
> +    client->buflen += len;
> +
> +    struct json_tokener *tok = json_tokener_new();
> +    struct json_object *jobj = NULL;
> +    enum json_tokener_error jerr = json_tokener_success;
> +    while (jerr == json_tokener_success && client->buflen != 0) {
> +	jobj = json_tokener_parse_ex(tok, client->buf, (int)client->buflen);
> +	jerr = json_tokener_get_error(tok);
> +	unsigned int offset = (unsigned int)tok->char_offset;
> +	switch (jerr) {
> +	    case json_tokener_success:
> +		// move rest from buffer to front
> +		memmove(client->buf, client->buf + offset, client->buflen - offset);
> +		client->buflen -= offset;
> +		if (json_object_is_type(jobj, json_type_object)) {
> +		    struct json_object *obj;
> +		    if (json_object_object_get_ex(jobj, "QMP", &obj)) {
> +			handle_qmp_handshake(client);
> +		    } else if (json_object_object_get_ex(jobj, "event", &obj)) {
> +			handle_qmp_event(client, jobj);
> +		    } // else ignore message
> +		}
> +		break;
> +	    case json_tokener_continue:
> +		if (client->buflen >= BUF_SIZE) {
> +		    VERBOSE_PRINT("%s, msg too large, discarding buffer\n",
> +				  client->vmid);
> +		    memset(client->buf, 0, BUF_SIZE);
> +		    client->buflen = 0;
> +		} // else we have enough space try again after next read
> +		break;
> +	    case json_tokener_error_depth:
> +	    case json_tokener_error_parse_eof:
> +	    case json_tokener_error_parse_unexpected:
> +	    case json_tokener_error_parse_null:
> +	    case json_tokener_error_parse_boolean:
> +	    case json_tokener_error_parse_number:
> +	    case json_tokener_error_parse_array:
> +	    case json_tokener_error_parse_object_key_name:
> +	    case json_tokener_error_parse_object_key_sep:
> +	    case json_tokener_error_parse_object_value_sep:
> +	    case json_tokener_error_parse_string:
> +	    case json_tokener_error_parse_comment:
> +	    case json_tokener_error_size:

This should probably just be `default:`, otherwise, without a default
handler, this would loop endlessly over the same erroneous content.
Unless json_tokener_parse_ex() somehow deals with getting the same
buffer passed to it again with some internal state?

> +		VERBOSE_PRINT("%s: parse error: %d, discarding buffer\n",
> +			      client->vmid, jerr);
> +		memset(client->buf, 0, client->buflen);
> +		client->buflen = 0;
> +		break;
> +	}
> +	json_object_put(jobj);
> +    }
> +    json_tokener_free(tok);
> +}
> +
> +
> +int
> +main (int argc, char *argv[])

remove the space before (

> +{
> +    int opt;
> +    int daemonize = 1;
> +    char *socket_path = NULL;
> +
> +    while ((opt = getopt(argc, argv, "hfv")) != -1) {
> +	switch (opt) {
> +	    case 'f':
> +		daemonize = 0;
> +		break;
> +	    case 'v':
> +		verbose = 1;
> +		break;
> +	    case 'h':
> +	    default:
> +		usage(argv);
> +		exit(EXIT_FAILURE);
> +	}
> +    }
> +
> +    if (optind >= argc) {
> +	usage(argv);
> +	exit(EXIT_FAILURE);
> +    }
> +
> +    socket_path = argv[optind];
> +
> +    int sock = socket(AF_UNIX, SOCK_STREAM, 0);
> +    PERR_NEG(sock, "socket");
> +
> +    struct sockaddr_un addr;
> +    memset(&addr, 0, sizeof(addr));
> +    addr.sun_family = AF_UNIX;
> +    strncpy(addr.sun_path, socket_path, sizeof(addr.sun_path) - 1);
> +
> +    unlink(socket_path);
> +    PERR_NEG(bind(sock, (struct sockaddr*)&addr, sizeof(addr)), "bind");
> +
> +    struct epoll_event ev, events[1];
> +    epoll_fd = epoll_create1(0);

We definitely want to pass EPOLL_CLOEXEC here, otherwise all our hooks
get a copy of the epoll file descriptor.

> +    PERR_NEG(epoll_fd, "epoll_create1");
> +
> +    ev.events = EPOLLIN;
> +    ev.data.fd = sock;
> +    PERR_NEG(epoll_ctl(epoll_fd, EPOLL_CTL_ADD, sock, &ev), "epoll_ctl");
> +
> +    PERR_NEG(listen(sock, 10), "listen");
> +
> +    if (daemonize) {
> +	PERR_NEG(daemon(0, 1), "daemon");
> +    }
> +
> +    int should_exit = 0;
> +    int nevents, n;
> +
> +    while(!should_exit) {
> +	nevents = epoll_wait(epoll_fd, events, 1, -1);
> +	if (nevents < 0 && errno == EINTR) {
> +	    // signal happened, try again
> +	    continue;
> +	}
> +	PERR_NEG(nevents, "epoll_wait");
> +
> +	for (n = 0; n < nevents; n++) {
> +	    if (events[n].data.fd == sock) {
> +
> +		int conn_sock = accept(sock, NULL, NULL);

Please use accept4(2), passing SOCK_CLOEXEC as flags. You can then also
add SOCK_NONBLOCK as well and skip the two fcntl() calls below
altogether.

> +		PERR_NEG(conn_sock, "accept");

This should probably not be a fatal error. The worst I'd expect is an
ECONNABORTED from a killed/crashed qemu instance. Anything else (ENOMEM,
ENOBUFS, ...) usually means there's a chaos monkey on the loose anyway,
EMFILE and ENFILE would just keep happening to the next instance of
qmeventd, etc.
So just log and ignore.

> +		int flags = fcntl(conn_sock, F_GETFL);
> +		PERR_NEG(fcntl(conn_sock, F_SETFL, flags | O_NONBLOCK), "fcntl");
> +
> +		add_new_client(conn_sock);
> +	    } else {
> +		handle_client((struct Client *)events[n].data.ptr);
> +	    }
> +	}
> +    }
> +}
> diff --git a/qmeventd.h b/qmeventd.h
> new file mode 100644
> index 0000000..39024e6
> --- /dev/null
> +++ b/qmeventd.h
> @@ -0,0 +1,45 @@
> +/*
> +
> +    Copyright (C) 2018 Proxmox Server Solutions GmbH
> +
> +    Copyright: qemumonitor is under GNU GPL, the GNU General Public License.
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; version 2 dated June, 1991.
> +
> +    This program is distributed in the hope that it will be useful,
> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +    GNU General Public License for more details.
> +
> +    You should have received a copy of the GNU General Public License
> +    along with this program; if not, write to the Free Software
> +    Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> +    02111-1307, USA.
> +
> +    Author: Dominik Csapak <d.csapak at proxmox.com>
> +*/
> +
> +#define PERR_NEG(X,Y) do { if (X < 0) { perror(Y); exit(EXIT_FAILURE); }} while (0)

I'd prefer this as a real function (`static inline _Noreturn`)

Also, this effectively aborts on error - (see the individual comments
at some of its uses).

> +#define VERBOSE_PRINT(...) do { if (verbose) { printf(__VA_ARGS__); } } while (0)
> +
> +#define BUF_SIZE 4096

Quite a generic name for a macro in C ;-)
I'd put that directly into the struct and use sizeof() instead of
BUF_SIZE in the .c file everywhere

> +#define VMID_LEN 16
> +#define QMP_ANSWER "{\"execute\":\"qmp_capabilities\"}\n"

This is a rather specific part of the protocol which could jus as well
be a `static const char qmp_answer[]` inside handle_qmp_handshake().

> +
> +struct Client {
> +    char buf[BUF_SIZE];
> +    char vmid[VMID_LEN];
> +    int fd;
> +    pid_t pid;
> +    unsigned int buflen;
> +    unsigned short graceful;
> +    unsigned short guest;
> +};
> +
> +void handle_qmp_handshake(struct Client *client);
> +void handle_qmp_event(struct Client *client, struct json_object *obj);
> +void handle_client(struct Client *client);
> +void add_new_client(int client_fd);
> +void cleanup_client(struct Client *client);
> diff --git a/qmeventd.rst b/qmeventd.rst
> new file mode 100644
> index 0000000..2eefa31
> --- /dev/null
> +++ b/qmeventd.rst
> @@ -0,0 +1,38 @@
> +========
> +qmeventd
> +========
> +
> +-------------------------
> +listen to qemu qmp events
> +-------------------------
> +
> +:Author: Proxmox Support Team <support at proxmox.com>
> +:Manual section: 1
> +:Manual group: qmeventd Manual
> +
> +SYNOPSIS
> +========
> +
> +``qmeventd`` [``-f``] [``-v``] PATH
> +
> +DESCRIPTION
> +===========
> +
> +``qmeventd`` is a daemon that listens on PATH for incoming connections from
> +a qemu qmp socket, and waits for SHUTDOWN events. When a client then
> +disconnects, it executes ``/usr/sbin/qm cleanup``. This makes it easy
> +to clean up leftover tap devices, vgpus, etc.
> +
> +``-v``
> +    Be verbose about (dis)connecting clients and their messages.
> +
> +``-f``
> +    Don't daemonize and run in foreground.
> +
> +``PATH``
> +    The path to listen on.
> +
> +BUGS
> +====
> +
> +Please report bugs at https://bugzilla.proxmox.com/
> diff --git a/qmeventd.service b/qmeventd.service
> new file mode 100644
> index 0000000..42a12c1
> --- /dev/null
> +++ b/qmeventd.service
> @@ -0,0 +1,10 @@
> +[Unit]
> +Description=PVE Qemu Event Daemon
> +ConditionPathExists=/usr/sbin/qmeventd

This should be implied by the ExecStart line, no?

> +
> +[Service]
> +ExecStart=/usr/sbin/qmeventd /var/run/qemu-server/event.socket
> +Type=forking
> +
> +[Install]
> +WantedBy=multi-user.target
> -- 
> 2.11.0




More information about the pve-devel mailing list