January 2014 - Devel - libvirt List Archives

[libvirt] [ruby-libvirt] Allow to set URI via environment variable
by Guido Günther 08 Jan '14

08 Jan '14

This allows us to run tests with the destdriver like: RUBY_LIBVIRT_TEST_URI=test:///default \ RUBYLIB=lib/:ext/libvirt/ \ ruby tests/test_open.rb --- I've only converted three modules so far since they were simple but these already help to run tests during package build. tests/test_nodedevice.rb | 2 +- tests/test_open.rb | 36 ++++++++++++++++++------------------ tests/test_stream.rb | 2 +- tests/test_utils.rb | 2 ++ 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/test_nodedevice.rb b/tests/test_nodedevice.rb index 7da1a5f..50a1304 100644 --- a/tests/test_nodedevice.rb +++ b/tests/test_nodedevice.rb @@ -9,7 +9,7 @@ require 'test_utils.rb' set_test_object("nodedevice") -conn = Libvirt::open("qemu:///system") +conn = Libvirt::open(URI) # TESTGROUP: nodedevice.name testnode = conn.lookup_nodedevice_by_name(conn.list_nodedevices[0]) diff --git a/tests/test_open.rb b/tests/test_open.rb index 17baa93..c67eaab 100644 --- a/tests/test_open.rb +++ b/tests/test_open.rb @@ -21,49 +21,49 @@ expect_success(Libvirt, "nil arg", "version", nil) {|x| x.class == Array and x.l expect_success(Libvirt, "Test arg", "version", "Test") {|x| x.class == Array and x.length == 2} # TESTGROUP: Libvirt::open -expect_too_many_args(Libvirt, "open", "qemu:///system", 1) +expect_too_many_args(Libvirt, "open", URI, 1) expect_connect_error("open", "foo:///system") conn = expect_success(Libvirt, "no args", "open") {|x| x.class == Libvirt::Connect } conn.close -conn = expect_success(Libvirt, "qemu:///system", "open", "qemu:///system") {|x| x.class == Libvirt::Connect } +conn = expect_success(Libvirt, URI, "open", URI) {|x| x.class == Libvirt::Connect } conn.close conn = expect_success(Libvirt, "nil arg", "open", nil) {|x| x.class == Libvirt::Connect } conn.close # TESTGROUP: Libvirt::open_read_only -expect_too_many_args(Libvirt, "open_read_only", "qemu:///system", 1) +expect_too_many_args(Libvirt, "open_read_only", URI, 1) expect_connect_error("open_read_only", "foo:///system") conn = expect_success(Libvirt, "no args", "open_read_only") {|x| x.class == Libvirt::Connect } conn.close -conn = expect_success(Libvirt, "qemu:///system", "open_read_only", "qemu:///system") {|x| x.class == Libvirt::Connect } +conn = expect_success(Libvirt, URI, "open_read_only", URI) {|x| x.class == Libvirt::Connect } conn.close conn = expect_success(Libvirt, "nil arg", "open_read_only", nil) {|x| x.class == Libvirt::Connect } conn.close # TESTGROUP: Libvirt::open_auth -expect_too_many_args(Libvirt, "open_auth", "qemu:///system", [], "hello there", 1, 2) +expect_too_many_args(Libvirt, "open_auth", URI, [], "hello there", 1, 2) expect_connect_error("open_auth", "foo:///system") expect_invalid_arg_type(Libvirt, "open_auth", 1) -expect_invalid_arg_type(Libvirt, "open_auth", "qemu:///system", [], "hello", "foo") +expect_invalid_arg_type(Libvirt, "open_auth", URI, [], "hello", "foo") conn = expect_success(Libvirt, "no args", "open_auth") {|x| x.class == Libvirt::Connect } conn.close -conn = expect_success(Libvirt, "uri arg", "open_auth", "qemu:///system") {|x| x.class == Libvirt::Connect } +conn = expect_success(Libvirt, "uri arg", "open_auth", URI) {|x| x.class == Libvirt::Connect } conn.close -conn = expect_success(Libvirt, "uri and empty cred args", "open_auth", "qemu:///system", []) {|x| x.class == Libvirt::Connect } +conn = expect_success(Libvirt, "uri and empty cred args", "open_auth", URI, []) {|x| x.class == Libvirt::Connect } conn.close -conn = expect_success(Libvirt, "uri and full cred args", "open_auth", "qemu:///system", [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE]) {|x| x.class == Libvirt::Connect } +conn = expect_success(Libvirt, "uri and full cred args", "open_auth", URI, [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE]) {|x| x.class == Libvirt::Connect } conn.close -conn = expect_success(Libvirt, "uri, full cred, and user args", "open_auth", "qemu:///system", [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE], "hello") {|x| x.class == Libvirt::Connect } +conn = expect_success(Libvirt, "uri, full cred, and user args", "open_auth", URI, [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE], "hello") {|x| x.class == Libvirt::Connect } conn.close # equivalent to expect_invalid_arg_type begin - conn = Libvirt::open_auth("qemu:///system", {}) do |cred| + conn = Libvirt::open_auth(URI, {}) do |cred| end rescue TypeError => e puts_ok "#{$test_object}.open_auth invalid arg type threw #{TypeError.to_s}" @@ -73,7 +73,7 @@ end # equivalent to expect_invalid_arg_type begin - conn = Libvirt::open_auth("qemu:///system", 1) do |cred| + conn = Libvirt::open_auth(URI, 1) do |cred| end rescue TypeError => e puts_ok "#{$test_object}.open_auth invalid arg type threw #{TypeError.to_s}" @@ -83,7 +83,7 @@ end # equivalent to expect_invalid_arg_type begin - conn = Libvirt::open_auth("qemu:///system", 'foo') do |cred| + conn = Libvirt::open_auth(URI, 'foo') do |cred| end rescue TypeError => e puts_ok "#{$test_object}.open_auth invalid arg type threw #{TypeError.to_s}" @@ -93,7 +93,7 @@ end # equivalent to "expect_success" begin - conn = Libvirt::open_auth("qemu:///system", [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE], "hello") do |cred| + conn = Libvirt::open_auth(URI, [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE], "hello") do |cred| if not cred["userdata"].nil? puts "userdata is #{cred["userdata"]}" end @@ -122,7 +122,7 @@ end # equivalent to "expect_success" begin - conn = Libvirt::open_auth("qemu:///system") do |cred| + conn = Libvirt::open_auth(URI) do |cred| end puts_ok "Libvirt.open_auth uri, succeeded" @@ -135,7 +135,7 @@ end # equivalent to "expect_success" begin - conn = Libvirt::open_auth("qemu:///system", [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE], "hello", Libvirt::CONNECT_RO) do |cred| + conn = Libvirt::open_auth(URI, [Libvirt::CRED_AUTHNAME, Libvirt::CRED_PASSPHRASE], "hello", Libvirt::CONNECT_RO) do |cred| if not cred["userdata"].nil? puts "userdata is #{cred["userdata"]}" end @@ -163,7 +163,7 @@ rescue => e end # TESTGROUP: Libvirt::event_invoke_handle_callback -conn = Libvirt::open("qemu:///system") +conn = Libvirt::open(URI) expect_too_many_args(Libvirt, "event_invoke_handle_callback", 1, 2, 3, 4, 5) expect_too_few_args(Libvirt, "event_invoke_handle_callback") @@ -188,7 +188,7 @@ expect_invalid_arg_type(Libvirt, "event_invoke_handle_callback", 1, 1, 1, { "lib conn.close # TESTGROUP: Libvirt::event_invoke_timeout_callback -conn = Libvirt::open("qemu:///system") +conn = Libvirt::open(URI) expect_too_many_args(Libvirt, "event_invoke_timeout_callback", 1, 2, 3) expect_too_few_args(Libvirt, "event_invoke_timeout_callback") diff --git a/tests/test_stream.rb b/tests/test_stream.rb index 07ec222..e272104 100644 --- a/tests/test_stream.rb +++ b/tests/test_stream.rb @@ -9,7 +9,7 @@ require 'test_utils.rb' set_test_object("stream") -conn = Libvirt::open("qemu:///system") +conn = Libvirt::open(URI) # TESTGROUP: stream.send st = conn.stream diff --git a/tests/test_utils.rb b/tests/test_utils.rb index 1bac1cc..519de27 100644 --- a/tests/test_utils.rb +++ b/tests/test_utils.rb @@ -2,6 +2,8 @@ $FAIL = 0 $SUCCESS = 0 $SKIPPED = 0 +URI = ENV['RUBY_LIBVIRT_TEST_URI'] || "qemu:///system" + $GUEST_BASE = '/var/lib/libvirt/images/ruby-libvirt-tester' $GUEST_DISK = $GUEST_BASE + '.qcow2' $GUEST_SAVE = $GUEST_BASE + '.save' -- 1.8.5.2

