- Devel - libvirt List Archives

[libvirt] [PATCH 00/14] conf: disk: Continue refactoring of virDomainDiskDefParseXML
by Peter Krempa 21 Apr '16

21 Apr '16

This is yet another dump of my quest to despaghettize virDomainDiskDefParseXML. My eyes are starting to bleed a bit less, but there's still a lot of patches to come. Peter Krempa (14): conf: disk: Extract verification of disk config conf: disk: Split out parsing of disk <driver> element conf: disk: Avoid temporary variable when parsing driver name conf: disk: Sanitize parsing of disk format conf: disk: Mark VIR_DOMAIN_DISK_IO_DEFAULT as 0 and simplify parsing conf: disk: Remove custom single-use temporary variables conf: disk: Extract disk type and device right away conf: disk: Initialize closed device tray state to 0 conf: disk: Don't bother setting removable state to 0 by default conf: disk: extract sgio/rawio validation conf: disk: Extract verification of disk tray conf: disk: Move validation of disk bus vs disk type conf: disk: Extract checking of removable status conf: disk: extract validation of startup policy src/conf/domain_conf.c | 452 ++++++++++++++++++++++++------------------------- src/conf/domain_conf.h | 4 +- 2 files changed, 221 insertions(+), 235 deletions(-) -- 2.8.1

2 28

[libvirt] [PATCH 0/4] Introduce big test mock
by Michal Privoznik 21 Apr '16

21 Apr '16

