May 2013 - Devel - libvirt List Archives

[libvirt] [PATCH 0/4] Extra validation for the <sysinfo> section
by John Ferlan 14 May '13

14 May '13

Not necessary for 1.0.5, but figured I'd put it out there now. https://bugzilla.redhat.com/show_bug.cgi?id=890494 The <sysinfo> section needs an extra uuid validation check. The current check compares the numerical sysinfo/system_uuid with the domain uuid; however, it's possible that someone added extra hyphens into the sysinfo uuid which results in the inability to start the domain (at least qemu). The bug report indicates the 'date' field should at least be syntax checked based on what's desribed in the SMBIOS spec. From the spec: "String number of the BIOS release date. The date string, if supplied, is in either mm/dd/yy or mm/dd/yyyy format. If the year portion of the string is two digits, the year is assumed to be 19yy. NOTE: The mm/dd/yyyy format is required for SMBIOS version 2.3 and later" John Ferlan (4): docs: Fix syntax in sysinfo description docs: Update description of SMBIOS fields Validate the bios_date format for <sysinfo> Need better validation of <sysinfo> uuid docs/formatdomain.html.in | 51 ++++++++++++++++++++++++++++++++++++++--------- src/conf/domain_conf.c | 47 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 85 insertions(+), 13 deletions(-) -- 1.8.1.4

3 19

[libvirt] [PATCH] build: avoid shadowed variable in fdstreamtest
by Eric Blake 14 May '13

14 May '13