2 1

[libvirt] [PATCH] Allow to install apparmor profiles
by Guido Günther 08 Jan '14

08 Jan '14

Make it easy to install the shipped examples. The aim is to have reasonably working templates so that distros only need to minimally patch these and can feed things upstream more easily. This was prompted by http://bugs.debian.org/725144 --- configure.ac | 12 +++++++++++- examples/apparmor/Makefile.am | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2622dfd..97752f4 100644 --- a/configure.ac +++ b/configure.ac @@ -1418,6 +1418,16 @@ elif test "with_secdriver_apparmor" != "no" ; then fi AM_CONDITIONAL([WITH_SECDRIVER_APPARMOR], [test "$with_secdriver_apparmor" != "no"]) +if test "$with_apparmor" != "no"; then +AC_ARG_WITH([apparmor-profiles], + [AS_HELP_STRING([--with-apparmor-profiles], + [install apparmor profiles @<:@default=no@:>@])], + [with_apparmor_profiles=yes], + [with_apparmor_profiles=no]) +else + with_apparmor_profiles="no" +fi +AM_CONDITIONAL([WITH_APPARMOR_PROFILES], [test "$with_apparmor_profiles" != "no"]) dnl DTrace static probes AC_ARG_WITH([dtrace], @@ -2607,7 +2617,7 @@ AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ SELinux: $with_secdriver_selinux ($SELINUX_MOUNT)]) -AC_MSG_NOTICE([AppArmor: $with_secdriver_apparmor]) +AC_MSG_NOTICE([AppArmor: $with_secdriver_apparmor (install profiles: $with_apparmor_profiles)]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Driver Loadable Modules]) AC_MSG_NOTICE([]) diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am index 79cf5c9..6e69440 100644 --- a/examples/apparmor/Makefile.am +++ b/examples/apparmor/Makefile.am @@ -19,3 +19,21 @@ EXTRA_DIST= \ libvirt-qemu \ usr.lib.libvirt.virt-aa-helper \ usr.sbin.libvirtd + +if WITH_APPARMOR_PROFILES +apparmordir = $(sysconfdir)/apparmor.d/ +apparmor_DATA = \ + usr.lib.libvirt.virt-aa-helper \ + usr.sbin.libvirtd \ + $(NULL) + +abstractionsdir = $(apparmordir)/abstractions +abstractions_DATA = \ + libvirt-qemu \ + $(NULL) + +templatesdir = $(apparmordir)/libvirtd +templates_DATA = \ + TEMPLATE \ + $(NULL) +endif WITH_APPARMOR_PROFILES -- 1.8.5.1

