- Devel - libvirt List Archives

[libvirt] NFS over RDMA small block DIRECT_IO bug
by Andrew Holway 20 Sep '12

20 Sep '12

Hello. # Avi Kivity avi(a)redhat recommended I copy kvm in on this. It would also seem relevent to libvirt. # I have a Centos 6.2 server and Centos 6.2 client. [root@store ~]# cat /etc/exports /dev/shm 10.149.0.0/16(rw,fsid=1,no_root_squash,insecure) (I have tried with non tempfs targets also) [root@node001 ~]# cat /etc/fstab store.ibnet:/dev/shm /mnt nfs rdma,port=2050,defaults 0 0 I wrote a little for loop one liner that dd'd the centos net install image to a file called 'hello' then checksummed that file. Each iteration uses a different block size. Non DIRECT_IO seems to work fine. DIRECT_IO with 512byte, 1K and 2K block sizes get corrupted. I want to run my KVM guests on top of NFS over RDMA. My guests cannot create filesystems. Thanks, Andrew. bug report: https://bugzilla.linux-nfs.org/show_bug.cgi?id=228 [root@node001 mnt]# for f in 512 1024 2048 4096 8192 16384 32768 65536 131072; do dd bs="$f" if=CentOS-6.3-x86_64-netinstall.iso of=hello iflag=direct oflag=direct && md5sum hello && rm -f hello; done 409600+0 records in 409600+0 records out 209715200 bytes (210 MB) copied, 62.3649 s, 3.4 MB/s aadd0ffe3c9dfa35d8354e99ecac9276 hello -- 512 byte block 204800+0 records in 204800+0 records out 209715200 bytes (210 MB) copied, 41.3876 s, 5.1 MB/s 336f6da78f93dab591edc18da81f002e hello -- 1K block 102400+0 records in 102400+0 records out 209715200 bytes (210 MB) copied, 21.1712 s, 9.9 MB/s f4cefe0a05c9b47ba68effdb17dc95d6 hello -- 2k block 51200+0 records in 51200+0 records out 209715200 bytes (210 MB) copied, 10.9631 s, 19.1 MB/s 690138908de516b6e5d7d180d085c3f3 hello -- 4k block 25600+0 records in 25600+0 records out 209715200 bytes (210 MB) copied, 5.4136 s, 38.7 MB/s 690138908de516b6e5d7d180d085c3f3 hello 12800+0 records in 12800+0 records out 209715200 bytes (210 MB) copied, 3.1448 s, 66.7 MB/s 690138908de516b6e5d7d180d085c3f3 hello 6400+0 records in 6400+0 records out 209715200 bytes (210 MB) copied, 1.77304 s, 118 MB/s 690138908de516b6e5d7d180d085c3f3 hello 3200+0 records in 3200+0 records out 209715200 bytes (210 MB) copied, 1.4331 s, 146 MB/s 690138908de516b6e5d7d180d085c3f3 hello 1600+0 records in 1600+0 records out 209715200 bytes (210 MB) copied, 0.922167 s, 227 MB/s 690138908de516b6e5d7d180d085c3f3 hello

4 8

[libvirt] [PATCH] build: force libnl1 if netcf also used libnl1
by Eric Blake 20 Sep '12

20 Sep '12

Recent spec file changes ensure that in distro situations, netcf and libvirt will link against the same libnl in order to avoid dumping core. But for every-day development, if you are F17 and have the libnl3-devel headers available, libvirt was blindly linking against libnl3 even though F17 netcf still links against libnl1, making testing a self-built binary on F17 impossible. By making configure a little bit smarter, we can avoid this situation. I intentionally wrote the test so that we still favor libnl-3 if netcf is not installed or if we couldn't use ldd to determine which library netcf linked against. * configure.ac (LIBNL): Don't probe libnl3 if netcf doesn't use it. --- Does this patch look safe enough to use? It was sufficient to let me resume self-tests on my F17 box, where I had intentionally installed libnl3-devel. configure.ac | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 47a72b9..7528894 100644 --- a/configure.ac +++ b/configure.ac @@ -2902,14 +2902,24 @@ LIBNL_LIBS="" have_libnl=no if test "$with_linux" = "yes"; then - PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ - have_libnl=yes - AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) - AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) - PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) - LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" - LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" - ], [PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ + # When linking with netcf, we must ensure that we pick the same version + # of libnl that netcf picked. Prefer libnl-3 unless we can prove + # netcf linked against libnl-1. + ncftool=`which ncftool` + case `(ldd "$ncftool") 2>&1` in + *libnl.so.1*) ;; + *) + PKG_CHECK_MODULES([LIBNL], [libnl-3.0], [ + have_libnl=yes + AC_DEFINE([HAVE_LIBNL3], [1], [Use libnl-3.0]) + AC_DEFINE([HAVE_LIBNL], [1], [whether the netlink library is available]) + PKG_CHECK_MODULES([LIBNL_ROUTE3], [libnl-route-3.0]) + LIBNL_CFLAGS="$LIBNL_CFLAGS $LIBNL_ROUTE3_CFLAGS" + LIBNL_LIBS="$LIBNL_LIBS $LIBNL_ROUTE3_LIBS" + ], []) ;; + esac + if test "$have_libnl" = no; then + PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ have_libnl=yes AC_DEFINE_UNQUOTED([HAVE_LIBNL], [1], [whether the netlink library is available]) @@ -2920,7 +2930,7 @@ if test "$with_linux" = "yes"; then AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) fi ]) - ]) + fi fi AM_CONDITIONAL([HAVE_LIBNL], [test "$have_libnl" = "yes"]) -- 1.7.11.4

