qemu: use line breaks in command line args written to log

The QEMU command line arguments are very long and currently all written
on a single line to /var/log/libvirt/qemu/$GUEST.log. This introduces
logic to add line breaks after every env variable and "-" optional
argument, and every positional argument. This will create a clearer log
file, which will in turn present better in bug reports when people cut +
paste from the log into a bug comment.

An example log file entry now looks like this:

  2018-12-14 12:57:03.677+0000: starting up libvirt version: 5.0.0, qemu version: 3.0.0qemu-3.0.0-1.fc29, kernel: 4.19.5-300.fc29.x86_64, hostname: localhost.localdomain
  LC_ALL=C \
  PATH=/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin \
  HOME=/home/berrange \
  USER=berrange \
  LOGNAME=berrange \
  QEMU_AUDIO_DRV=none \
  /usr/bin/qemu-system-ppc64 \
  -name guest=guest,debug-threads=on \
  -S \
  -object secret,id=masterKey0,format=raw,file=/home/berrange/.config/libvirt/qemu/lib/domain-33-guest/master-key.aes \
  -machine pseries-2.10,accel=tcg,usb=off,dump-guest-core=off \
  -m 1024 \
  -realtime mlock=off \
  -smp 1,sockets=1,cores=1,threads=1 \
  -uuid c8a74977-ab18-41d0-ae3b-4041c7fffbcd \
  -display none \
  -no-user-config \
  -nodefaults \
  -chardev socket,id=charmonitor,fd=23,server,nowait \
  -mon chardev=charmonitor,id=monitor,mode=control \
  -rtc base=utc \
  -no-shutdown \
  -boot strict=on \
  -device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
  -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
  -msg timestamp=on
  2018-12-14 12:57:03.730+0000: shutting down, reason=failed

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2018-12-14 12:07:08 +00:00
parent 912c6b22fc
commit 07c9d6601d
15 changed files with 95 additions and 19 deletions

View File

@ -426,7 +426,7 @@ dprintf(logfd, "%s: ", timestamp);
VIR_FREE(timestamp);
virCommandWriteArgLog(cmd, logfd);
string = virCommandToString(cmd);
string = virCommandToString(cmd, false);
if (string)
VIR_DEBUG("about to run %s", string);
VIR_FREE(string);

View File

@ -733,14 +733,14 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn,
NULL)))
goto cleanup;
virBufferAdd(&buf, virCommandToString(loadcmd), -1);
virBufferAdd(&buf, virCommandToString(loadcmd, false), -1);
virBufferAddChar(&buf, '\n');
}
if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, true)))
goto cleanup;
virBufferAdd(&buf, virCommandToString(cmd), -1);
virBufferAdd(&buf, virCommandToString(cmd, false), -1);
if (virBufferCheckError(&buf) < 0)
goto cleanup;

View File

@ -7425,7 +7425,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
VIR_QEMU_PROCESS_START_COLD)))
goto cleanup;
ret = virCommandToString(cmd);
ret = virCommandToString(cmd, false);
cleanup:
virCommandFree(cmd);

View File

@ -360,7 +360,7 @@ qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
char ebuf[1024];
char *errstr = NULL;
if (!(cmdstr = virCommandToString(cmd)))
if (!(cmdstr = virCommandToString(cmd, false)))
goto cleanup;
virCommandAbort(cmd);

View File

@ -4353,7 +4353,7 @@ qemuLogOperation(virDomainObjPtr vm,
goto cleanup;
if (cmd) {
char *args = virCommandToString(cmd);
char *args = virCommandToString(cmd, true);
qemuDomainLogContextWrite(logCtxt, "%s\n", args);
VIR_FREE(args);
}

View File

@ -654,7 +654,7 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
if ((profile_name = get_profile_name(def)) == NULL)
goto cleanup;
cmd_str = virCommandToString(cmd);
cmd_str = virCommandToString(cmd, false);
VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name, cmd_str);
virCommandSetAppArmorProfile(cmd, profile_name);
rc = 0;

