-----Original Message-----
From: Michael Kelley (LINUX) <redacted>
Sent: Thursday, August 3, 2023 3:15 AM
To: Saurabh Sengar <ssengar@linux.microsoft.com>; KY Srinivasan
[off-list ref]; Haiyang Zhang [off-list ref];
wei.liu@kernel.org; Dexuan Cui [off-list ref];
gregkh@linuxfoundation.org; corbet@lwn.net; linux-kernel@vger.kernel.org;
linux-hyperv@vger.kernel.org; linux-doc@vger.kernel.org
Subject: [EXTERNAL] RE: [PATCH v3 3/3] tools: hv: Add new fcopy application
based on uio driver

From: Saurabh Sengar <ssengar@linux.microsoft.com> Sent: Friday, July 14,
2023 3:26 AM

quoted

Implement the file copy service for Linux guests on Hyper-V. This
permits the host to copy a file (over VMBus) into the guest. This
facility is part of "guest integration services" supported on the
Hyper-V platform.

Here is a link that provides additional details on this functionality:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flear
n.microsoft.com%2Fen-us%2Fpowershell%2Fmodule%2Fhyper-v%2Fcopy-

vmfile%

quoted

3Fview%3Dwindowsserver2022-

ps&data=05%7C01%7Cssengar%40microsoft.com%7

quoted

Ca5edc1b9d6574e2e6e3108db93a1c558%7C72f988bf86f141af91ab2d7cd011
db47%7

quoted

C1%7C0%7C638266095311741847%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
MC4wLjAwMD

quoted

AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
&sdata

quoted

=VudSwIKYJJJgPKxNbbfCnOjia1lfKCdijnSn94OWm8Q%3D&reserved=0

This new fcopy application uses uio_hv_vmbus_client driver which makes
the earlier hv_util based driver and application obsolete.

Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
[V3]
- Improve cover letter and commit messages
- Improve debug prints
- Instead of hardcoded instance id, query from class id sysfs
- Set the ring_size value from application
- Update the application to mmap /dev/uio instead of sysfs
- new application compilation dependent on x86

[V2]
- simpler sysfs path

 tools/hv/Build                 |   1 +
 tools/hv/Makefile              |  10 +-
 tools/hv/hv_fcopy_uio_daemon.c | 578
+++++++++++++++++++++++++++++++++
 3 files changed, 588 insertions(+), 1 deletion(-)  create mode 100644
tools/hv/hv_fcopy_uio_daemon.c

diff --git a/tools/hv/Build b/tools/hv/Build index

2a667d3d94cb..efcbb74a0d23 100644

--- a/tools/hv/Build
+++ b/tools/hv/Build

@@ -2,3 +2,4 @@ hv_kvp_daemon-y += hv_kvp_daemon.o

hv_vss_daemon-y +=

quoted

hv_vss_daemon.o  hv_fcopy_daemon-y += hv_fcopy_daemon.o
vmbus_bufring-y += vmbus_bufring.o
+hv_fcopy_uio_daemon-y += hv_fcopy_uio_daemon.o

diff --git a/tools/hv/Makefile b/tools/hv/Makefile index

33cf488fd20f..678c6c450a53 100644

--- a/tools/hv/Makefile
+++ b/tools/hv/Makefile

@@ -21,8 +21,10 @@ override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -

I$(OUTPUT)include

 ifeq ($(SRCARCH),x86)
 ALL_LIBS := libvmbus_bufring.a
-endif
+ALL_TARGETS := hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon
hv_fcopy_uio_daemon
+else
 ALL_TARGETS := hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon
+endif
 ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS)) $(patsubst
%,$(OUTPUT)%,$(ALL_LIBS))

 ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh
hv_set_ifconfig.sh @@ -56,6 +58,12 @@ $(HV_FCOPY_DAEMON_IN):

FORCE

quoted

 $(OUTPUT)hv_fcopy_daemon: $(HV_FCOPY_DAEMON_IN)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@