7 23

[libvirt] [PATCH] network: fix incorrect VIR_NETWORK_UPDATE_COMMAND_* values
by Laine Stump 19 Sep '12

19 Sep '12

These enums originally were put into the flags for virNetworkUpdate, and when they were moved into their own enum, the numbers weren't appropriately changed, causing the commands to start with value 2 instead of 1. This causes problems for things like ENUM_IMPL, which wants a string for every value in the requested range, including those not used in the enum. --- include/libvirt/libvirt.h.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84ac2d0..0f67cbb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2356,10 +2356,10 @@ int virNetworkUndefine (virNetworkPtr network); */ typedef enum { VIR_NETWORK_UPDATE_COMMAND_NONE = 0, /* (invalid) */ - VIR_NETWORK_UPDATE_COMMAND_MODIFY = 2, /* modify an existing element */ - VIR_NETWORK_UPDATE_COMMAND_DELETE = 3, /* delete an existing element */ - VIR_NETWORK_UPDATE_COMMAND_ADD_LAST = 4, /* add an element at end of list */ - VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST = 5, /* add an element at start of list */ + VIR_NETWORK_UPDATE_COMMAND_MODIFY = 1, /* modify an existing element */ + VIR_NETWORK_UPDATE_COMMAND_DELETE = 2, /* delete an existing element */ + VIR_NETWORK_UPDATE_COMMAND_ADD_LAST = 3, /* add an element at end of list */ + VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST = 4, /* add an element at start of list */ #ifdef VIR_ENUM_SENTINELS VIR_NETWORK_UPDATE_COMMAND_LAST #endif -- 1.7.11.4

2 2

[libvirt] [PATCHv2 0/9] new virNetworkUpdate API
by Laine Stump 19 Sep '12

19 Sep '12

