So far we only have an API that truncates the file prior to writing it. However, experience show need for new API that just appends a string to file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 48 ++++++++++++++++++++++++++++++++++++++++-------- src/util/virfile.h | 2 ++ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0c16343..f9bdd8c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1202,6 +1202,7 @@ virBuildPathInternal; virDirCreate; virFileAbsPath; virFileAccessibleAs; +virFileAppendStr; virFileBuildPath; virFileClose; virFileDeleteTree; diff --git a/src/util/virfile.c b/src/util/virfile.c index 631cd06..1be3367 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1291,19 +1291,16 @@ virFileReadAll(const char *path, int maxlen, char **buf) return len; } -/* Truncate @path and write @str to it. If @mode is 0, ensure that - @path exists; otherwise, use @mode if @path must be created. - Return 0 for success, nonzero for failure. - Be careful to preserve any errno value upon failure. */ -int -virFileWriteStr(const char *path, const char *str, mode_t mode) +static int +virFileWriteAppendStr(const char *path, const char *str, + int o_flags, mode_t mode) { int fd; if (mode) - fd = open(path, O_WRONLY|O_TRUNC|O_CREAT, mode); + fd = open(path, o_flags | O_CREAT, mode); else - fd = open(path, O_WRONLY|O_TRUNC); + fd = open(path, o_flags); if (fd == -1) return -1; @@ -1319,6 +1316,41 @@ virFileWriteStr(const char *path, const char *str, mode_t mode) return 0; } +/** + * virFileWriteStr: + * @path: file to write to + * @str: string to write + * @mode: file mode used when creating @path + * + * Truncate @path and write @str to it. If @mode is 0, ensure + * that @path exists; otherwise, use @mode if @path must be + * created. Be careful to preserve any errno value upon failure. + * + * Returns 0 on success, -1 on failure. + */ +int +virFileWriteStr(const char *path, const char *str, mode_t mode) +{ + return virFileWriteAppendStr(path, str, O_WRONLY | O_TRUNC, mode); +} + +/** + * virFileAppendStr: + * @path: file to write to + * @str: string to write + * @mode: file mode used when creating @path + * + * Append @str to @path. If the file doesn't exist, + * it is created using @mode. + * + * Returns 0 on success, -1 on failure. + */ +int +virFileAppendStr(const char *path, const char *str, mode_t mode) +{ + return virFileWriteAppendStr(path, str, O_WRONLY | O_APPEND, mode); +} + int virFileMatchesNameSuffix(const char *file, const char *name, diff --git a/src/util/virfile.h b/src/util/virfile.h index 0d20cdb..4bac829 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -131,6 +131,8 @@ int virFileReadAll(const char *path, int maxlen, char **buf) int virFileWriteStr(const char *path, const char *str, mode_t mode) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virFileAppendStr(const char *path, const char *str, mode_t mode) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virFileMatchesNameSuffix(const char *file, const char *name, -- 1.8.5.2

There are some units within libvirt that utilize virCommand API to run some commands and deserve own unit testing. These units are, however, not desired to be rewritten to dig virCommand API usage out. As a great example virNetDevBandwidth could be used. The problem with the bandwidth unit is: it uses virComamnd API heavily. Therefore we need a mechanism to not really run a command, but rather see its string representation after which we can decide if the unit construct the correct sequence of commands or not. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/vircommand.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-- src/util/vircommand.h | 2 ++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f9bdd8c..814c74c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1096,6 +1096,7 @@ virCommandRequireHandshake; virCommandRun; virCommandRunAsync; virCommandSetAppArmorProfile; +virCommandSetDryRun; virCommandSetErrorBuffer; virCommandSetErrorFD; virCommandSetGID; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index a52a1ab..313b6a4 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -129,6 +129,9 @@ struct _virCommand { #endif }; +/* See virCommandSetDryRun for description on this variable */ +static const char *outfile; + /* * virCommandFDIsSet: * @fd: FD to test @@ -2262,8 +2265,22 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) } str = virCommandToString(cmd); - VIR_DEBUG("About to run %s", str ? str : cmd->args[0]); - VIR_FREE(str); + str = str ? str : cmd->args[0]; + VIR_DEBUG("About to run %s", str); + if (outfile) { + /* Dry-run is requested. Just append @str to @outfile + * and claim success. */ + + if (virFileAppendStr(outfile, str, S_IRUSR | S_IWUSR) < 0 || + virFileAppendStr(outfile, "\n", 0) < 0) { + virReportSystemError(errno, _("unable to append to %s"), + outfile); + goto cleanup; + } + + ret = 0; + goto cleanup; + } ret = virExec(cmd); VIR_DEBUG("Command result %d, with PID %d", @@ -2303,6 +2320,7 @@ cleanup: VIR_FORCE_CLOSE(cmd->infd); VIR_FORCE_CLOSE(cmd->inpipe); } + VIR_FREE(str); return ret; } @@ -2334,6 +2352,13 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) return -1; } + if (outfile) { + /* Dry-run requested. Claim success. */ + if (exitstatus) + *exitstatus = 0; + return 0; + } + if (cmd->pid == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("command is not yet running")); @@ -2669,3 +2694,33 @@ virCommandDoAsyncIO(virCommandPtr cmd) cmd->flags |= VIR_EXEC_ASYNC_IO | VIR_EXEC_NONBLOCK; } + +/** + * virCommandSetDryRun: + * @file: destination file + * + * Sometimes it's desired to not actually run given command, but + * see its string representation without having to change the + * callee. As a great example can serve unit testing. In such + * cases, the callee constructs the command and calls it via + * virCommandRun* API. The virCommandSetDryRun allows you to + * modify this behavior: once called, every call to + * virCommandRun* results in command string representation being + * appended to @file instead of being executed. For example: + * + * virCommandSetDryRun("/tmp/test.txt"); + * + * const char *const echocmd[] = {"/bin/echo", "Hello world"} + * virCommandRun(echocmd, NULL); + * + * After this, there's /tmp/test.txt with contents of: + * + * /bin/echo Hello world + * + * To cancel this effect pass NULL. + */ +void +virCommandSetDryRun(const char *file) +{ + outfile = file; +} diff --git a/src/util/vircommand.h b/src/util/vircommand.h index e977f93..d942c5b 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -184,4 +184,6 @@ void virCommandAbort(virCommandPtr cmd); void virCommandFree(virCommandPtr cmd); void virCommandDoAsyncIO(virCommandPtr cmd); + +void virCommandSetDryRun(const char *file); #endif /* __VIR_COMMAND_H__ */ -- 1.8.5.2

The test tries to set some QoS limits and check if the commands that are actually executed are the expected ones. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virnetdevbandwidthtest.c | 98 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 989018e..9358511 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -20,7 +20,10 @@ #include <config.h> +#include <fcntl.h> +#include <stdlib.h> #include "testutils.h" +#include "vircommand.h" #include "virnetdevbandwidth.h" #include "netdev_bandwidth_conf.c" @@ -32,6 +35,14 @@ struct testMinimalStruct { const char *band2; }; +struct testSetStruct { + const char *outfile; + const char *band; + const char *exp_cmd; + const char *iface; + const bool hierarchical_class; +}; + #define PARSE(xml, var) \ do { \ xmlDocPtr doc; \ @@ -104,9 +115,64 @@ cleanup: } static int +testVirNetDevBandwidthSet(const void *data) +{ + int ret = -1; + const struct testSetStruct *info = data; + const char *iface = info->iface; + virNetDevBandwidthPtr band = NULL; + char *actual_cmd = NULL; + + PARSE(info->band, band); + + if (!iface) + iface = "eth0"; + + /* Clear the temporary file between runs */ + if (truncate(info->outfile, 0) < 0) { + fprintf(stderr, "Unable to truncate %s %d\n", info->outfile, errno); + goto cleanup; + } + + virCommandSetDryRun(info->outfile); + + if (virNetDevBandwidthSet(iface, band, info->hierarchical_class) < 0) + goto cleanup; + + if (virFileReadAll(info->outfile, 2048, &actual_cmd) < 0) + goto cleanup; + + if (STRNEQ(info->exp_cmd, actual_cmd)) { + virtTestDifference(stderr, info->exp_cmd, actual_cmd); + goto cleanup; + } + + ret = 0; +cleanup: + virNetDevBandwidthFree(band); + VIR_FREE(actual_cmd); + return ret; +} + + +#define OUTFILETEMPLATE abs_builddir "/virnetdevbandwidthtest-XXXXXX" + +static int mymain(void) { int ret = 0; + int outfile_fd = -1; + char *outfile; + + if (VIR_STRDUP_QUIET(outfile, OUTFILETEMPLATE) < 0) { + fprintf(stderr, "Out of memory\n"); + abort(); + } + + if ((outfile_fd = mkostemp(outfile, O_CLOEXEC)) < 0) { + fprintf(stderr, "Cannot create outfile"); + abort(); + } #define DO_TEST_MINIMAL(r, ...) \ do { \ @@ -117,6 +183,18 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_SET(Band, Exp_cmd, ...) \ + do { \ + struct testSetStruct data = {.outfile = outfile, \ + .band = Band, \ + .exp_cmd = Exp_cmd, \ + __VA_ARGS__}; \ + if (virtTestRun("virNetDevBandwidthSet", \ + testVirNetDevBandwidthSet, \ + &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST_MINIMAL(NULL, NULL, NULL); @@ -149,6 +227,26 @@ mymain(void) " <inbound average='1' peak='2' floor='3'/>" " <outbound average='5' peak='6'/>" "</bandwidth>"); + + DO_TEST_SET(("<bandwidth>" + " <inbound average='1' peak='2' floor='3' burst='4'/>" + " <outbound average='5' peak='6' burst='7'/>" + "</bandwidth>"), + ("/sbin/tc qdisc del dev eth0 root\n" + "/sbin/tc qdisc del dev eth0 ingress\n" + "/sbin/tc qdisc add dev eth0 root handle 1: htb default 1\n" + "/sbin/tc class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb\n" + "/sbin/tc qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n" + "/sbin/tc filter add dev eth0 parent 1:0 protocol ip handle 1 fw flowid 1\n" + "/sbin/tc qdisc add dev eth0 ingress\n" + "/sbin/tc filter add dev eth0 parent ffff: protocol ip u32 match ip src 0.0.0.0/0 " + "police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n")); + + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) + unlink(outfile); + + VIR_FORCE_CLOSE(outfile_fd); + return ret; } -- 1.8.5.2