2 2

[libvirt] [PATCHv2 00/14] event cleanups
by Eric Blake 08 Jan '14

08 Jan '14

I'm sitting on quite a few event fixes, prior to being able to reach my goal of adding a new api to libvirt-qemu.so for querying arbitrary monitor events. For testing purposes, I split the tests from the code changes, to make it easier (at least for me) to prove what the bugs are, and how the code fixes the bug. Each patch in this series should pass 'make', but those labeled '[squash with next]' will fail 'make check' until you also apply the next patch in the series. Before pushing, I will merge the patches as documented. That is, although there are 14 mails, there are only 9 commits to push when I finally get acks. If you exclude tests/, this series removes more lines than it adds (always a desirable goal when refactoring to fix bugs). Eric Blake (14): event [squash with next]: add test of old-style events event: make deregister return value match docs event [squash with next]: test shared state driver in test:///default event: share state driver between test:///default connections event: rename confusing variable in test, remote drivers event [squash with next]: test that old-style can't clobber new-style event: don't let old-style events clobber per-domain events event: tighten scope of object_event event: properly filter count of remaining events event [squash with next]: test that old-style is distinct from new-style event: don't allow mix of old- and new-style registration event [squash with next]: test per-domain events of offline domain event: don't turn offline domain into global event event: make network events easier to use without casts src/conf/domain_event.c | 185 ++++------------------------- src/conf/network_event.c | 16 +-- src/conf/network_event.h | 3 +- src/conf/object_event.c | 255 +++++++++++++++++++++++++++++----------- src/conf/object_event.h | 5 +- src/conf/object_event_private.h | 63 ++-------- src/libvirt.c | 17 ++- src/libxl/libxl_driver.c | 35 +++--- src/lxc/lxc_driver.c | 30 ++--- src/network/bridge_driver.c | 14 ++- src/remote/remote_driver.c | 86 ++++++-------- src/test/test_driver.c | 135 +++++++++++++-------- src/uml/uml_driver.c | 29 +++-- src/vbox/vbox_tmpl.c | 32 +++-- src/xen/xen_driver.c | 25 ++-- tests/objecteventtest.c | 196 +++++++++++++++++++++++++++--- 16 files changed, 631 insertions(+), 495 deletions(-) -- 1.8.4.2

2 33

[libvirt] vm tunable memory and cgroup
by yue 08 Jan '14

08 Jan '14

hi,all 1. is there any relationship between cgroup and tunable memory ? if i adjust vm's memory, if cgroup has any responding action? <memory unit='KiB'>1048576</memory> <currentMemory unit='KiB'>524288</currentMemory> 2. it seems that the memory.limit_in_bytes of a vm in cgroup is twice over vm's assigned memory? addtional, blkio.sectors does not equal sectors calculated from inner vm. how limit_in_bytes, sectors are calculative? what is the relationship between them? thanks

1 0

[libvirt] [PATCH V2 1/1] Add hw random number generator (/dev/hwrng) to cgroup ACL
by Pradipta Kr. Banerjee 08 Jan '14

08 Jan '14