+HV_FCOPY_UIO_DAEMON_IN := $(OUTPUT)hv_fcopy_uio_daemon-in.o
+$(HV_FCOPY_UIO_DAEMON_IN): FORCE
+	$(Q)$(MAKE) $(build)=hv_fcopy_uio_daemon
+$(OUTPUT)hv_fcopy_uio_daemon: $(HV_FCOPY_UIO_DAEMON_IN)
libvmbus_bufring.a
+	$(QUIET_LINK)$(CC) -lm $< -L. -lvmbus_bufring -o $@
+
 clean:
 	rm -f $(ALL_PROGRAMS)
 	find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.d' -delete

diff --git a/tools/hv/hv_fcopy_uio_daemon.c

b/tools/hv/hv_fcopy_uio_daemon.c new file mode 100644 index
000000000000..e8618a30dc7e

--- /dev/null
+++ b/tools/hv/hv_fcopy_uio_daemon.c

@@ -0,0 +1,578 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * An implementation of host to guest copy functionality for Linux.
+ *
+ * Copyright (C) 2023, Microsoft, Inc.
+ *
+ * Author : K. Y. Srinivasan <kys@microsoft.com>
+ * Author : Saurabh Sengar <ssengar@microsoft.com>
+ *
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <locale.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <linux/hyperv.h>
+#include "vmbus_bufring.h"
+
+#define ICMSGTYPE_NEGOTIATE	0
+#define ICMSGTYPE_FCOPY		7
+
+#define WIN8_SRV_MAJOR		1
+#define WIN8_SRV_MINOR		1
+#define WIN8_SRV_VERSION	(WIN8_SRV_MAJOR << 16 |

WIN8_SRV_MINOR)

quoted

+
+#define MAX_PATH_LEN		300
+#define MAX_LINE_LEN		40
+#define DEVICES_SYSFS		"/sys/bus/vmbus/devices"
+#define FCOPY_CLASS_ID		"34d14be3-dee4-41c8-9ae7-

6b174977c192"

quoted