View File

@ -1960,6 +1960,7 @@ virCommandWriteArgLog(virCommandPtr cmd, int logfd)
/**
* virCommandToString:
* @cmd: the command to convert
* @linebreaks: true to break line after each env var or option
*
* Call after adding all arguments and environment settings, but
* before Run/RunAsync, to return a string representation of the
@ -1969,10 +1970,11 @@ virCommandWriteArgLog(virCommandPtr cmd, int logfd)
* Caller is responsible for freeing the resulting string.
*/
char *
virCommandToString(virCommandPtr cmd)
virCommandToString(virCommandPtr cmd, bool linebreaks)
{
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool prevopt = false;
/* Cannot assume virCommandRun will be called; so report the error
* now. If virCommandRun is called, it will report the same error. */
@ -2001,11 +2003,22 @@ virCommandToString(virCommandPtr cmd)
virBufferAdd(&buf, cmd->env[i], eq - cmd->env[i]);
virBufferEscapeShell(&buf, eq);
virBufferAddChar(&buf, ' ');
if (linebreaks)
virBufferAddLit(&buf, "\\\n");
}
virBufferEscapeShell(&buf, cmd->args[0]);
for (i = 1; i < cmd->nargs; i++) {
virBufferAddChar(&buf, ' ');
if (linebreaks) {
/* Line break if this is a --arg or if
* the previous arg was a positional option
*/
if (cmd->args[i][0] == '-' ||
!prevopt)
virBufferAddLit(&buf, "\\\n");
}
virBufferEscapeShell(&buf, cmd->args[i]);
prevopt = (cmd->args[i][0] == '-');
}
if (virBufferCheckError(&buf) < 0)
@ -2448,7 +2461,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
goto cleanup;
}
str = virCommandToString(cmd);
str = virCommandToString(cmd, false);
if (dryRunBuffer || dryRunCallback) {
dryRunStatus = 0;
if (!str) {
@ -2589,7 +2602,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
if (exitstatus && (cmd->rawStatus || WIFEXITED(status))) {
*exitstatus = cmd->rawStatus ? status : WEXITSTATUS(status);
} else if (status) {
VIR_AUTOFREE(char *) str = virCommandToString(cmd);
VIR_AUTOFREE(char *) str = virCommandToString(cmd, false);
VIR_AUTOFREE(char *) st = virProcessTranslateStatus(status);
bool haveErrMsg = cmd->errbuf && *cmd->errbuf && (*cmd->errbuf)[0];

View File

@ -172,7 +172,7 @@ void virCommandSetPreExecHook(virCommandPtr cmd,
void virCommandWriteArgLog(virCommandPtr cmd,
int logfd);
char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
char *virCommandToString(virCommandPtr cmd, bool linebreaks) ATTRIBUTE_RETURN_CHECK;
int virCommandExec(virCommandPtr cmd, gid_t *groups, int ngroups) ATTRIBUTE_RETURN_CHECK;

View File

@ -694,7 +694,7 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule,
if (ignoreErrors) {
VIR_DEBUG("Ignoring error running command");
} else {
VIR_AUTOFREE(char *) args = virCommandToString(cmd);
VIR_AUTOFREE(char *) args = virCommandToString(cmd, false);
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to apply firewall rules %s: %s"),
NULLSTR(args), NULLSTR(error));

View File

@ -68,13 +68,13 @@ static int testCompareXMLToArgvFiles(const char *xml,
goto out;
}
if (!(actualargv = virCommandToString(cmd)))
if (!(actualargv = virCommandToString(cmd, false)))
goto out;
if (actualdm != NULL)
virTrimSpaces(actualdm, NULL);
if (!(actualld = virCommandToString(ldcmd)))
if (!(actualld = virCommandToString(ldcmd, false)))
goto out;
if (virTestCompareToFile(actualargv, cmdline) < 0)

View File

@ -0,0 +1 @@
A=B C=D E true --foo bar --oooh -f --wizz eek eek -w -z -l --mmm flash bang wallop

View File

@ -630,7 +630,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED)
virCommandAddArg(cmd, "F");
virCommandAddArg(cmd, "G H");
if ((outactual = virCommandToString(cmd)) == NULL) {
if ((outactual = virCommandToString(cmd, false)) == NULL) {
printf("Cannot convert to string: %s\n", virGetLastErrorMessage());
goto cleanup;
}
@ -1135,6 +1135,67 @@ static int test25(const void *unused ATTRIBUTE_UNUSED)
}
/*
* Don't run program; rather, log what would be run.
*/
static int test26(const void *unused ATTRIBUTE_UNUSED)
{
virCommandPtr cmd = virCommandNew("true");
char *outactual = NULL;
const char *outexpect =
"A=B \\\n"
"C='D E' \\\n"
"true \\\n"
"--foo bar \\\n"
"--oooh \\\n"
"-f \\\n"
"--wizz 'eek eek' \\\n"
"-w \\\n"
"-z \\\n"
"-l \\\n"
"--mmm flash \\\n"
"bang \\\n"
"wallop";
int ret = -1;
int fd = -1;
virCommandAddEnvPair(cmd, "A", "B");
virCommandAddEnvPair(cmd, "C", "D E");
virCommandAddArgList(cmd, "--foo", "bar", "--oooh", "-f",
"--wizz", "eek eek", "-w", "-z", "-l",
"--mmm", "flash", "bang", "wallop",
NULL);
if ((outactual = virCommandToString(cmd, true)) == NULL) {
printf("Cannot convert to string: %s\n", virGetLastErrorMessage());
goto cleanup;
}
if ((fd = open(abs_builddir "/commandhelper.log",
O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) {
printf("Cannot open log file: %s\n", strerror(errno));
goto cleanup;
}
virCommandWriteArgLog(cmd, fd);
if (VIR_CLOSE(fd) < 0) {
printf("Cannot close log file: %s\n", strerror(errno));
goto cleanup;
}
if (STRNEQ(outactual, outexpect)) {
virTestDifference(stderr, outexpect, outactual);
goto cleanup;
}
ret = checkoutput("test26", NULL);
cleanup:
virCommandFree(cmd);
VIR_FORCE_CLOSE(fd);
VIR_FREE(outactual);
return ret;
}
static void virCommandThreadWorker(void *opaque)
{
virCommandTestDataPtr test = opaque;
@ -1288,6 +1349,7 @@ mymain(void)
DO_TEST(test23);
DO_TEST(test24);
DO_TEST(test25);
DO_TEST(test26);
virMutexLock(&test->lock);
if (test->running) {

View File

@ -620,7 +620,7 @@ testCompareXMLToArgv(const void *data)
goto cleanup;
}
if (!(actualargv = virCommandToString(cmd)))
if (!(actualargv = virCommandToString(cmd, false)))
goto cleanup;
if (virTestCompareToFile(actualargv, args) < 0)

View File

@ -71,7 +71,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
goto cleanup;
};
if (!(actualCmdline = virCommandToString(cmd))) {
if (!(actualCmdline = virCommandToString(cmd, false))) {
VIR_TEST_DEBUG("pool type %d failed to get commandline\n", def->type);
goto cleanup;
}

View File

@ -104,14 +104,14 @@ testCompareXMLToArgvFiles(bool shouldFail,
}
if (convertStep != VIR_STORAGE_VOL_ENCRYPT_CONVERT) {
if (!(actualCmdline = virCommandToString(cmd)))
if (!(actualCmdline = virCommandToString(cmd, false)))
goto cleanup;
} else {
char *createCmdline = actualCmdline;
char *cvtCmdline;
int rc;
if (!(cvtCmdline = virCommandToString(cmd)))
if (!(cvtCmdline = virCommandToString(cmd, false)))
goto cleanup;
rc = virAsprintf(&actualCmdline, "%s\n%s",