From: "Pradipta Kr. Banerjee" <bpradip(a)in.ibm.com> Creating a qemu VM with /dev/hwrng as backend RNG device throws the following error - "Could not open '/dev/hwrng': Permission denied" This patch fixes the issue Signed-off-by: Pradipta Kr. Banerjee <bpradip(a)in.ibm.com> --- src/qemu/qemu_cgroup.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f0cacd0..8e2076e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -533,6 +533,18 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, goto cleanup; } + if (vm->def->rng && + (vm->def->rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM)) { + VIR_DEBUG("Setting Cgroup ACL for RNG device"); + rv = virCgroupAllowDevicePath(priv->cgroup, vm->def->rng->source.file, + VIR_CGROUP_DEVICE_RW); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", + vm->def->rng->source.file, "rw", rv == 0); + if (rv < 0 && + !virLastErrorIsSystemErrno(ENOENT)) + goto cleanup; + } + ret = 0; cleanup: virObjectUnref(cfg); -- 1.8.3.1

2 2

[libvirt] [PATCH 0/5] qemu: Fix job usage in several APIs
by Jiri Denemark 08 Jan '14

08 Jan '14

When fixing https://bugzilla.redhat.com/show_bug.cgi?id=1043069 I realized qemuDomainBlockStats is not the only API that does not acquire a job early enough. Generally, every API that is going to begin a job should do that before fetching data from vm->def. The following 5 APIs failed to do so and moreover used the data fetched early from vm->def after starting a job. In some circumstances this can lead to a crash. Jiri Denemark (5): qemu: Do not access stale data in virDomainBlockStats qemu: Avoid using stale data in virDomainGetBlockInfo qemu: Fix job usage in qemuDomainBlockJobImpl qemu: Fix job usage in qemuDomainBlockCopy qemu: Fix job usage in virDomainGetBlockIoTune src/qemu/qemu_driver.c | 92 ++++++++++++++++++++++++-------------------------- 1 file changed, 44 insertions(+), 48 deletions(-) -- 1.8.5.2

3 15

[libvirt] [PATCH] Read PCI class from sysfs class file instead of config space.
by Thadeu Lima de Souza Cascardo 08 Jan '14

08 Jan '14

When determining if a device is behind a PCI bridge, the PCI device class is checked by reading the config space. However, there are some devices which have the wrong class on the config space, but the class is initialized by Linux correctly as a PCI BRIDGE. This class can be read by the sysfs file '/sys/bus/pci/devices/xxxx:xx:xx.x/class'. One example of such bridge is IBM PCI Bridge 1014:03b9, which is identified as a Host Bridge when reading the config space. Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo(a)linux.vnet.ibm.com> --- The new test cases succeed with this patch. The second test fails without the changes to src/util/virpci.c. --- src/util/virpci.c | 41 +++++++++++++++++++++++++++-- tests/virpcimock.c | 33 ++++++++++++++++++++++++ tests/virpcitest.c | 34 ++++++++++++++++++++++++ tests/virpcitestdata/0001:00:00.0.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0001:01:00.0.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0001:01:00.1.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0005:80:00.0.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0005:90:01.0.config | Bin 0 -> 256 bytes tests/virpcitestdata/0005:90:01.1.config | Bin 0 -> 256 bytes tests/virpcitestdata/0005:90:01.2.config | Bin 0 -> 256 bytes 10 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/virpcitestdata/0001:00:00.0.config create mode 100644 tests/virpcitestdata/0001:01:00.0.config create mode 100644 tests/virpcitestdata/0001:01:00.1.config create mode 100644 tests/virpcitestdata/0005:80:00.0.config create mode 100644 tests/virpcitestdata/0005:90:01.0.config create mode 100644 tests/virpcitestdata/0005:90:01.1.config create mode 100644 tests/virpcitestdata/0005:90:01.2.config diff --git a/src/util/virpci.c b/src/util/virpci.c index 8ec642f..8c10a93 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -344,6 +344,37 @@ virPCIDeviceRead32(virPCIDevicePtr dev, int cfgfd, unsigned int pos) } static int +virPCIDeviceReadClass(virPCIDevicePtr dev, uint16_t *device_class) +{ + char *path = NULL; + char *id_str; + int ret = 0; + unsigned int value; + + if (virPCIFile(&path, dev->name, "class") < 0) + return -1; + + /* class string is '0xNNNNNN\n' ... i.e. 9 bytes */ + if (virFileReadAll(path, 9, &id_str) < 0) { + VIR_FREE(path); + return -1; + } + + VIR_FREE(path); + + id_str[8] = '\0'; + ret = virStrToLong_ui(id_str, NULL, 16, &value); + if (ret == 0) + *device_class = (value >> 8) & 0xFFFF; + else + VIR_WARN("Unusual value in " PCI_SYSFS "devices/%s/class: %s", + dev->name, id_str); + + VIR_FREE(id_str); + return ret; +} + +static int virPCIDeviceWrite(virPCIDevicePtr dev, int cfgfd, unsigned int pos, @@ -645,8 +676,8 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void *data) return 0; /* Is it a bridge? */ - device_class = virPCIDeviceRead16(check, fd, PCI_CLASS_DEVICE); - if (device_class != PCI_CLASS_BRIDGE_PCI) + ret = virPCIDeviceReadClass(check, &device_class); + if (ret == -1 || device_class != PCI_CLASS_BRIDGE_PCI) goto cleanup; /* Is it a plane? */ @@ -2110,6 +2141,7 @@ virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev) unsigned int pos; int fd; int ret = 0; + uint16_t device_class; if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0) return -1; @@ -2119,8 +2151,11 @@ virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev) goto cleanup; } + if (virPCIDeviceReadClass(dev, &device_class)) + goto cleanup; + pos = dev->pcie_cap_pos; - if (!pos || virPCIDeviceRead16(dev, fd, PCI_CLASS_DEVICE) != PCI_CLASS_BRIDGE_PCI) + if (!pos || device_class != PCI_CLASS_BRIDGE_PCI) goto cleanup; flags = virPCIDeviceRead16(dev, fd, pos + PCI_EXP_FLAGS); diff --git a/tests/virpcimock.c b/tests/virpcimock.c index a5cef46..49759b0 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -29,6 +29,7 @@ # include <fcntl.h> # include <sys/stat.h> # include <stdarg.h> +# include <dirent.h> # include "viralloc.h" # include "virstring.h" # include "virfile.h" @@ -42,6 +43,7 @@ static int (*real__xstat)(int ver, const char *path, struct stat *sb); static char *(*realcanonicalize_file_name)(const char *path); static int (*realopen)(const char *path, int flags, ...); static int (*realclose)(int fd); +static DIR * (*realopendir)(const char *name); /* Don't make static, since it causes problems with clang * when passed as an arg to virAsprintf() @@ -112,6 +114,7 @@ struct pciDevice { char *id; int vendor; int device; + int class; struct pciDriver *driver; /* Driver attached. NULL if attached to no driver */ }; @@ -351,6 +354,10 @@ pci_device_new_from_stub(const struct pciDevice *data) ABORT("@tmp overflow"); make_file(devpath, "device", tmp, -1); + if (snprintf(tmp, sizeof(tmp), "0x%.4x", dev->class) < 0) + ABORT("@tmp overflow"); + make_file(devpath, "class", tmp, -1); + if (pci_device_autobind(dev) < 0) ABORT("Unable to bind: %s", data->id); @@ -747,6 +754,7 @@ init_syms(void) LOAD_SYM(canonicalize_file_name); LOAD_SYM(open); LOAD_SYM(close); + LOAD_SYM(opendir); } static void @@ -776,6 +784,13 @@ init_env(void) MAKE_PCI_DEVICE("0000:00:01.0", 0x8086, 0x0044); MAKE_PCI_DEVICE("0000:00:02.0", 0x8086, 0x0046); MAKE_PCI_DEVICE("0000:00:03.0", 0x8086, 0x0048); + MAKE_PCI_DEVICE("0001:00:00.0", 0x1014, 0x03b9, .class = 0x060400); + MAKE_PCI_DEVICE("0001:01:00.0", 0x8086, 0x105e); + MAKE_PCI_DEVICE("0001:01:00.1", 0x8086, 0x105e); + MAKE_PCI_DEVICE("0005:80:00.0", 0x10b5, 0x8112, .class = 0x060400); + MAKE_PCI_DEVICE("0005:90:01.0", 0x1033, 0x0035); + MAKE_PCI_DEVICE("0005:90:01.1", 0x1033, 0x0035); + MAKE_PCI_DEVICE("0005:90:01.2", 0x1033, 0x00e0); } @@ -934,6 +949,24 @@ open(const char *path, int flags, ...) return ret; } +DIR * +opendir(const char *path) +{ + DIR *ret; + char *newpath = NULL; + + init_syms(); + + if (STRPREFIX(path, PCI_SYSFS_PREFIX) && + getrealpath(&newpath, path) < 0) + return NULL; + + ret = realopendir(newpath ? newpath : path); + + VIR_FREE(newpath); + return ret; +} + int close(int fd) { diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 5fe6d49..82a173a 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -183,6 +183,31 @@ cleanup: return ret; } +struct testPCIDevData { + unsigned int domain; + unsigned int bus; + unsigned int slot; + unsigned int function; +}; + +static int +testVirPCIDeviceIsAssignable(const void *opaque) +{ + const struct testPCIDevData *data = opaque; + int ret = -1; + virPCIDevicePtr dev; + + if (!(dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function))) + goto cleanup; + + if (virPCIDeviceIsAssignable(dev, true)) + ret = 0; + + virPCIDeviceFree(dev); +cleanup: + return ret; +} + # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" static int @@ -209,10 +234,19 @@ mymain(void) ret = -1; \ } while (0) +# define DO_TEST_PCI(fnc, domain, bus, slot, function) \ + do { \ + struct testPCIDevData data = { domain, bus, slot, function }; \ + if (virtTestRun(#fnc, fnc, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST(testVirPCIDeviceNew); DO_TEST(testVirPCIDeviceDetach); DO_TEST(testVirPCIDeviceReset); DO_TEST(testVirPCIDeviceReattach); + DO_TEST_PCI(testVirPCIDeviceIsAssignable, 5, 0x90, 1, 0); + DO_TEST_PCI(testVirPCIDeviceIsAssignable, 1, 1, 0, 0); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); diff --git a/tests/virpcitestdata/0001:00:00.0.config b/tests/virpcitestdata/0001:00:00.0.config new file mode 100644 index 0000000000000000000000000000000000000000..808d48993cfc0f41223fcb5f49deffc594f136b7 GIT binary patch literal 4096 zcmeH@I}XAy5Jca`5~RZg60MJb!~ys;a0<jx5^hCDNy!mt=t)n(8Yfir2x&(4YG(bB z{ig90wibyn0^=hytn<`78f&|D=zEvd5U8+Sxa1hw*nI*%I6fEDtkd58IUH=(-@Ei& z`ONZ@#r(MD|GagrnWu5_32tAW*RPg6sv;l)A|L`HAOa#F0wN#+A|L`H@J9q*PZkdc literal 0 HcmV?d00001 diff --git a/tests/virpcitestdata/0001:01:00.0.config b/tests/virpcitestdata/0001:01:00.0.config new file mode 100644 index 0000000000000000000000000000000000000000..f63aacbea08fc83b345e1ee404756145e7d349a5 GIT binary patch literal 4096 zcmZo`h!bFD5MW?qU|>>UXkY*WAi>nY$iN6<qW}>B2WF7K379CR5||9#X~qjmCm0kM z6j>iMK<#Fl0AdIL{c(^7NNY$jIHrJ{?<fFO0H%Qc6alFM0YL>exUYeN42%s7Ec~1x z877=QWd`bav)TXue_{D2AeEzFGz3ONU^E0qLtr!nMnhmU1V%$(Gz3ONU^E0qLtr!n I24e^S0CUn1m;e9( literal 0 HcmV?d00001 diff --git a/tests/virpcitestdata/0001:01:00.1.config b/tests/virpcitestdata/0001:01:00.1.config new file mode 100644 index 0000000000000000000000000000000000000000..db9e3876a528266bb5a6e9651d7cd18b45b7d71f GIT binary patch literal 4096 zcmZo`h!bFD5MW?qU|>>UXkcJqU;z?r4T=nmKsG865pZAziJX85Gr<@GPcvRnI>Dg8 zpvd~50qPj02_PQ`0R3^02S{s3F*v4xobM<ARRE?K85l)Csz5+cfer3!pdbTd0|N^` zCrE|~GcZ8HK)?*t@n*CC|Np}BPaq<r<Y)+thQMeDjE2By2#kinXb6mkz-S1JhQMeD NjE2By2n@au000k05W)Ze literal 0 HcmV?d00001 diff --git a/tests/virpcitestdata/0005:80:00.0.config b/tests/virpcitestdata/0005:80:00.0.config new file mode 100644 index 0000000000000000000000000000000000000000..cc4f94a849ed3d0a47f5b47331aaecdea88765c3 GIT binary patch literal 4096 zcmdlgAk@gtAi%JSfrX8Mfsp|Q8YawV_`sl`#L)Di>BE1RD1>%^ae>Mi1DK*fTmcO} zuqbOn1DJ*p0t|%^rUDm(pbry}Ey&Qo0}^ro5pYlpXVI6zg5+oV+Kg)3K=~il6$8f` z_5Z+y{a~<pH2w!eEDW3*M&p0r!hSH=JR1LlAr=PC4WsctaA7|fY#xpO!4L}r=Z4Yv IADFNQ0PH*z9smFU literal 0 HcmV?d00001 diff --git a/tests/virpcitestdata/0005:90:01.0.config b/tests/virpcitestdata/0005:90:01.0.config new file mode 100644 index 0000000000000000000000000000000000000000..a60599bd342d3ebcdc7b8367ca36ad337f602fde GIT binary patch literal 256 zcmXpOFlAt45MXi^VCG@qXkY+>CJ=!Q7z5RUfCHEW5{!&mj0{Y5Fz#TaS&cX3;ByxM DCDjDB literal 0 HcmV?d00001 diff --git a/tests/virpcitestdata/0005:90:01.1.config b/tests/virpcitestdata/0005:90:01.1.config new file mode 100644 index 0000000000000000000000000000000000000000..beee76534041a7020c08ae9ac03d9a349c6ea12e GIT binary patch literal 256 ycmXpOFlAt45MXi^VCG@qU|?VnU}yr8Sb;H6EeJS(Ng%<*sKv;@R0rb@MH&E<&;s)S literal 0 HcmV?d00001 diff --git a/tests/virpcitestdata/0005:90:01.2.config b/tests/virpcitestdata/0005:90:01.2.config new file mode 100644 index 0000000000000000000000000000000000000000..cfd72e4d7165bff2751ecbdc570ce7f1e0646226 GIT binary patch literal 256 zcmXpOc)-BMAi%_;z|6zo!oa|wz|aIFu>xbDS`csmlR$!5K#7rosSd`)Mk^@TV-u#E T7_0Gy9FS#<5E~CbC<F-rZiWW} literal 0 HcmV?d00001 -- 1.7.1