+
+#define FCOPY_VER_COUNT		1
+static const int fcopy_versions[] = {
+	WIN8_SRV_VERSION
+};
+
+#define FW_VER_COUNT		1
+static const int fw_versions[] = {
+	UTIL_FW_VERSION
+};
+
+#define HV_RING_SIZE		(4 * 4096)
+
+unsigned char desc[HV_RING_SIZE];
+
+static int target_fd;
+static char target_fname[PATH_MAX];
+static unsigned long long filesize;
+
+static int hv_fcopy_create_file(char *file_name, char *path_name,
+__u32 flags) {
+	int error = HV_E_FAIL;
+	char *q, *p;
+
+	filesize = 0;
+	p = (char *)path_name;
+	snprintf(target_fname, sizeof(target_fname), "%s/%s",
+		 (char *)path_name, (char *)file_name);
+
+	/*
+	 * Check to see if the path is already in place; if not,
+	 * create if required.
+	 */
+	while ((q = strchr(p, '/')) != NULL) {
+		if (q == p) {
+			p++;
+			continue;
+		}
+		*q = '\0';
+		if (access(path_name, F_OK)) {
+			if (flags & CREATE_PATH) {
+				if (mkdir(path_name, 0755)) {
+					syslog(LOG_ERR, "Failed to create

%s",

quoted

+					       path_name);
+					goto done;
+				}
+			} else {
+				syslog(LOG_ERR, "Invalid path: %s",

path_name);

quoted

+				goto done;
+			}
+		}
+		p = q + 1;
+		*q = '/';
+	}
+
+	if (!access(target_fname, F_OK)) {
+		syslog(LOG_INFO, "File: %s exists", target_fname);
+		if (!(flags & OVER_WRITE)) {
+			error = HV_ERROR_ALREADY_EXISTS;
+			goto done;
+		}
+	}
+
+	target_fd = open(target_fname,
+			 O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC,

0744);

quoted

+	if (target_fd == -1) {
+		syslog(LOG_INFO, "Open Failed: %s", strerror(errno));
+		goto done;
+	}
+
+	error = 0;
+done:
+	if (error)
+		target_fname[0] = '\0';
+	return error;
+}
+
+static int hv_copy_data(struct hv_do_fcopy *cpmsg) {
+	ssize_t bytes_written;
+	int ret = 0;
+
+	bytes_written = pwrite(target_fd, cpmsg->data, cpmsg->size,
+			       cpmsg->offset);
+
+	filesize += cpmsg->size;
+	if (bytes_written != cpmsg->size) {
+		switch (errno) {
+		case ENOSPC:
+			ret = HV_ERROR_DISK_FULL;
+			break;
+		default:
+			ret = HV_E_FAIL;
+			break;
+		}
+		syslog(LOG_ERR, "pwrite failed to write %llu bytes: %ld (%s)",
+		       filesize, (long)bytes_written, strerror(errno));
+	}
+
+	return ret;
+}
+
+/*
+ * Reset target_fname to "" in the two below functions for
+hibernation: if
+ * the fcopy operation is aborted by hibernation, the daemon should
+remove the
+ * partially-copied file; to achieve this, the hv_utils driver always
+fakes a
+ * CANCEL_FCOPY message upon suspend, and later when the VM

resumes

quoted

+back,
+ * the daemon calls hv_copy_cancel() to remove the file; if a file is
+copied
+ * successfully before suspend, hv_copy_finished() must reset
+target_fname to
+ * avoid that the file can be incorrectly removed upon resume, since
+the faked
+ * CANCEL_FCOPY message is spurious in this case.
+ */
+static int hv_copy_finished(void)
+{
+	close(target_fd);
+	target_fname[0] = '\0';
+	return 0;
+}
+
+static void print_usage(char *argv[]) {
+	fprintf(stderr, "Usage: %s [options]\n"
+		"Options are:\n"
+		"  -n, --no-daemon        stay in foreground, don't

daemonize\n"

quoted

+		"  -h, --help             print this help\n", argv[0]);
+}
+
+static bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,

unsigned char *buf,

quoted

+				      unsigned int buflen, const int *fw_version,

int fw_vercnt,

quoted

+				const int *srv_version, int srv_vercnt,
+				int *nego_fw_version, int *nego_srv_version)