On RHEL 6.4 (gcc 4.4.7), I got: fdstreamtest.c: In function 'testFDStreamReadCommon': fdstreamtest.c:44: error: declaration of 'tmpfile' shadows a global declaration [-Wshadow] * tests/fdstreamtest.c (testFDStreamReadCommon) (testFDStreamWriteCommon): Rename 'tmpfile' variable. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. tests/fdstreamtest.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/fdstreamtest.c b/tests/fdstreamtest.c index cda2689..9f780c9 100644 --- a/tests/fdstreamtest.c +++ b/tests/fdstreamtest.c @@ -41,7 +41,7 @@ static int testFDStreamReadCommon(const char *scratchdir, bool blocking) { int fd = -1; - char *tmpfile = NULL; + char *file = NULL; int ret = -1; char *pattern = NULL; char *buf = NULL; @@ -63,10 +63,10 @@ static int testFDStreamReadCommon(const char *scratchdir, bool blocking) for (i = 0 ; i < PATTERN_LEN ; i++) pattern[i] = i; - if (virAsprintf(&tmpfile, "%s/input.data", scratchdir) < 0) + if (virAsprintf(&file, "%s/input.data", scratchdir) < 0) goto cleanup; - if ((fd = open(tmpfile, O_CREAT|O_WRONLY|O_EXCL, 0600)) < 0) + if ((fd = open(file, O_CREAT|O_WRONLY|O_EXCL, 0600)) < 0) goto cleanup; for (i = 0 ; i < 10 ; i++) { @@ -83,7 +83,7 @@ static int testFDStreamReadCommon(const char *scratchdir, bool blocking) /* Start reading 1/2 way through first pattern * and end 1/2 way through last pattern */ - if (virFDStreamOpenFile(st, tmpfile, + if (virFDStreamOpenFile(st, file, PATTERN_LEN / 2, PATTERN_LEN * 9, O_RDONLY) < 0) goto cleanup; @@ -149,11 +149,11 @@ cleanup: if (st) virStreamFree(st); VIR_FORCE_CLOSE(fd); - if (tmpfile != NULL) - unlink(tmpfile); + if (file != NULL) + unlink(file); if (conn) virConnectClose(conn); - VIR_FREE(tmpfile); + VIR_FREE(file); VIR_FREE(pattern); VIR_FREE(buf); return ret; @@ -173,7 +173,7 @@ static int testFDStreamReadNonblock(const void *data) static int testFDStreamWriteCommon(const char *scratchdir, bool blocking) { int fd = -1; - char *tmpfile = NULL; + char *file = NULL; int ret = -1; char *pattern = NULL; char *buf = NULL; @@ -195,7 +195,7 @@ static int testFDStreamWriteCommon(const char *scratchdir, bool blocking) for (i = 0 ; i < PATTERN_LEN ; i++) pattern[i] = i; - if (virAsprintf(&tmpfile, "%s/input.data", scratchdir) < 0) + if (virAsprintf(&file, "%s/input.data", scratchdir) < 0) goto cleanup; if (!(st = virStreamNew(conn, flags))) @@ -204,7 +204,7 @@ static int testFDStreamWriteCommon(const char *scratchdir, bool blocking) /* Start writing 1/2 way through first pattern * and end 1/2 way through last pattern */ - if (virFDStreamCreateFile(st, tmpfile, + if (virFDStreamCreateFile(st, file, PATTERN_LEN / 2, PATTERN_LEN * 9, O_WRONLY, 0600) < 0) goto cleanup; @@ -244,7 +244,7 @@ static int testFDStreamWriteCommon(const char *scratchdir, bool blocking) goto cleanup; } - if ((fd = open(tmpfile, O_RDONLY)) < 0) + if ((fd = open(file, O_RDONLY)) < 0) goto cleanup; for (i = 0 ; i < 10 ; i++) { @@ -292,11 +292,11 @@ cleanup: if (st) virStreamFree(st); VIR_FORCE_CLOSE(fd); - if (tmpfile != NULL) - unlink(tmpfile); + if (file != NULL) + unlink(file); if (conn) virConnectClose(conn); - VIR_FREE(tmpfile); + VIR_FREE(file); VIR_FREE(pattern); VIR_FREE(buf); return ret; -- 1.8.1.4

1 0

[libvirt] [PATCH] Don't overwrite useful message when creating macvlan fails
by Daniel P. Berrange 14 May '13

14 May '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Currently we report a bogus error message when macvlan creation fails: error: Failed to start domain migtest error: operation failed: Unable to create macvlan device With this removed, we see the real error: error: Failed to start domain migtest error: Unable to get index for interface p31p1: No such device Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/util/virnetdevmacvlan.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 0c4fcbd..5316520 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -897,11 +897,8 @@ create_name: } virMutexUnlock(&virNetDevMacVLanCreateMutex); - if (!cr_ifname) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("Unable to create macvlan device")); + if (!cr_ifname) return -1; - } } if (virNetDevVPortProfileAssociate(cr_ifname, -- 1.8.2.1

2 1

[libvirt] [PATCH] Remove & ban use of select() for waiting for I/O
by Daniel P. Berrange 14 May '13

14 May '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Use of the select() system call is inherantly dangerous since applications will hit a buffer overrun if any FD number exceeds the size of the select set size (typically 1024). Replace the two uses of select() with poll() and use cfg.mk to ban any future use of select(). NB: This changes the phyp driver so that it uses an infinite timeout, instead of busy-waiting for 1ms at a time. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- cfg.mk | 8 ++++++++ src/phyp/phyp_driver.c | 20 +++++++------------- src/util/virnetlink.c | 16 +++++++--------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/cfg.mk b/cfg.mk index 0bf5bfc..0e809fc 100644 --- a/cfg.mk +++ b/cfg.mk @@ -444,6 +444,14 @@ sc_prohibit_nonreentrant: done ; \ exit $$fail +sc_prohibit_select: + @fail=0 ; \ + (prohibit="\\<select *\\(" \ + halt="use poll(), not select()" \ + $(_sc_search_regexp) \ + ) || fail=1; \ + exit $$fail + # Prohibit the inclusion of <ctype.h>. sc_prohibit_ctype_h: @prohibit='^# *include *<ctype\.h>' \ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index f0007c0..4594cbf 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -40,6 +40,7 @@ #include <netdb.h> #include <fcntl.h> #include <domain_event.h> +#include <poll.h> #include "internal.h" #include "virauth.h" @@ -72,29 +73,22 @@ static unsigned const int PHYP_MAC_SIZE= 12; static int waitsocket(int socket_fd, LIBSSH2_SESSION * session) { - struct timeval timeout; - fd_set fd; - fd_set *writefd = NULL; - fd_set *readfd = NULL; + struct pollfd fds[1]; int dir; - timeout.tv_sec = 0; - timeout.tv_usec = 1000; - - FD_ZERO(&fd); - - FD_SET(socket_fd, &fd); + memset(fds, 0, sizeof(fds)); + fds[0].fd = socket_fd; /* now make sure we wait in the correct direction */ dir = libssh2_session_block_directions(session); if (dir & LIBSSH2_SESSION_BLOCK_INBOUND) - readfd = &fd; + fds[0].events |= POLLIN; if (dir & LIBSSH2_SESSION_BLOCK_OUTBOUND) - writefd = &fd; + fds[0].events |= POLLOUT; - return select(socket_fd + 1, readfd, writefd, NULL, &timeout); + return poll(fds, ARRAY_CARDINALITY(fds), -1); } /* this function is the layer that manipulates the ssh channel itself diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index d340eda..f5865a0 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -48,7 +48,7 @@ #define VIR_FROM_THIS VIR_FROM_NET -#define NETLINK_ACK_TIMEOUT_S 2 +#define NETLINK_ACK_TIMEOUT_S 2*1000 #if defined(__linux__) && defined(HAVE_LIBNL) /* State for a single netlink event handle */ @@ -185,10 +185,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, .nl_groups = 0, }; ssize_t nbytes; - struct timeval tv = { - .tv_sec = NETLINK_ACK_TIMEOUT_S, - }; - fd_set readfds; + struct pollfd fds[1]; int fd; int n; struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); @@ -242,14 +239,15 @@ int virNetlinkCommand(struct nl_msg *nl_msg, goto error; } - FD_ZERO(&readfds); - FD_SET(fd, &readfds); + memset(fds, 0, sizeof(fds)); + fds[0].fd = fd; + fds[0].events = POLLIN; - n = select(fd + 1, &readfds, NULL, NULL, &tv); + n = poll(fds, ARRAY_CARDINALITY(fds), NETLINK_ACK_TIMEOUT_S); if (n <= 0) { if (n < 0) virReportSystemError(errno, "%s", - _("error in select call")); + _("error in poll call")); if (n == 0) virReportSystemError(ETIMEDOUT, "%s", _("no valid netlink response was received")); -- 1.8.2.1

2 1

[libvirt] [PATCH] portability: fix virNetDevSetMAC and virNetDevExists on BSD
by Roman Bogorodskiy 14 May '13

14 May '13

- provide virNetDevSetMAC() implementation based on SIOCSIFLLADDR ioctl. - adjust virNetDevExists() to check for ENXIO error because FreeBSD throws it when device doesn't exist --- configure.ac | 8 ++++++++ src/util/virnetdev.c | 41 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 992a778..95d303f 100644 --- a/configure.ac +++ b/configure.ac @@ -2372,6 +2372,14 @@ AC_CHECK_MEMBERS([struct ifreq.ifr_newname, #include <net/if.h> ]) +AC_CHECK_DECLS([link_addr], + [], [], + [#include <sys/types.h> + #include <sys/socket.h> + #include <net/if_dl.h> + ]) + + # Only COPYING.LIB is under version control, yet COPYING # is included as part of the distribution tarball. # Copy one to the other, but only if this is a srcdir-build. diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index d987b8e..0527a27 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -47,6 +47,11 @@ # undef HAVE_STRUCT_IFREQ #endif +#ifdef HAVE_DECL_LINK_ADDR +# include <sys/sockio.h> +# include <net/if_dl.h> +#endif + #define VIR_FROM_THIS VIR_FROM_NONE #if defined(HAVE_STRUCT_IFREQ) @@ -110,7 +115,7 @@ int virNetDevExists(const char *ifname) return -1; if (ioctl(fd, SIOCGIFFLAGS, &ifr)) { - if (errno == ENODEV) + if (errno == ENODEV || errno == ENXIO) ret = 0; else virReportSystemError(errno, @@ -179,6 +184,40 @@ cleanup: VIR_FORCE_CLOSE(fd); return ret; } +#elif defined(SIOCSIFLLADDR) && defined(HAVE_STRUCT_IFREQ) && \ + defined(HAVE_DECL_LINK_ADDR) +int virNetDevSetMAC(const char *ifname, + const virMacAddrPtr macaddr) +{ + struct ifreq ifr; + struct sockaddr_dl sdl; + char mac[VIR_MAC_STRING_BUFLEN + 1] = ":"; + int s; + int ret = -1; + + if ((s = virNetDevSetupControl(ifname, &ifr)) < 0) + return -1; + + virMacAddrFormat(macaddr, mac + 1); + sdl.sdl_len = sizeof(sdl); + link_addr(mac, &sdl); + + memcpy(ifr.ifr_addr.sa_data, sdl.sdl_data, VIR_MAC_BUFLEN); + ifr.ifr_addr.sa_len = VIR_MAC_BUFLEN; + + if (ioctl(s, SIOCSIFLLADDR, &ifr) < 0) { + virReportSystemError(errno, + _("Cannot set interface MAC on '%s'"), + ifname); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FORCE_CLOSE(s); + + return ret; +} #else int virNetDevSetMAC(const char *ifname, const virMacAddrPtr macaddr ATTRIBUTE_UNUSED) -- 1.8.0

2 3

[libvirt] [PATCH] Fix starting domains when kernel has no cgroups support
by Jim Fehlig 14 May '13

14 May '13

Found that I was unable to start existing domains after updating to a kernel with no cgroups support # zgrep CGROUP /proc/config.gz # CONFIG_CGROUPS is not set # virsh start test error: Failed to start domain test error: Unable to initialize /machine cgroup: Cannot allocate memory virCgroupPartitionNeedsEscaping() correctly returns errno (ENOENT) when attempting to open /proc/cgroups on such a system, but it was being dropped in virCgroupSetPartitionSuffix(). Change virCgroupSetPartitionSuffix() to propogate errors returned by its callees. Also check for ENOENT in qemuInitCgroup() when determining if cgroups support is available. --- src/qemu/qemu_cgroup.c | 3 ++- src/util/vircgroup.c | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9c45b76..40777aa 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -414,7 +414,8 @@ int qemuInitCgroup(virQEMUDriverPtr driver, if (rc != 0) { if (rc == -ENXIO || rc == -EPERM || - rc == -EACCES) { /* No cgroups mounts == success */ + rc == -EACCES || + rc == -ENOENT) { /* No cgroups mounts == success */ VIR_DEBUG("No cgroups present/configured/accessible, ignoring error"); goto done; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 473d2fc..ef619dc 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1167,14 +1167,14 @@ static int virCgroupPartitionEscape(char **path) return 0; } -static char *virCgroupSetPartitionSuffix(const char *path) +static int virCgroupSetPartitionSuffix(const char *path, char **res) { char **tokens = virStringSplit(path, "/", 0); size_t i; - char *ret = NULL; + int ret = -1; if (!tokens) - return NULL; + return ret; for (i = 0 ; tokens[i] != NULL ; i++) { /* Whitelist the 3 top level fixed dirs @@ -1193,20 +1193,27 @@ static char *virCgroupSetPartitionSuffix(const char *path) !strchr(tokens[i], '.')) { if (VIR_REALLOC_N(tokens[i], strlen(tokens[i]) + strlen(".partition") + 1) < 0) { + ret = -ENOMEM; virReportOOMError(); goto cleanup; } strcat(tokens[i], ".partition"); } - if (virCgroupPartitionEscape(&(tokens[i])) < 0) { - virReportOOMError(); + ret = virCgroupPartitionEscape(&(tokens[i])); + if (ret < 0) { + if (ret == -ENOMEM) + virReportOOMError(); goto cleanup; } } - if (!(ret = virStringJoin((const char **)tokens, "/"))) + if (!(*res = virStringJoin((const char **)tokens, "/"))) { + ret = -ENOMEM; goto cleanup; + } + + ret = 0; cleanup: virStringFreeList(tokens); -- 1.8.0.1

2 4

[libvirt] [PATCH] .gitignore: add fchosttest
by Ján Tomko 14 May '13

14 May '13

--- Pushed under the trivial rule. .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f181f89..3d874a9 100644 --- a/.gitignore +++ b/.gitignore @@ -144,6 +144,7 @@ /tests/domainsnapshotxml2xmltest /tests/esxutilstest /tests/eventtest +/tests/fchosttest /tests/fdstreamtest /tests/hashtest /tests/jsontest -- 1.8.1.5

1 0

[libvirt] RFC version information in API docs
by Claudio Bley 14 May '13

14 May '13

Hi. Sometimes, it's a bit hard to determine when exactly a function, flag or macro appeared in libvirt, ie. whether it will be supported on my target machine having a specific version of libvirt or not. So, I have created an enriched version of the API docs, using a XSL stylesheet enumerating the libvirt?-api.xml files of all libvirt releases. For an example, you can have a look here: http://avdv.github.io/libvirt/html/libvirt-libvirt.html#virVcpuState Hovering over an enum value displays version information in a tooltip. What do you think? Should this information be included by default in the API docs? If so, I could use a little help on how to integrate the generation of this information into the automake process. Basically, the XSL stylesheet enriches the generated libvirt-api.xml with the current version given as a parameter to xsltproc plus version information gathered from a previous enriched libvirt-api.xml file. That way, every symbol/node() gets a since="..." version attribute. So, during a build there's an extra step necessary when generating the libvirt-api.xml file: 1. apibuild.py => libvirt-api.1.xml 2. (libvirt-api.1.xml + libvirt-api.0.xml) => XSLT => libvirt-api.xml When releasing, libvirt-api.xml should be moved to libvirt-api.0.xml. Alternatively, there could be a second XSL file only extracting the version information without the boilerplate. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

3 6

[libvirt] [PATCH v2 0/2] Support for VNC WebSocket
by Martin Kletzander 13 May '13

13 May '13

This series, which is meant to be applied _after_ 1.0.5 release, is adding support for the VNC WebSocket to be configured for QEMU. Documentation from qemu_options.hx: qemu -vnc ...,websocket[=<port>] Opens an additional TCP listening port dedicated to VNC Websocket connections. By definition the Websocket port is 5700+@var{display}. If @var{host} is specified connections will only be allowed from this host. As an alternative the Websocket port could be specified by using @code{websocket}=@var{port}. v2: - Incorporated recommendations from RFC [1] [1] http://www.redhat.com/archives/libvir-list/2013-April/msg02056.html Martin Kletzander (2): Add VNC WebSocket support qemu: Add VNC WebSocket support docs/formatdomain.html.in | 5 ++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 16 ++++++++++ src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 2 ++ src/qemu/qemu.conf | 7 +++++ src/qemu/qemu_capabilities.c | 11 +++++-- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 60 ++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_command.h | 5 +++- src/qemu/qemu_conf.c | 32 ++++++++++++++++++++ src/qemu/qemu_conf.h | 6 ++++ src/qemu/qemu_driver.c | 5 ++++ src/qemu/qemu_process.c | 31 ++++++++++++++++---- src/qemu/test_libvirtd_qemu.aug.in | 2 ++ tests/qemuargv2xmltest.c | 1 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 18 files changed, 180 insertions(+), 12 deletions(-) -- 1.8.2.1

3 10

[libvirt] qemu command line quoting (was: Re: [Libguestfs] [PATCH 0/5] rbd improvements)
by Richard W.M. Jones 13 May '13

13 May '13

On Sun, May 12, 2013 at 02:42:36PM -0400, Mike Kelly wrote: > On Thu, May 9, 2013 at 12:21 PM, Richard W.M. Jones <rjones(a)redhat.com> wrote: > > On Thu, May 09, 2013 at 11:23:55AM -0400, Mike Kelly wrote: > >> On Wed, May 8, 2013 at 6:53 AM, Richard W.M. Jones <rjones(a)redhat.com> wrote: > >> > One worry I have is whether quoting is required for the server > >> > name(s), export name, username and secret. > >> > >> Well. I think the main things we had to quote were ':' and ';', but > >> none of those are valid in a hostname. Username also probably doesn't > >> contain anything special, and secret is a base64-encoded string. I > >> confirmed that even with the string ending in '==', it was parsed just > >> fine by qemu, at least in my limited manual testing. > >> > >> If you can suggest a way to be more robust this, though, then I can > >> try to work that into a future patch series. > > > > The quoting problem happens when someone writes a program which takes > > (eg) a hostname string from the user and passes it unmodified to the > > guestfs API. It's an issue if this string can cause unexpected [even > > malicious/exploitable] things to happen when passed unquoted on the > > qemu command line. > > Well, I'm not sure if this way of setting things up is still > encouraged, but at least this documentation suggests basically using > the fact that libvirt won't quote the image name as a "feature": > > http://ceph.com/w/index.php?title=QEMU-RBD#Caching > > <disk type='network' device='disk'> > <source protocol='rbd' > name='poolname/imagename:rbd_cache=1:rbd_cache_size=67108864:rbd_cache_max_dirty=0'/> Hmmm ... This is a bug in libvirt, but also a missing feature of libvirt since it cannot express these other configuration fields. > <driver name='qemu' type='rbd'/> > <target dev='vda' bus='virtio'/> > </disk> > > The more official documentation for configuring caching doesn't seem > to make any specific mention of this: > > http://ceph.com/docs/master/rbd/qemu-rbd/#qemu-cache-options > http://ceph.com/docs/master/rbd/libvirt/ > http://ceph.com/docs/master/rbd/rbd-config-ref/ By the way, I fixed some qemu-img command line quoting issues yesterday (but not in ceph): https://github.com/libguestfs/libguestfs/commit/914d3e68ec272436f91080f47dd… Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#)

2 1