===== Changes from V1: 1) implemented Eric's suggested change to make "command" a separate arg rather than squeezing it into the flags 2) already pushed the first two ACKed patches (not directly related to new API 3) added new patch at the end implementing updates of dhcp host entries. This patchset implements a new API function called virNetworkUpdate which enables updating certain parts of a libvirt network's definition without the need to destroy/re-start the network. This is especially useful, for example, to add/remove hosts from the dhcp static hosts table, or change portgroup settings. This was previously discussed in this thread: https://www.redhat.com/archives/libvir-list/2012-August/msg01535.html continuing here in September: https://www.redhat.com/archives/libvir-list/2012-September/msg00328.html with the final form here: https://www.redhat.com/archives/libvir-list/2012-September/msg00465.html In short, the single function has a "section" specifier which tells the part of the network definition to be updated, a "parentIndex" that gives the index of the *parent* element containing this section (when there are multiples - in particular in the case of the <ip> element), and a fully formed XML element which will be added as-is in the case of VIR_NETWORK_UPDATE_ADD_* (after checking for a duplicate), used to search for the specific element to delete in case of VIR_NETWORK_UPDATE_DELETE, and used both to find the existing element and replace its current contents in the case of VIR_UPDATE_EXISTING (this implies that you can't change the change the attribute used for indexing, e.g. the name of a portgroup, or mac address of a dhcp host entry). An example of use: to add a dhcp host entry to network "net", you would do this: virNetworkUpdate(net, VIR_NETWORK_UPDATE_COMMAND_ADD_LAST, VIR_NETWORK_SECTION_IP_DHCP_HOST, -1, "<host mac='00:11:22:33:44:55' ip='192.168.122.5'/>", VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG); To delete that same entry: virNetworkUpdate(net, VIR_NETWORK_UPDATE_COMMAND_DELETE, VIR_NETWORK_SECTION_IP_DHCP_HOST, -1, "<host mac='00:11:22:33:44:55'/>", VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG); If you wanted to force any of these to affect the dhcp host list in the 3rd <ip> element of the network, you would replace "-1" with "2". Another example: to modify the portgroup named "engineering" (e.g. to increase the inbound average bandwidth from 1000 to 2000): virNetworkUpdate(net, VIR_NETWORK_UPDATE_COMMAND_MODIFY, VIR_NETWORK_SECTION_PORTGROUP, -1, "<portgroup name='engineering' default='yes'>" " <virtualport type='802.1Qbh'>" " <parameters profileid='test'/>" " </virtualport>" " <bandwidth>" " <inbound average='2000' peak='5000' burst='5120'/>" " <outbound average='1000' peak='5000' burst='5120'/>" " </bandwidth>" "</portgroup>", VIR_NETWORK_UPDATE_LIVE | VIR_NETWORK_UPDATE_CONFIG) (note that parentIndex is irrelevant for PORTGROUP, since they are in the toplevel of <network>, so there aren't multiple instances of parents. In such cases, the caller *must* set parentIndex to -1 or 0 - any other value indicates that they don't understand the purpose/usage of parentIndex, so it must result in an error. Also note that the above function would fail if it couldn't find an existing portgroup with name='engineering' (i.e. it wouldn't automatically add a new one).) Adding support for each of the different sections has been reduced to a single function that handles the update of a virNetworkDef; all the logic to determine which virNetworkDef (def or newDef) and to restart/SIGHUP the appropriate daemons is in higher levels and is 100% complete. The low level functions aren't yet finished, although the function for IP_DHCP_HOST is nearly done. As usual, several of the patches are re-factoring existing code, and a couple are bugfixes that are only peripherally related: 1/9+2/9 - actual API 3/9 - utility functions to simplify API implementation 4/9 - framework for backend that updates the virNetworkDef 5/9 - refactoring in bridge_driver 6/9 - virNetworkUpdate for bridge_driver 7/9 - virNetworkUpdate for test_driver 8/9 - simple troubleshooting aid - restart dnsmasq/radvd when libvirtd is restarted (if its process is missing). 9/9 - implement backend for VIR_NETWORK_SECTION_IP_DHCP_HOST

3 21

[libvirt] [PATCH] virsh: Fix resource leaks when editing files.
by Peter Krempa 19 Sep '12

19 Sep '12

The cleanup path in virsh-edit helper was never reached when the edit was successful leaking the document in memory as well as the temporary file. --- tools/virsh-edit.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c index 512ac0d..0ed0e8f 100644 --- a/tools/virsh-edit.c +++ b/tools/virsh-edit.c @@ -68,6 +68,7 @@ do { char *doc_edited = NULL; char *doc_reread = NULL; const char *msg = NULL; + bool edit_success = false; /* Get the XML configuration of the object. */ doc = (EDIT_GET_XML); @@ -139,7 +140,7 @@ redefine: } } - break; + edit_success = true; edit_cleanup: VIR_FREE(doc); @@ -149,7 +150,9 @@ edit_cleanup: unlink (tmp); VIR_FREE(tmp); } - goto cleanup; + + if (!edit_success) + goto cleanup; } while (0); -- 1.7.12

2 2

[libvirt] [PATCH] util: don't free dmidecode path string before printing it
by Ján Tomko 19 Sep '12

19 Sep '12