{

quoted

+	int icframe_major, icframe_minor;
+	int icmsg_major, icmsg_minor;
+	int fw_major, fw_minor;
+	int srv_major, srv_minor;
+	int i, j;
+	bool found_match = false;
+	struct icmsg_negotiate *negop;
+
+	/* Check that there's enough space for icframe_vercnt, icmsg_vercnt

*/

quoted

+	if (buflen < ICMSG_HDR + offsetof(struct icmsg_negotiate, reserved)) {
+		syslog(LOG_ERR, "Invalid icmsg negotiate");
+		return false;
+	}
+
+	icmsghdrp->icmsgsize = 0x10;
+	negop = (struct icmsg_negotiate *)&buf[ICMSG_HDR];
+
+	icframe_major = negop->icframe_vercnt;
+	icframe_minor = 0;
+
+	icmsg_major = negop->icmsg_vercnt;
+	icmsg_minor = 0;
+
+	/* Validate negop packet */
+	if (icframe_major > IC_VERSION_NEGOTIATION_MAX_VER_COUNT ||
+	    icmsg_major > IC_VERSION_NEGOTIATION_MAX_VER_COUNT ||
+	    ICMSG_NEGOTIATE_PKT_SIZE(icframe_major, icmsg_major) >

buflen) {

quoted

+		syslog(LOG_ERR, "Invalid icmsg negotiate - icframe_major:

%u, icmsg_major: %u\n",

quoted

+		       icframe_major, icmsg_major);
+		goto fw_error;
+	}
+
+	/*
+	 * Select the framework version number we will
+	 * support.
+	 */
+
+	for (i = 0; i < fw_vercnt; i++) {
+		fw_major = (fw_version[i] >> 16);
+		fw_minor = (fw_version[i] & 0xFFFF);
+
+		for (j = 0; j < negop->icframe_vercnt; j++) {
+			if (negop->icversion_data[j].major == fw_major &&
+			    negop->icversion_data[j].minor == fw_minor) {
+				icframe_major = negop-
icversion_data[j].major;
+				icframe_minor = negop-
icversion_data[j].minor;
+				found_match = true;
+				break;
+			}
+		}
+
+		if (found_match)
+			break;
+	}
+
+	if (!found_match)
+		goto fw_error;
+
+	found_match = false;
+
+	for (i = 0; i < srv_vercnt; i++) {
+		srv_major = (srv_version[i] >> 16);
+		srv_minor = (srv_version[i] & 0xFFFF);
+
+		for (j = negop->icframe_vercnt;
+			(j < negop->icframe_vercnt + negop->icmsg_vercnt);
+			j++) {
+			if (negop->icversion_data[j].major == srv_major &&
+			    negop->icversion_data[j].minor == srv_minor) {
+				icmsg_major = negop-
icversion_data[j].major;
+				icmsg_minor = negop-
icversion_data[j].minor;
+				found_match = true;
+				break;
+			}
+		}
+
+		if (found_match)
+			break;
+	}
+
+	/*
+	 * Respond with the framework and service
+	 * version numbers we can support.
+	 */
+fw_error:
+	if (!found_match) {
+		negop->icframe_vercnt = 0;
+		negop->icmsg_vercnt = 0;
+	} else {
+		negop->icframe_vercnt = 1;
+		negop->icmsg_vercnt = 1;
+	}
+
+	if (nego_fw_version)
+		*nego_fw_version = (icframe_major << 16) | icframe_minor;
+
+	if (nego_srv_version)
+		*nego_srv_version = (icmsg_major << 16) | icmsg_minor;
+
+	negop->icversion_data[0].major = icframe_major;
+	negop->icversion_data[0].minor = icframe_minor;
+	negop->icversion_data[1].major = icmsg_major;
+	negop->icversion_data[1].minor = icmsg_minor;
+
+	return found_match;
+}
+
+static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
+{
+	size_t len = 0;
+
+	while (len < dest_size) {
+		if (src[len] < 0x80)
+			dest[len++] = (char)(*src++);
+		else
+			dest[len++] = 'X';
+	}
+
+	dest[len] = '\0';
+}
+
+static int hv_fcopy_start(struct hv_start_fcopy *smsg_in) {
+	setlocale(LC_ALL, "en_US.utf8");
+	size_t file_size, path_size;
+	char *file_name, *path_name;
+	char *in_file_name = (char *)smsg_in->file_name;
+	char *in_path_name = (char *)smsg_in->path_name;
+
+	file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) +

1;

quoted

+	path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name,

0)

quoted

