From nobody Fri May 3 04:17:20 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1544792840491900.192421188365; Fri, 14 Dec 2018 05:07:20 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 816FC3078A5D; Fri, 14 Dec 2018 13:07:18 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1A7A467144; Fri, 14 Dec 2018 13:07:18 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 6401F181B9F6; Fri, 14 Dec 2018 13:07:17 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id wBED7G8T030851 for ; Fri, 14 Dec 2018 08:07:16 -0500 Received: by smtp.corp.redhat.com (Postfix) id 57FE366D46; Fri, 14 Dec 2018 13:07:16 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-19.ams2.redhat.com [10.36.112.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 10CCD60BF6; Fri, 14 Dec 2018 13:07:12 +0000 (UTC) From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Date: Fri, 14 Dec 2018 13:07:10 +0000 Message-Id: <20181214130710.16442-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH] qemu: use line breaks in command line args written to log X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Fri, 14 Dec 2018 13:07:19 +0000 (UTC) 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 ve= rsion: 3.0.0qemu-3.0.0-1.fc29, kernel: 4.19.5-300.fc29.x86_64, hostname: lo= calhost.localdomain LC_ALL=3DC \ PATH=3D/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin \ HOME=3D/home/berrange \ USER=3Dberrange \ LOGNAME=3Dberrange \ QEMU_AUDIO_DRV=3Dnone \ /usr/bin/qemu-system-ppc64 \ -name guest=3Dguest,debug-threads=3Don \ -S \ -object secret,id=3DmasterKey0,format=3Draw,file=3D/home/berrange/.config= /libvirt/qemu/lib/domain-33-guest/master-key.aes \ -machine pseries-2.10,accel=3Dtcg,usb=3Doff,dump-guest-core=3Doff \ -m 1024 \ -realtime mlock=3Doff \ -smp 1,sockets=3D1,cores=3D1,threads=3D1 \ -uuid c8a74977-ab18-41d0-ae3b-4041c7fffbcd \ -display none \ -no-user-config \ -nodefaults \ -chardev socket,id=3Dcharmonitor,fd=3D23,server,nowait \ -mon chardev=3Dcharmonitor,id=3Dmonitor,mode=3Dcontrol \ -rtc base=3Dutc \ -no-shutdown \ -boot strict=3Don \ -device qemu-xhci,id=3Dusb,bus=3Dpci.0,addr=3D0x1 \ -device virtio-balloon-pci,id=3Dballoon0,bus=3Dpci.0,addr=3D0x2 \ -sandbox on,obsolete=3Ddeny,elevateprivileges=3Ddeny,spawn=3Ddeny,resourc= econtrol=3Ddeny \ -msg timestamp=3Don 2018-12-14 12:57:03.730+0000: shutting down, reason=3Dfailed Signed-off-by: Daniel P. Berrang=C3=A9 --- docs/internals/command.html.in | 2 +- src/bhyve/bhyve_driver.c | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_interface.c | 2 +- src/qemu/qemu_process.c | 2 +- src/security/security_apparmor.c | 2 +- src/util/vircommand.c | 19 ++++++++-- src/util/vircommand.h | 2 +- src/util/virfirewall.c | 2 +- tests/bhyvexml2argvtest.c | 4 +- tests/commanddata/test26.log | 1 + tests/commandtest.c | 64 +++++++++++++++++++++++++++++++- tests/qemuxml2argvtest.c | 2 +- tests/storagepoolxml2argvtest.c | 2 +- tests/storagevolxml2argvtest.c | 4 +- 15 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 tests/commanddata/test26.log diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index 43f51a49bb..8a9061e70f 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -426,7 +426,7 @@ dprintf(logfd, "%s: ", timestamp); VIR_FREE(timestamp); virCommandWriteArgLog(cmd, logfd); =20 -string =3D virCommandToString(cmd); +string =3D virCommandToString(cmd, false); if (string) VIR_DEBUG("about to run %s", string); VIR_FREE(string); diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4998100bc2..912797cfcf 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -733,14 +733,14 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, NULL))) goto cleanup; =20 - virBufferAdd(&buf, virCommandToString(loadcmd), -1); + virBufferAdd(&buf, virCommandToString(loadcmd, false), -1); virBufferAddChar(&buf, '\n'); } =20 if (!(cmd =3D virBhyveProcessBuildBhyveCmd(conn, def, true))) goto cleanup; =20 - virBufferAdd(&buf, virCommandToString(cmd), -1); + virBufferAdd(&buf, virCommandToString(cmd, false), -1); =20 if (virBufferCheckError(&buf) < 0) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f9a22f8a4b..67b6f9097b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7425,7 +7425,7 @@ static char *qemuConnectDomainXMLToNative(virConnectP= tr conn, VIR_QEMU_PROCESS_START_COLD))) goto cleanup; =20 - ret =3D virCommandToString(cmd); + ret =3D virCommandToString(cmd, false); =20 cleanup: virCommandFree(cmd); diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index cea1fd3046..ac0a17acc3 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -368,7 +368,7 @@ qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr= cfg, char ebuf[1024]; char *errstr =3D NULL; =20 - if (!(cmdstr =3D virCommandToString(cmd))) + if (!(cmdstr =3D virCommandToString(cmd, false))) goto cleanup; virCommandAbort(cmd); =20 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6de510660a..8d8145d18f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4360,7 +4360,7 @@ qemuLogOperation(virDomainObjPtr vm, goto cleanup; =20 if (cmd) { - char *args =3D virCommandToString(cmd); + char *args =3D virCommandToString(cmd, true); qemuDomainLogContextWrite(logCtxt, "%s\n", args); VIR_FREE(args); } diff --git a/src/security/security_apparmor.c b/src/security/security_appar= mor.c index 0d28cae0b7..43310361ba 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -654,7 +654,7 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager= Ptr mgr ATTRIBUTE_UNUSED, if ((profile_name =3D get_profile_name(def)) =3D=3D NULL) goto cleanup; =20 - cmd_str =3D virCommandToString(cmd); + cmd_str =3D virCommandToString(cmd, false); VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name, cmd_s= tr); virCommandSetAppArmorProfile(cmd, profile_name); rc =3D 0; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 6f106df33b..3559f4bafa 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1950,6 +1950,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 @@ -1959,10 +1960,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 =3D VIR_BUFFER_INITIALIZER; + bool prevopt =3D false; =20 /* Cannot assume virCommandRun will be called; so report the error * now. If virCommandRun is called, it will report the same error. */ @@ -1991,11 +1993,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 =3D 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] =3D=3D '-' || + !prevopt) + virBufferAddLit(&buf, "\\\n"); + } virBufferEscapeShell(&buf, cmd->args[i]); + prevopt =3D (cmd->args[i][0] =3D=3D '-'); } =20 if (virBufferCheckError(&buf) < 0) @@ -2438,7 +2451,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) goto cleanup; } =20 - str =3D virCommandToString(cmd); + str =3D virCommandToString(cmd, false); if (dryRunBuffer || dryRunCallback) { dryRunStatus =3D 0; if (!str) { @@ -2579,7 +2592,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) if (exitstatus && (cmd->rawStatus || WIFEXITED(status))) { *exitstatus =3D cmd->rawStatus ? status : WEXITSTATUS(status); } else if (status) { - VIR_AUTOFREE(char *) str =3D virCommandToString(cmd); + VIR_AUTOFREE(char *) str =3D virCommandToString(cmd, false); VIR_AUTOFREE(char *) st =3D virProcessTranslateStatus(status); bool haveErrMsg =3D cmd->errbuf && *cmd->errbuf && (*cmd->errb= uf)[0]; =20 diff --git a/src/util/vircommand.h b/src/util/vircommand.h index f299acc775..dbf5041890 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -172,7 +172,7 @@ void virCommandSetPreExecHook(virCommandPtr cmd, void virCommandWriteArgLog(virCommandPtr cmd, int logfd); =20 -char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK; +char *virCommandToString(virCommandPtr cmd, bool linebreaks) ATTRIBUTE_RET= URN_CHECK; =20 int virCommandExec(virCommandPtr cmd, gid_t *groups, int ngroups) ATTRIBUT= E_RETURN_CHECK; =20 diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index c25d2ddfad..5a0cf95a44 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -694,7 +694,7 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule, if (ignoreErrors) { VIR_DEBUG("Ignoring error running command"); } else { - VIR_AUTOFREE(char *) args =3D virCommandToString(cmd); + VIR_AUTOFREE(char *) args =3D virCommandToString(cmd, false); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to apply firewall rules %s: %s"), NULLSTR(args), NULLSTR(error)); diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 6d5f19e2c6..d1b486fa64 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -68,13 +68,13 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } =20 - if (!(actualargv =3D virCommandToString(cmd))) + if (!(actualargv =3D virCommandToString(cmd, false))) goto out; =20 if (actualdm !=3D NULL) virTrimSpaces(actualdm, NULL); =20 - if (!(actualld =3D virCommandToString(ldcmd))) + if (!(actualld =3D virCommandToString(ldcmd, false))) goto out; =20 if (virTestCompareToFile(actualargv, cmdline) < 0) diff --git a/tests/commanddata/test26.log b/tests/commanddata/test26.log new file mode 100644 index 0000000000..db0d424875 --- /dev/null +++ b/tests/commanddata/test26.log @@ -0,0 +1 @@ +A=3DB C=3DD E true --foo bar --oooh -f --wizz eek eek -w -z -l --mmm flas= h bang wallop diff --git a/tests/commandtest.c b/tests/commandtest.c index 4d9a468db2..816a70860f 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -630,7 +630,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) virCommandAddArg(cmd, "F"); virCommandAddArg(cmd, "G H"); =20 - if ((outactual =3D virCommandToString(cmd)) =3D=3D NULL) { + if ((outactual =3D virCommandToString(cmd, false)) =3D=3D NULL) { printf("Cannot convert to string: %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -1135,6 +1135,67 @@ static int test25(const void *unused ATTRIBUTE_UNUSE= D) } =20 =20 +/* + * Don't run program; rather, log what would be run. + */ +static int test26(const void *unused ATTRIBUTE_UNUSED) +{ + virCommandPtr cmd =3D virCommandNew("true"); + char *outactual =3D NULL; + const char *outexpect =3D + "A=3DB \\\n" + "C=3D'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 =3D -1; + int fd =3D -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 =3D virCommandToString(cmd, true)) =3D=3D NULL) { + printf("Cannot convert to string: %s\n", virGetLastErrorMessage()); + goto cleanup; + } + if ((fd =3D 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 =3D checkoutput("test26", NULL); + + cleanup: + virCommandFree(cmd); + VIR_FORCE_CLOSE(fd); + VIR_FREE(outactual); + return ret; +} + static void virCommandThreadWorker(void *opaque) { virCommandTestDataPtr test =3D opaque; @@ -1288,6 +1349,7 @@ mymain(void) DO_TEST(test23); DO_TEST(test24); DO_TEST(test25); + DO_TEST(test26); =20 virMutexLock(&test->lock); if (test->running) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 287e9d4ea3..5812e85436 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -620,7 +620,7 @@ testCompareXMLToArgv(const void *data) goto cleanup; } =20 - if (!(actualargv =3D virCommandToString(cmd))) + if (!(actualargv =3D virCommandToString(cmd, false))) goto cleanup; =20 if (virTestCompareToFile(actualargv, args) < 0) diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtes= t.c index 196989cb6d..2f2d40e027 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -71,7 +71,7 @@ testCompareXMLToArgvFiles(bool shouldFail, goto cleanup; }; =20 - if (!(actualCmdline =3D virCommandToString(cmd))) { + if (!(actualCmdline =3D virCommandToString(cmd, false))) { VIR_TEST_DEBUG("pool type %d failed to get commandline\n", def->ty= pe); goto cleanup; } diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 105705f348..bc2da37410 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -104,14 +104,14 @@ testCompareXMLToArgvFiles(bool shouldFail, } =20 if (convertStep !=3D VIR_STORAGE_VOL_ENCRYPT_CONVERT) { - if (!(actualCmdline =3D virCommandToString(cmd))) + if (!(actualCmdline =3D virCommandToString(cmd, false))) goto cleanup; } else { char *createCmdline =3D actualCmdline; char *cvtCmdline; int rc; =20 - if (!(cvtCmdline =3D virCommandToString(cmd))) + if (!(cvtCmdline =3D virCommandToString(cmd, false))) goto cleanup; =20 rc =3D virAsprintf(&actualCmdline, "%s\n%s", --=20 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list