The path was freed before printing the error message, resulting in: error : virSysinfoRead:773 : internal error Failed to execute command (null) --- src/util/sysinfo.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 92c3539..8ecd1e3 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -765,7 +765,6 @@ virSysinfoRead(void) { } cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL); - VIR_FREE(path); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -796,6 +795,7 @@ virSysinfoRead(void) { goto no_memory; cleanup: + VIR_FREE(path); VIR_FREE(outbuf); virCommandFree(cmd); -- 1.7.8.6

3 3

[libvirt] [PATCH] security: Don't ignore errors when parsing DAC security labels
by Peter Krempa 19 Sep '12

19 Sep '12

The DAC security driver silently ignored errors when parsing the DAC label and used default values instead. With a domain containing the following label definition: <seclabel type='static' model='dac' relabel='yes'> <label>sdfklsdjlfjklsdjkl</label> </seclabel> the domain would start normaly but the disk images would be still owned by root and no error was displayed. This patch changes the behavior if the parsing of the label fails (note that a not present label is not a failure and in this case the default label should be used) the error isn't masked but is raised that causes the domain start to fail with a descriptive error message: virsh # start tr error: Failed to start domain tr error: invalid argument: failed to parse uid and gid for DAC security driver: sdfklsdjlfjklsdjkl I also changed the error code to "invalid argument" from "internal error". --- src/security/security_dac.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 211fb37..c669496 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -140,6 +140,8 @@ int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, return -1; } + +/* returns 1 if label isn't found, 0 on success, -1 on error */ static int virSecurityDACParseImageIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) @@ -149,19 +151,19 @@ int virSecurityDACParseImageIds(virDomainDefPtr def, virSecurityLabelDefPtr seclabel; if (def == NULL) - return -1; + return 1; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (seclabel == NULL || seclabel->imagelabel == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label for DAC not found in domain %s"), def->name); - return -1; + return 1; } if (seclabel->imagelabel && parseIds(seclabel->imagelabel, &uid, &gid)) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_INVALID_ARG, _("failed to parse uid and gid for DAC " "security driver: %s"), seclabel->label); return -1; @@ -179,8 +181,10 @@ static int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, uid_t *uidPtr, gid_t *gidPtr) { - if (virSecurityDACParseImageIds(def, uidPtr, gidPtr) == 0) - return 0; + int ret; + + if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) <= 0) + return ret; if (priv) { if (uidPtr) -- 1.7.12

2 2

[libvirt] [PATCH 0/4] Add support for reboot-timeout
by Martin Kletzander 19 Sep '12

19 Sep '12

This series introduces simple reboot timeout support. That means what should be done in case all boot options fail, reboot or not. And if yes, then how long the machine should wait. Before adding the support for that into XML parser, builder, and qemu, I felt the need to cleanup the boot parsing and enums (I couldn't look at that), so that's the first patch. Most of it is mechanical, the rest should be pretty straight-forward. --- I'd *love* to have it in 0.10.2, but I understand if it's too late :( Martin Kletzander (4): qemu: Cleanup boot parameter building Add support for reboot-timeout qemu: Add support for reboot-timeout QEMU Tests for reboot-timeout docs/formatdomain.html.in | 9 ++ docs/schemas/domaincommon.rng | 20 ++++ src/conf/domain_conf.c | 56 ++++++++- src/conf/domain_conf.h | 78 ++++++++----- src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 129 +++++++++++++++++---- tests/qemuargv2xmltest.c | 4 + ...xml2argv-boot-menu-disable-drive-bootindex.args | 1 + .../qemuxml2argv-reboot-timeout-disabled.args | 3 + .../qemuxml2argv-reboot-timeout-disabled.xml | 21 ++++ .../qemuxml2argv-reboot-timeout-enabled.args | 3 + .../qemuxml2argv-reboot-timeout-enabled.xml | 21 ++++ tests/qemuxml2argvtest.c | 5 + tests/qemuxml2xmltest.c | 4 + 16 files changed, 304 insertions(+), 57 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-enabled.xml -- 1.7.12

3 11

[libvirt] [PATCH 0/3] more build fixes
by Eric Blake 19 Sep '12

19 Sep '12

Pushing all three under the build-breaker rules. Eric Blake (3): bitmap: fix problems in previous commit build: fix nodeinfo build on non-Linux platforms build: avoid unused symbol src/libvirt_private.syms | 1 - src/nodeinfo.c | 4 ++++ src/util/bitmap.c | 3 +-- tests/virbitmaptest.c | 12 +++++++----- 4 files changed, 12 insertions(+), 8 deletions(-) -- 1.7.11.4

2 6

[libvirt] [PATCH] Ensure existing selinux mount is removed before mounting new one in LXC
by Daniel P. Berrange 19 Sep '12

19 Sep '12

Some kernel versions (at least RHEL-6 2.6.32) do not let you over-mount an existing selinuxfs instance with a new one. Thus we must unmount the existing instance inside our namespace. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/lxc/lxc_container.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9bc5610..f5b0bc6 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1533,6 +1533,14 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, lxcContainerUnmountSubtree("/proc", false) < 0)) goto cleanup; +#if HAVE_SELINUX + /* Some versions of Linux kernel don't let you overmount + * the selinux filesystem, so make sure we kill it first + */ + if (lxcContainerUnmountSubtree(SELINUX_MOUNT, false) < 0) + goto cleanup; +#endif + /* Mounts the core /proc, /sys, etc filesystems */ if (lxcContainerMountBasicFS(true, sec_mount_options) < 0) goto cleanup; @@ -1615,6 +1623,14 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, lxcContainerUnmountSubtree("/proc", false) < 0) goto cleanup; +#if HAVE_SELINUX + /* Some versions of Linux kernel don't let you overmount + * the selinux filesystem, so make sure we kill it first + */ + if (lxcContainerUnmountSubtree(SELINUX_MOUNT, false) < 0) + goto cleanup; +#endif + /* Mounts the core /proc, /sys, etc filesystems */ if (lxcContainerMountBasicFS(false, sec_mount_options) < 0) goto cleanup; -- 1.7.1

2 1