++ 1;
+
+	file_name = (char *)malloc(file_size * sizeof(char));
+	path_name = (char *)malloc(path_size * sizeof(char));
+
+	wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
+	wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+
+	return hv_fcopy_create_file(file_name, path_name,
+smsg_in->copy_flags); }
+
+static int hv_fcopy_send_data(struct hv_fcopy_hdr *fcopy_msg, int
+recvlen) {
+	int operation = fcopy_msg->operation;
+
+	/*
+	 * The  strings sent from the host are encoded in
+	 * utf16; convert it to utf8 strings.
+	 * The host assures us that the utf16 strings will not exceed
+	 * the max lengths specified. We will however, reserve room
+	 * for the string terminating character - in the utf16s_utf8s()
+	 * function we limit the size of the buffer where the converted
+	 * string is placed to W_MAX_PATH -1 to guarantee
+	 * that the strings can be properly terminated!
+	 */
+
+	switch (operation) {
+	case START_FILE_COPY:
+		return hv_fcopy_start((struct hv_start_fcopy *)fcopy_msg);
+	case WRITE_TO_FILE:
+		return hv_copy_data((struct hv_do_fcopy *)fcopy_msg);
+	case COMPLETE_FCOPY:
+		return hv_copy_finished();
+	}
+
+	return HV_E_FAIL;
+}
+
+/* process the packet recv from host */ static int
+fcopy_pkt_process(struct vmbus_br *txbr) {
+	int ret, offset, pktlen;
+	int fcopy_srv_version;
+	const struct vmbus_chanpkt_hdr *pkt;
+	struct hv_fcopy_hdr *fcopy_msg;
+	struct icmsg_hdr *icmsghdr;
+
+	pkt = (const struct vmbus_chanpkt_hdr *)desc;
+	offset = pkt->hlen << 3;
+	pktlen = (pkt->tlen << 3) - offset;
+	icmsghdr = (struct icmsg_hdr *)&desc[offset + sizeof(struct

vmbuspipe_hdr)];

quoted

+	icmsghdr->status = HV_E_FAIL;
+
+	if (icmsghdr->icmsgtype == ICMSGTYPE_NEGOTIATE) {
+		if (vmbus_prep_negotiate_resp(icmsghdr, desc + offset,

pktlen, fw_versions,

quoted

+					      FW_VER_COUNT, fcopy_versions,

FCOPY_VER_COUNT,

quoted

+					      NULL, &fcopy_srv_version)) {
+			syslog(LOG_INFO, "FCopy IC version %d.%d",
+			       fcopy_srv_version >> 16, fcopy_srv_version &

0xFFFF);

quoted

+			icmsghdr->status = 0;
+		}
+	} else if (icmsghdr->icmsgtype == ICMSGTYPE_FCOPY) {
+		/* Ensure recvlen is big enough to contain hv_fcopy_hdr */
+		if (pktlen < ICMSG_HDR + sizeof(struct hv_fcopy_hdr)) {
+			syslog(LOG_ERR, "Invalid Fcopy hdr. Packet length too

small: %u",

quoted

+			       pktlen);
+			return -ENOBUFS;
+		}
+
+		fcopy_msg = (struct hv_fcopy_hdr *)&desc[offset +

ICMSG_HDR];

quoted

+		icmsghdr->status = hv_fcopy_send_data(fcopy_msg, pktlen);
+	}
+
+	icmsghdr->icflags = ICMSGHDRFLAG_TRANSACTION |

ICMSGHDRFLAG_RESPONSE;

quoted

+	ret = rte_vmbus_chan_send(txbr, 0x6, desc + offset, pktlen, 0);
+	if (ret) {
+		syslog(LOG_ERR, "Write to ringbuffer failed err: %d", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void fcopy_get_first_folder(char *path, char *chan_no) {
+	DIR *dir = opendir(path);
+	struct dirent *entry;
+
+	if (!dir) {
+		syslog(LOG_ERR, "Failed to open directory (errno=%s).\n",

strerror(errno));

quoted

+		return;
+	}
+
+	while ((entry = readdir(dir)) != NULL) {
+		if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".")

!= 0 &&

quoted

+		    strcmp(entry->d_name, "..") != 0) {
+			strcpy(chan_no, entry->d_name);
+			break;
+		}
+	}
+
+	closedir(dir);
+}
+
+static void fcopy_set_ring_size(char *path, char *inst, int size) {
+	char ring_size_path[MAX_PATH_LEN] = {0};
+	FILE *fd;
+
+	snprintf(ring_size_path, sizeof(ring_size_path), "%s/%s/%s", path, inst,

"ring_size");

quoted

+	fd = fopen(ring_size_path, "w");
+	if (!fd) {
+		syslog(LOG_WARNING, "Failed to open ring_size file

(errno=%s).\n", strerror(errno));

quoted

+		return;
+	}
+	fprintf(fd, "%d", size);

Check for and log an error if the new value isn't accepted by the kernel
driver?
The code is using a ring size value that should be accepted by the kernel
driver, but weird stuff happens and it's probably better to know about it.

Ok, will add error check.

quoted

+	fclose(fd);
+}
+
+static char *fcopy_read_sysfs(char *path, char *buf, int len) {
+	FILE *fd;
+	char *ret;
+
+	fd = fopen(path, "r");
+	if (!fd)
+		return NULL;
+
+	ret = fgets(buf, len, fd);
+	fclose(fd);
+
+	return ret;
+}
+
+static int fcopy_get_instance_id(char *path, char *class_id, char
+*inst) {
+	DIR *dir = opendir(path);
+	struct dirent *entry;
+	char tmp_path[MAX_PATH_LEN] = {0};
+	char line[MAX_LINE_LEN];
+
+	if (!dir) {
+		syslog(LOG_ERR, "Failed to open directory (errno=%s).\n",

strerror(errno));

quoted

+		return -EINVAL;
+	}
+
+	while ((entry = readdir(dir)) != NULL) {
+		if (entry->d_type == DT_LNK && strcmp(entry->d_name, ".")

!= 0 &&

quoted

+		    strcmp(entry->d_name, "..") != 0) {
+			/* search for the sysfs path with matching class_id */
+			snprintf(tmp_path, sizeof(tmp_path), "%s/%s/%s",
+				 path, entry->d_name, "class_id");
+			if (!fcopy_read_sysfs(tmp_path, line, MAX_LINE_LEN))
+				continue;
+
+			/* class id matches, now fetch the instance id from

device_id */

quoted

+			if (strstr(line, class_id)) {
+				snprintf(tmp_path, sizeof(tmp_path),

"%s/%s/%s",

quoted

+					 path, entry->d_name, "device_id");
+				if (!fcopy_read_sysfs(tmp_path, line,

MAX_LINE_LEN))

quoted

+					continue;
+				/* remove braces */
+				strncpy(inst, line + 1, strlen(line) - 3);
+				break;
+			}
+		}
+	}
+
+	closedir(dir);
+	return 0;

If this function doesn't find a matching class_id, it appears that it returns 0,
but with the "inst" parameter unset.  The caller will then proceed as if "inst"
was set when it is actually an uninitialized stack variable.  Probably need
some better error detection and handling.

Good point !
Let me fix it in next version.

quoted

+}
+
+int main(int argc, char *argv[])
+{
+	int fcopy_fd = -1, tmp = 1;
+	int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
+	struct vmbus_br txbr, rxbr;
+	void *ring;
+	uint32_t len = HV_RING_SIZE;
+	char uio_name[10] = {0};
+	char uio_dev_path[15] = {0};
+	char uio_path[MAX_PATH_LEN] = {0};
+	char inst[MAX_LINE_LEN] = {0};
+
+	static struct option long_options[] = {
+		{"help",	no_argument,	   0,  'h' },
+		{"no-daemon",	no_argument,	   0,  'n' },
+		{0,		0,		   0,  0   }
+	};
+
+	while ((opt = getopt_long(argc, argv, "hn", long_options,
+				  &long_index)) != -1) {
+		switch (opt) {
+		case 'n':
+			daemonize = 0;
+			break;
+		case 'h':
+		default:
+			print_usage(argv);
+			exit(EXIT_FAILURE);
+		}
+	}
+
+	if (daemonize && daemon(1, 0)) {
+		syslog(LOG_ERR, "daemon() failed; error: %s",

strerror(errno));

quoted

+		exit(EXIT_FAILURE);
+	}
+
+	openlog("HV_UIO_FCOPY", 0, LOG_USER);
+	syslog(LOG_INFO, "starting; pid is:%d", getpid());
+
+	/* get instance id */
+	if (fcopy_get_instance_id(DEVICES_SYSFS, FCOPY_CLASS_ID, inst))
+		exit(EXIT_FAILURE);

Per above, need better error handling.  And since the syslog is now open, any
errors should be logged rather than having the process just mysteriously exit.

OK.

- Saurabh