What? Big test mock that checks all our file accesses whether we are touching anything outside $srcdir or $builddir. Why? Because it has happened in the past (and is happening even now as you'll see later) that our test suite produces nondeterministic results because of a file in live system being accessible or having some specific value. How? Well, this is merely introducing a mock library that every C program on Linux will use. I'm not aware of any approach for our shell tests. The mock library then collects all the paths outside $srcdir or $builddir and prints them into a file. The file is then checked against a white list, because some accesses might be desirable. Unfortunately, as you'll learn in the 4/4 commit message, I was unable to wire this up to 'make check' so this is accessible under 'make check-access' target. If there's some access outside aforementioned directories, the path will be printed out. So far there is plenty of them. For instance, in qemuxml2argvtest we are checking /proc/sys/crypto/fips_enabled for each test. Ouch. What's left to do? a) Enhance white list b) Wire this up to 'check' c) Test d) Review Michal Privoznik (4): seclabeltest: Update to use VIRT_TEST_MAIN tests: Introduce global mock library virtestmock: Print invalid file accesses into a file tests: Introduce check-file-access.pl .gitignore | 1 + HACKING | 11 ++ Makefile.am | 3 + cfg.mk | 8 +- docs/hacking.html.in | 15 +++ tests/Makefile.am | 28 +++- tests/check-file-access.pl | 106 +++++++++++++++ tests/file_access_whitelist.txt | 19 +++ tests/seclabeltest.c | 7 +- tests/testutils.c | 58 ++++++++- tests/testutils.h | 29 +---- tests/vircgroupmock.c | 6 +- tests/virpcimock.c | 6 +- tests/virtestmock.c | 279 ++++++++++++++++++++++++++++++++++++++++ 14 files changed, 535 insertions(+), 41 deletions(-) create mode 100755 tests/check-file-access.pl create mode 100644 tests/file_access_whitelist.txt create mode 100644 tests/virtestmock.c -- 2.7.3

2 6

[libvirt] [PATCH] build: Fix default network generation on FreeBSD
by Andrea Bolognani 21 Apr '16

21 Apr '16

FreeBSD's sed(1) doesn't support using "\n" to insert a newline, so the installed default.xml file ends up containing a literal "n" between tags; to work around this problem, add a tr(1) invocation as suggested by the sed FAQ[1]. [1] http://sed.sourceforge.net/sedfaq4.html (4.1 c) --- src/Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index c639e37..f020545 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -3072,8 +3072,9 @@ if WITH_NETWORK $(INSTALL_DATA) $(srcdir)/network/default.xml \ $(DESTDIR)$(confdir)/qemu/networks/default.xml test -z "$(UUID)" || \ - { sed -e "s,</name>,</name>\n <uuid>$(UUID)</uuid>," \ - $(DESTDIR)$(confdir)/qemu/networks/default.xml > \ + { sed -e "s,</name>,</name>; <uuid>$(UUID)</uuid>," \ + $(DESTDIR)$(confdir)/qemu/networks/default.xml | \ + tr ";" "\n" > \ $(DESTDIR)$(confdir)/qemu/networks/default.xml.t && \ cp $(DESTDIR)$(confdir)/qemu/networks/default.xml.t \ $(DESTDIR)$(confdir)/qemu/networks/default.xml && \ -- 2.5.5

2 2

[libvirt] [PATCH] docs: Update style for <dt> elements
by John Ferlan 21 Apr '16

21 Apr '16

Add bolding for <dt> elements to make them "stick out" on the page rather that just a stream of text where the <dt> elements only differ by slightly different font style since most uses encase the text within using <code> Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- I had suggested this during a review of Andrea's changes late last month: http://www.redhat.com/archives/libvir-list/2016-March/msg01501.html but figure I'll make the more 'formal' patch request... docs/generic.css | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/generic.css b/docs/generic.css index 208e31e..b86bb88 100644 --- a/docs/generic.css +++ b/docs/generic.css @@ -25,6 +25,7 @@ ul, ol { dt { margin-left: 1em; margin-right: 2em; + font-weight: bold; } dl dd { -- 2.5.5

2 3

[libvirt] [PATCH] tools: Fix connect command
by Martin Kletzander 21 Apr '16

21 Apr '16

The man page says: "(Re)-Connect to the hypervisor. When the shell is first started, this is automatically run with the URI parameter requested by the "-c" option on the command line." However, if you run: virsh -c 'test://default' 'connect; uri' the output will not be 'test://default'. That's because the 'connect' command does not care about any virsh-only related settings and if it is run without parameters, it connects with @uri == NULL. Not only that doesn't comply to what the man page describes, but it also doesn't make sense. It also means you aren't able to reconnect to whatever you are connected currently. So let's fix that in both virsh and virt-admin add a test case for it. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- tests/test-lib.sh | 6 ++++++ tests/virsh-uriprecedence | 54 +++++++++++++++++++++++++++++++++++------------ tools/virsh.c | 6 ++++-- tools/virt-admin.c | 6 ++++-- 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/tests/test-lib.sh b/tests/test-lib.sh index 8e0ce83e118c..49e8d2209572 100644 --- a/tests/test-lib.sh +++ b/tests/test-lib.sh @@ -232,6 +232,12 @@ if test -n "$VIR_TEST_DEBUG" || test -n "$VIR_TEST_VERBOSE" ; then verbose=1 fi +debug() { :; } + +if test "$VIR_TEST_DEBUG" = "2"; then + debug() { echo "$@"; } +fi + # This is a stub function that is run upon trap (upon regular exit and # interrupt). Override it with a per-test function, e.g., to unmount # a partition, or to undo any other global state changes. diff --git a/tests/virsh-uriprecedence b/tests/virsh-uriprecedence index f9e325658900..4bcce3aeb08f 100755 --- a/tests/virsh-uriprecedence +++ b/tests/virsh-uriprecedence @@ -8,7 +8,8 @@ test_intro "virsh-uriprecedence" virsh_bin="$abs_top_builddir/tools/virsh" -counter=1 +virsh_cmd="$virsh_bin" +counter=0 ret=0 cleanup_() { rm -rf "$tmphome"; } @@ -23,16 +24,44 @@ mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh" mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh" mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh" -# Main function checking for the proper uri being returned +is_uri_good() +{ + echo "$1" | grep -q -F "$good_uri" +} + +test_uri_internal() +{ + test_name=$1 + test_cmd="$virsh_cmd \"$2\"" + result=0 + + debug "Running '$test_cmd'" + out="$($virsh_cmd "$2")" + + if ! is_uri_good "$out"; then + debug "Invalid output: '$out'" + result=1 + ret=1 + fi + + counter="$((counter+1))" + test_result "$counter" "$1" "$result" +} + +test_uri_connect() +{ + test_uri_internal "$1" "connect; uri" +} + +test_uri_noconnect() +{ + test_uri_internal "$1" "uri" +} + test_uri() { - result=0 - if [ "$($virsh_bin uri)" != "$good_uri" ]; then - result=1 - ret=1 - fi - test_result "$counter" "$1" "$result" - counter="$((counter+1))" + test_uri_connect "$1" + test_uri_noconnect "$1" } # Precedence is the following (lowest priority first): @@ -57,6 +86,7 @@ good_uri="test:///default?good_uri" printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf" if uid_is_privileged_; then + counter="$((counter+1))" test_skip_case "$counter" "User config file" "must not be run as root" else test_uri "User config file" @@ -71,10 +101,8 @@ export VIRSH_DEFAULT_CONNECT_URI="$good_uri" test_uri "VIRSH_DEFAULT_CONNECT_URI" export VIRSH_DEFAULT_CONNECT_URI="$bad_uri" -virsh_bin="$virsh_bin --connect $good_uri" +virsh_cmd="$virsh_bin --connect $good_uri" test_uri "Parameter" -# test_uri() increases $counter even for the last test, so we must -# decrement it -test_final "$((counter-1))" "$ret" +test_final "$counter" "$ret" (exit "$ret"); exit "$ret" diff --git a/tools/virsh.c b/tools/virsh.c index 366956cd5c75..d17aaf0be8ea 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -306,11 +306,13 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) priv->conn = NULL; } - VIR_FREE(ctl->connname); if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) return false; - ctl->connname = vshStrdup(ctl, name); + if (name) { + VIR_FREE(ctl->connname); + ctl->connname = vshStrdup(ctl, name); + } priv->useGetInfo = false; priv->useSnapshotOld = false; diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 22160ad929d0..4275aa37a1bf 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -291,8 +291,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) return false; - VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, name); + if (name) { + VIR_FREE(ctl->connname); + ctl->connname = vshStrdup(ctl, name); + } vshAdmReconnect(ctl); if (!connected) -- 2.8.1

2 1

[libvirt] [PATCH] tests: Use system() for test-wrap-argv.pl
by Cole Robinson 21 Apr '16

21 Apr '16

virCommand doesn't buy us anything here, and this simplifies the code. There's another usage of system() in testutils.c already FWIW, though it predates virCommand --- Had this sitting in a branch from an earlier discussion... I figure system() is safe in the test suite but I'm not going to argue if someone feels strongly the other way tests/testutils.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 79d0763..7664b06 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -432,28 +432,14 @@ virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, static int virTestRewrapFile(const char *filename) { - int ret = -1; - char *outbuf = NULL; - char *script = NULL; - virCommandPtr cmd = NULL; - - if (virAsprintf(&script, "%s/test-wrap-argv.pl", abs_srcdir) < 0) - goto cleanup; - - cmd = virCommandNewArgList(script, filename, NULL); - virCommandSetOutputBuffer(cmd, &outbuf); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + char *cmd; - if (virFileWriteStr(filename, outbuf, 0666) < 0) - goto cleanup; + /* The 'echo' syntax lets us read and write the file in one shot */ + if (virAsprintf(&cmd, "echo \"$(%s/test-wrap-argv.pl %s)\" > %s", + abs_srcdir, filename, filename) < 0) + return -1; - ret = 0; - cleanup: - VIR_FREE(script); - virCommandFree(cmd); - VIR_FREE(outbuf); - return ret; + return system(cmd); } /** -- 2.7.3

5 5

[libvirt] [PATCH] lxc: explicitly error on interface type=ethernet
by Cole Robinson 21 Apr '16

21 Apr '16

It isn't implemented and does not work: error: internal error: guest failed to start: /usr/lib/libvirt/libvirt_lxc: option '--veth' requires an argument syntax: /usr/lib/libvirt/libvirt_lxc [OPTIONS] ... We previously threw an explicit error, but this changed in 22cff52a2b8e06c913b1f97767e5d390fb17fc3b , which I suspect was untested for LXC --- src/lxc/lxc_process.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 5e0bbe2..0044ee5 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -559,8 +559,6 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - break; - case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: -- 2.7.3

3 4

[libvirt] [libvirt-glib] events: Mark 'eventlock' as static
by Christophe Fergeau 21 Apr '16

21 Apr '16

It's not used outside of the libvirt-glib-event.c file, so there is no good reason for not having it static. As it was not listed in libvirt-glib.sym, this will make no change to the publicly exported symbols (ie this is not an ABI change). --- libvirt-glib/libvirt-glib-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-glib/libvirt-glib-event.c b/libvirt-glib/libvirt-glib-event.c index f8227d6..4548aa6 100644 --- a/libvirt-glib/libvirt-glib-event.c +++ b/libvirt-glib/libvirt-glib-event.c @@ -110,7 +110,7 @@ struct gvir_event_timeout virFreeCallback ff; }; -GMutex *eventlock = NULL; +static GMutex *eventlock = NULL; static int nextwatch = 1; static GPtrArray *handles; -- 2.5.0

1 1

[libvirt] [PATCH 0/4] Fix python sanity check after moving typedParam stuff to libvirt-common.h
by Peter Krempa 21 Apr '16

21 Apr '16

Fixes the following breakage: Cannot get a value of enum VIR_TYPED_PARAM_BOOLEAN (originally VIR_DOMAIN_BLKIO_PARAM_BOOLEAN) Cannot get a value of enum VIR_TYPED_PARAM_DOUBLE (originally VIR_DOMAIN_BLKIO_PARAM_DOUBLE) Cannot get a value of enum VIR_TYPED_PARAM_INT (originally VIR_DOMAIN_BLKIO_PARAM_INT) ... Peter Krempa (4): include: common: Fix file header for apibuild.py apibuild: Allow completely skipping certain macros api: Generate docs for libvirt-common.h include: common: Fix function formatting docs/Makefile.am | 1 + docs/apibuild.py | 12 +++ docs/sitemap.html.in | 4 + include/libvirt/libvirt-common.h.in | 173 ++++++++++++++++-------------------- 4 files changed, 95 insertions(+), 95 deletions(-) -- 2.8.1

2 7

[libvirt] [PATCH 0/2] virsh: perf: Trivial fixes of a memleak and coding style
by Peter Krempa 21 Apr '16

21 Apr '16

Both are pushed under the trivial rule. Peter Krempa (2): virsh: perf: Don't leak domain virsh: perf: Remove unnecessary 'error' label tools/virsh-domain.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) -- 2.8.1

2 3