3 4

[libvirt] [PATCH] Fix bridge configuration when OUTPUT policy is DROP on the host
by Lénaïc Huard 08 Jan '14

08 Jan '14

When the host is configured with very restrictive firewall (default policy is DROP for all chains, including OUTPUT), the bridge driver for Linux adds netfilter entries to allow DHCP and DNS requests to go from the VM to the dnsmasq of the host. The issue that this commit fixes is the fact that a DROP policy on the OUTPUT chain blocks the DHCP replies from the host’s dnsmasq to the VM. As DHCP replies are sent in UDP, they are not caught by any --ctstate ESTABLISHED rule and so, need to be explicitly allowed. Signed-off-by: Lénaïc Huard <lenaic(a)lhuard.fr.eu.org> --- src/libvirt_private.syms | 2 ++ src/network/bridge_driver_linux.c | 35 +++++++++++++--------- src/util/viriptables.c | 61 +++++++++++++++++++++++++++++++++++++++ src/util/viriptables.h | 7 +++++ 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2dbb8f8..c26ce29 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1290,6 +1290,7 @@ iptablesAddForwardRejectOut; iptablesAddOutputFixUdpChecksum; iptablesAddTcpInput; iptablesAddUdpInput; +iptablesAddUdpOutput; iptablesRemoveDontMasquerade; iptablesRemoveForwardAllowCross; iptablesRemoveForwardAllowIn; @@ -1301,6 +1302,7 @@ iptablesRemoveForwardRejectOut; iptablesRemoveOutputFixUdpChecksum; iptablesRemoveTcpInput; iptablesRemoveUdpInput; +iptablesRemoveUdpOutput; # util/virjson.h diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 066779a..dcdd033 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -564,6 +564,13 @@ int networkAddGeneralFirewallRules(virNetworkObjPtr network) goto err2; } + if (iptablesAddUdpOutput(AF_INET, network->def->bridge, 68) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to add iptables rule to allow DHCP replies to '%s'"), + network->def->bridge); + goto err3; + } + /* If we are doing local DHCP service on this network, attempt to * add a rule that will fixup the checksum of DHCP response * packets back to the guests (but report failure without @@ -582,14 +589,14 @@ int networkAddGeneralFirewallRules(virNetworkObjPtr network) virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow DNS requests from '%s'"), network->def->bridge); - goto err3; + goto err4; } if (iptablesAddUdpInput(AF_INET, network->def->bridge, 53) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow DNS requests from '%s'"), network->def->bridge); - goto err4; + goto err5; } /* allow TFTP requests through to dnsmasq if necessary */ @@ -598,7 +605,7 @@ int networkAddGeneralFirewallRules(virNetworkObjPtr network) virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow TFTP requests from '%s'"), network->def->bridge); - goto err5; + goto err6; } /* Catch all rules to block forwarding to/from bridges */ @@ -607,14 +614,14 @@ int networkAddGeneralFirewallRules(virNetworkObjPtr network) virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to block outbound traffic from '%s'"), network->def->bridge); - goto err6; + goto err7; } if (iptablesAddForwardRejectIn(AF_INET, network->def->bridge) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to block inbound traffic to '%s'"), network->def->bridge); - goto err7; + goto err8; } /* Allow traffic between guests on the same bridge */ @@ -622,31 +629,33 @@ int networkAddGeneralFirewallRules(virNetworkObjPtr network) virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to add iptables rule to allow cross bridge traffic on '%s'"), network->def->bridge); - goto err8; + goto err9; } /* add IPv6 general rules, if needed */ if (networkAddGeneralIp6tablesRules(network) < 0) { - goto err9; + goto err10; } return 0; /* unwind in reverse order from the point of failure */ -err9: +err10: iptablesRemoveForwardAllowCross(AF_INET, network->def->bridge); -err8: +err9: iptablesRemoveForwardRejectIn(AF_INET, network->def->bridge); -err7: +err8: iptablesRemoveForwardRejectOut(AF_INET, network->def->bridge); -err6: +err7: if (ipv4def && ipv4def->tftproot) { iptablesRemoveUdpInput(AF_INET, network->def->bridge, 69); } -err5: +err6: iptablesRemoveUdpInput(AF_INET, network->def->bridge, 53); -err4: +err5: iptablesRemoveTcpInput(AF_INET, network->def->bridge, 53); +err4: + iptablesRemoveUdpOutput(AF_INET, network->def->bridge, 68); err3: iptablesRemoveUdpInput(AF_INET, network->def->bridge, 67); err2: diff --git a/src/util/viriptables.c b/src/util/viriptables.c index a1711bb..9b78d86 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -184,6 +184,28 @@ iptablesInput(int family, NULL); } +static int +iptablesOutput(int family, + const char *iface, + int port, + int action, + int tcp) +{ + char portstr[32]; + + snprintf(portstr, sizeof(portstr), "%d", port); + portstr[sizeof(portstr) - 1] = '\0'; + + return iptablesAddRemoveRule("filter", "OUTPUT", + family, + action, + "--out-interface", iface, + "--protocol", tcp ? "tcp" : "udp", + "--destination-port", portstr, + "--jump", "ACCEPT", + NULL); +} + /** * iptablesAddTcpInput: * @ctx: pointer to the IP table context @@ -262,6 +284,45 @@ iptablesRemoveUdpInput(int family, return iptablesInput(family, iface, port, REMOVE, 0); } +/** + * iptablesAddUdpOutput: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port to add + * + * Add an output to the IP table allowing access to the given @port from + * the given @iface interface for UDP packets + * + * Returns 0 in case of success or an error code in case of error + */ + +int +iptablesAddUdpOutput(int family, + const char *iface, + int port) +{ + return iptablesOutput(family, iface, port, ADD, 0); +} + +/** + * iptablesRemoveUdpOutput: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port to remove + * + * Removes an output from the IP table, hence forbidding access to the given + * @port from the given @iface interface for UDP packets + * + * Returns 0 in case of success or an error code in case of error + */ +int +iptablesRemoveUdpOutput(int family, + const char *iface, + int port) +{ + return iptablesOutput(family, iface, port, REMOVE, 0); +} + static char *iptablesFormatNetwork(virSocketAddr *netaddr, unsigned int prefix) diff --git a/src/util/viriptables.h b/src/util/viriptables.h index 6dfb992..2f9a212 100644 --- a/src/util/viriptables.h +++ b/src/util/viriptables.h @@ -40,6 +40,13 @@ int iptablesRemoveUdpInput (int family, const char *iface, int port); +int iptablesAddUdpOutput (int family, + const char *iface, + int port); +int iptablesRemoveUdpOutput (int family, + const char *iface, + int port); + int iptablesAddForwardAllowOut (virSocketAddr *netaddr, unsigned int prefix, const char *iface, -- 1.8.5.1

2 1

[libvirt] [PATCH] qemu: auto-create pci controller alias if missing from domain status
by Laine Stump 08 Jan '14

08 Jan '14

https://bugzilla.redhat.com/show_bug.cgi?id=1047429 The problem occurs when a host with a running guest is upgraded from pre-1.0.2 libvirt, and then a later attempt is made to hotplug a new PCI device (emulated or passthrough) to that guest without having power cycled / save-restored / migrated the guest. The hotplug fails with this error: internal error: Device alias was not set for PCI controller with index 0 required for device at address 0000:00:xx.x This happens because the pci controller newly auto-created for the status will not contain an alias because 1) assigning an alias is normally left up to the hypervisor driver to do at domain startup, or controller attach time. 2) under the older libvirt, the PCI controller was implicit, so it was never assigned an alias. 3) when libvirtd is restarted and re-attaches to each domain, there is no "re-attach device" operation, so the no-longer-implicit PCI controller will not get an alias assigned either. When explicit PCI controllers were first added (shortly before libvirt 1.0.2), the code was written such that each controller would get an alias named "pciN" placed in its status, but the commandlines would all ignore that and instead use "pci.N". This was changed in commit 01b8812 so that 1) the proper "pci.N" alias was stored in the status, 2) the alias name from the status was used when creating the qemu commandline, and 3) when a PCI device was being hotplugged, if the status for its PCI controller had no alias assigned, an internal error would be generated. This patch changes that error condition to instead make a temporary string to contain the required alias "pci.N" and put that in the proper place in the commandline. It does not fill in the alias in the status object just in case that might create some other problem (it didn't seem very polite for the qemuBuildDeviceAddressStr() for one device to be modifying the status object for a different device). Note that this does *not* address a similar problem that would occur when libvirt was upgraded from a version between 1.0.2 and 1.1.2 to a newer version - in these cases the status would contain "pciN" as the alias name, so qemuBuildDeviceAddressStr() would be successful, but the resulting command would fail (since the qemu process has no controller named "pciN", but instead has one named "pci.N"); the problem can anyway be worked around by restarting / save-restoring / migrating the domain, and solving this second problem would require special-casing a particular alias name pattern rather than just handling a missing name, which seems just a bit too ugly to have permanently cluttering the code. --- (Yes, another exceedingly long commit log message for a very short patch :-) I'm not 100% convinced that it's necessary to handle *this* case either (upgrading from pre-1.0.2), but thought I should send the patch to the list anyway to see what others think. My opinion is that it's creating permanent clutter in the code in order to handle a situation that will come up just once in the entire lifetime of a host (and even then only if libvirt is upgraded without somehow restarting a guest, and *then* a device is hotplugged to that guest), so it may be better if we just left the workaround ("restart/save-restore/migrate the domain if you need to hotplug a device") as a footnote somewhere instead. src/qemu/qemu_command.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d723dc8..a562e18 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1,7 +1,7 @@ /* * qemu_command.c: QEMU command generation * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -3021,6 +3021,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, { int ret = -1; char *devStr = NULL; + char *tempAlias = NULL; if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { const char *contAlias = NULL; @@ -3035,12 +3036,16 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, cont->idx == info->addr.pci.bus) { contAlias = cont->info.alias; if (!contAlias) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Device alias was not set for PCI " - "controller with index %u required " - "for device at address %s"), - info->addr.pci.bus, devStr); - goto cleanup; + /* this could be a case where the domain had been + * started with an earlier version of libvirt that + * had no device entry for the PCI bus + * (pre-1.0.2), and in that case no alias would be + * set in the status XML, so we create one using + * the standard "pci.N" pattern. + */ + if (virAsprintf(&tempAlias, "pci.%u", info->addr.pci.bus) < 0) + goto cleanup; + contAlias = tempAlias; } break; } @@ -3118,6 +3123,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, ret = 0; cleanup: VIR_FREE(devStr); + VIR_FREE(tempAlias); return ret; } -- 1.8.4.2

2 3

[libvirt] [PATCH] lxc_controller: Don't leak @name
by Michal Privoznik 08 Jan '14

08 Jan '14

The @name variable is VIR_STRDUP()-ed into, but never freed. In fact, there's no need to duplicate a command line argument since all places where @name is used expect const char. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/lxc/lxc_controller.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c013147..1b43bd2 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2249,7 +2249,7 @@ int main(int argc, char *argv[]) { pid_t pid; int rc = -1; - char *name = NULL; + const char *name = NULL; size_t nveths = 0; char **veths = NULL; int handshakeFd = -1; @@ -2300,8 +2300,7 @@ int main(int argc, char *argv[]) break; case 'n': - if (VIR_STRDUP(name, optarg) < 0) - goto cleanup; + name = optarg; break; case 'v': -- 1.8.5.1

2 3