- Devel - libvirt List Archives

[libvirt] [PATCH 1/3] Autodetect if the remote nc command supports the -q option
by Guido Günther 13 Oct '11

13 Oct '11

Based on a patch by Marc Deslauriers <marc.deslauriers(a)ubuntu.com> RH: https://bugzilla.redhat.com/show_bug.cgi?id=562176 Ubuntu: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/517478 Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=573172 --- src/rpc/virnetsocket.c | 23 ++++++++++++++++++++--- tests/virnetsockettest.c | 11 ++++++----- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index e4289b1..ea653da 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -634,9 +634,26 @@ int virNetSocketNewConnectSSH(const char *nodename, "-e", "none", NULL); if (noVerify) virCommandAddArgList(cmd, "-o", "StrictHostKeyChecking=no", NULL); - virCommandAddArgList(cmd, nodename, - netcat ? netcat : "nc", - "-U", path, NULL); + + virCommandAddArgList(cmd, nodename, "sh", "-c", NULL); + /* + * This ugly thing is a shell script to detect availability of + * the -q option for 'nc': debian and suse based distros need this + * flag to ensure the remote nc will exit on EOF, so it will go away + * when we close the connection tunnel. If it doesn't go away, subsequent + * connection attempts will hang. + * + * Fedora's 'nc' doesn't have this option, and defaults to the desired + * behavior. + */ + virCommandAddArgFormat(cmd, + "'if %s -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then" + " ARG=-q0;" + "fi;" + "%s $ARG -U %s'", + netcat ? netcat : "nc", + netcat ? netcat : "nc", + path); return virNetSocketNewConnectCommand(cmd, retsock); } diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index fae15a3..b3a2705 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -496,7 +496,7 @@ mymain(void) struct testSSHData sshData1 = { .nodename = "somehost", .path = "/tmp/socket", - .expectOut = "somehost nc -U /tmp/socket\n", + .expectOut = "somehost sh -c 'if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then ARG=-q0;fi;nc $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 1", 1, testSocketSSH, &sshData1) < 0) ret = -1; @@ -509,7 +509,7 @@ mymain(void) .noTTY = true, .noVerify = false, .path = "/tmp/socket", - .expectOut = "-p 9000 -l fred -T -o BatchMode=yes -e none somehost netcat -U /tmp/socket\n", + .expectOut = "-p 9000 -l fred -T -o BatchMode=yes -e none somehost sh -c 'if netcat -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then ARG=-q0;fi;netcat $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 2", 1, testSocketSSH, &sshData2) < 0) ret = -1; @@ -522,7 +522,7 @@ mymain(void) .noTTY = false, .noVerify = true, .path = "/tmp/socket", - .expectOut = "-p 9000 -l fred -o StrictHostKeyChecking=no somehost netcat -U /tmp/socket\n", + .expectOut = "-p 9000 -l fred -o StrictHostKeyChecking=no somehost sh -c 'if netcat -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then ARG=-q0;fi;netcat $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 3", 1, testSocketSSH, &sshData3) < 0) ret = -1; @@ -538,7 +538,8 @@ mymain(void) struct testSSHData sshData5 = { .nodename = "crashyhost", .path = "/tmp/socket", - .expectOut = "crashyhost nc -U /tmp/socket\n", + .expectOut = "crashyhost sh -c 'if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then ARG=-q0;fi;nc $ARG -U /tmp/socket'\n", + .dieEarly = true, }; if (virtTestRun("SSH test 5", 1, testSocketSSH, &sshData5) < 0) @@ -549,7 +550,7 @@ mymain(void) .path = "/tmp/socket", .keyfile = "/root/.ssh/example_key", .noVerify = true, - .expectOut = "-i /root/.ssh/example_key -o StrictHostKeyChecking=no example.com nc -U /tmp/socket\n", + .expectOut = "-i /root/.ssh/example_key -o StrictHostKeyChecking=no example.com sh -c 'if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then ARG=-q0;fi;nc $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 6", 1, testSocketSSH, &sshData6) < 0) ret = -1; -- 1.7.6.3

2 3

[libvirt] [PATCH] storage: plug memory leak on error
by Eric Blake 13 Oct '11

13 Oct '11

Detected by Coverity. Present since commit 82c1740. * src/storage/storage_backend_logical.c (virStorageBackendLogicalMakeVol): Fix leak. --- Pushing under the trivial rule. src/storage/storage_backend_logical.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 51624a7..50d0407 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -231,16 +231,17 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, if (!(offset_str = strndup(p + vars[j + 1].rm_so, len))) { virReportOOMError(); goto cleanup; } if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent offset value")); + VIR_FREE(offset_str); goto cleanup; } VIR_FREE(offset_str); vol->source.extents[vol->source.nextent].start = offset * size; vol->source.extents[vol->source.nextent].end = (offset * size) + length; vol->source.nextent++; -- 1.7.4.4

1 0

[libvirt] How to identify a VirtFS using libvirt
by Richard Maciel 13 Oct '11

13 Oct '11

Hi, Is there any way to fetch info on a VirtFS share, aside from reading the guest's XML file? For instance, I'd like to read the source and target dir of the VirtFS mount created on guest by using the following XML code: <filesystem type='mount' accessmode='passthrough'> <source dir='/mnt/9p_setup'/> <target dir='9p_test'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </filesystem> I tried to use the pool and volume-related functions on the API, but that didn't work. Richard Maciel

1 0

[libvirt] [PATCH] qemu: fix text block info parsing
by Eric Blake 13 Oct '11

13 Oct '11

Detected by Coverity. p (the pointer to the string) is always true; when in reality, we wanted to know whether the integer value of the just-parsed string is '0' or '1'. Logic bug since commit b1b5b51. * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBlockInfo): Set results to proper value. --- Second bug introduced by the same commit. And I think I reviewed it originally. Sorry for not catching it earlier. src/qemu/qemu_monitor_text.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 1eb9846..49c469b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -817,19 +817,19 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) VIR_DEBUG("error reading removable: %s", p); else - info->removable = p != NULL; + info->removable = tmp != 0; } else if (STRPREFIX(p, "locked=")) { p += strlen("locked="); if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) VIR_DEBUG("error reading locked: %s", p); else - info->locked = p ? true : false; + info->locked = tmp != 0; } else if (STRPREFIX(p, "tray_open=")) { p += strlen("tray_open="); if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) VIR_DEBUG("error reading tray_open: %s", p); else - info->tray_open = p ? true : false; + info->tray_open = tmp != 0; } else { /* ignore because we don't parse all options */ } -- 1.7.4.4

2 2

[libvirt] [PATCH] qemu: avoid text monitor null deref
by Eric Blake 13 Oct '11

13 Oct '11

Detected by Coverity. If, for some reason, our text monitor input does not match our assumptions, we end up incrementing p while it is NULL, then dereferencing the pointer 0x1, which will fault. * src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBlockStatsParamsNumber): Rewrite to avoid deref of strchr failure. Fix indentation. --- src/qemu/qemu_monitor_text.c | 33 +++++++++++++++------------------ 1 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 51e8c5c..1eb9846 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1036,26 +1036,23 @@ int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, * "floppy0: ") */ p = strchr(p, ' '); - p++; - while (*p) { - if (STRPREFIX (p, "rd_bytes=") || - STRPREFIX (p, "wr_bytes=") || - STRPREFIX (p, "rd_operations=") || - STRPREFIX (p, "wr_operations=") || - STRPREFIX (p, "rd_total_times_ns=") || - STRPREFIX (p, "wr_total_times_ns=") || - STRPREFIX (p, "flush_operations=") || - STRPREFIX (p, "flush_total_times_ns=")) { - num++; - } else { - VIR_DEBUG ("unknown block stat near %s", p); - } + while (p && p < eol) { + if (STRPREFIX (p, " rd_bytes=") || + STRPREFIX (p, " wr_bytes=") || + STRPREFIX (p, " rd_operations=") || + STRPREFIX (p, " wr_operations=") || + STRPREFIX (p, " rd_total_times_ns=") || + STRPREFIX (p, " wr_total_times_ns=") || + STRPREFIX (p, " flush_operations=") || + STRPREFIX (p, " flush_total_times_ns=")) { + num++; + } else { + VIR_DEBUG ("unknown block stat near %s", p); + } - /* Skip to next label. */ - p = strchr (p, ' '); - if (!p || p >= eol) break; - p++; + /* Skip to next label. */ + p = strchr(p + 1, ' '); } *nparams = num; -- 1.7.4.4

2 2

[libvirt] [PATCH] qemu: check for json allocation failure
by Eric Blake 13 Oct '11

13 Oct '11

Detected by Coverity. Introduced in commit b1b5b51. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetBlockInfo): Avoid null dereference on OOM. --- src/qemu/qemu_monitor_json.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 18d1c9f..cd8f1e5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1364,6 +1364,9 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr devices; + if (!cmd) + return -1; + ret = qemuMonitorJSONCommand(mon, cmd, &reply); if (ret == 0) ret = qemuMonitorJSONCheckError(cmd, reply); -- 1.7.4.4

2 2

[libvirt] [PATCH] virFDStream: close also given errfd (fd leak)
by Marc-André Lureau 13 Oct '11

13 Oct '11

In virFDStreamOpenFileInternal(), a errfd pipe is opened by virCommandRunAsync() and given to virFDStreamOpenInternal(). It seems virFDStream should close errfd, just like the other fd it is given. This fixes screenshots leaking FDs: http://bugzilla.redhat.com/show_bug.cgi?id=745761 --- src/fdstream.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index b60162c..841f979 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -268,6 +268,10 @@ virFDStreamClose(virStreamPtr st) virCommandFree(fdst->cmd); fdst->cmd = NULL; } + + if (VIR_CLOSE(fdst->errfd) < 0) + VIR_DEBUG("ignoring failed close on fd %d", fdst->errfd); + st->privateData = NULL; if (fdst->dispatching) { -- 1.7.6.2

2 1

[libvirt] [PATCH RESEND] command: avoid fd leak on failure
by Eric Blake 13 Oct '11

13 Oct '11

virCommandTransferFD promises that the fd is no longer owned by the caller. Normally, we want the fd to remain open until the child runs, but in error situations, we must close it earlier. * src/util/command.c (virCommandTransferFD): Close fd now if we can't track it to close later. (virCommandKeepFD): Adjust helper to make this easier. --- This leak can only happen on OOM or other extreme error conditions, but ought to be plugged. When I originally posted this: https://www.redhat.com/archives/libvir-list/2011-July/msg00674.html DV was worried that callers might abuse things and use fd even after this function closed it; but I proved to myself in writing a (non-working) v2 that all callers were already safe, and that this v1 was indeed a smaller solution. src/util/command.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 699ba2d..c3ce361 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -730,23 +730,26 @@ virCommandNewArgList(const char *binary, ...) * closing it. FD must not be one of the three standard streams. If * transfer is true, then fd will be closed in the parent after a call * to Run/RunAsync/Free, otherwise caller is still responsible for fd. + * Returns true if a transferring caller should close FD now, and + * false if the transfer is successfully recorded. */ -static void +static bool virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer) { if (!cmd) - return; + return fd > STDERR_FILENO; if (fd <= STDERR_FILENO || FD_SETSIZE <= fd) { if (!cmd->has_error) cmd->has_error = -1; VIR_DEBUG("cannot preserve %d", fd); - return; + return fd > STDERR_FILENO; } FD_SET(fd, &cmd->preserve); if (transfer) FD_SET(fd, &cmd->transfer); + return false; } /** @@ -761,7 +764,7 @@ virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer) void virCommandPreserveFD(virCommandPtr cmd, int fd) { - return virCommandKeepFD(cmd, fd, false); + virCommandKeepFD(cmd, fd, false); } /** @@ -771,12 +774,14 @@ virCommandPreserveFD(virCommandPtr cmd, int fd) * * Transfer the specified file descriptor * to the child, instead of closing it on exec. - * Close the fd in the parent during Run/RunAsync/Free. + * The parent should no longer use fd, and the parent's copy will + * be automatically closed no later than during Run/RunAsync/Free. */ void virCommandTransferFD(virCommandPtr cmd, int fd) { - return virCommandKeepFD(cmd, fd, true); + if (virCommandKeepFD(cmd, fd, true)) + VIR_FORCE_CLOSE(fd); } -- 1.7.4.4

2 2

[libvirt] ALERT Virtualization Automatic Builder
by berrange＠redhat.com 13 Oct '11

13 Oct '11

Overall status: failed Start date: Thu Oct 13 2011 Start time: 11:05:02 UTC / 07:05:02 EDT Build counter: 1318503902 Build timestamp: 1318503902 URL: http://builder.virt-tools.org/index.html Module: libvirt Status: failed URL: http://builder.virt-tools.org/module-libvirt.html

3 2

[libvirt] [PATCH RESEND] spec: mingw cleanups
by Eric Blake 13 Oct '11

13 Oct '11

* libvirt.spec.in (%configure): Drop unused %{one} macro. * mingw32-libvirt.spec.in (%{rhel}): Compile ESX but not HyperV on mingw build for RHEL. (%build): Make configure honor spec conditionals. Reorder to match libvirt.spec. * autobuild.sh (mingw): Update list to match. Suggested by Daniel P. Berrange. --- No comment when buried in a thread, so I'll resend: https://www.redhat.com/archives/libvir-list/2011-September/msg01072.html autobuild.sh | 16 +++++++----- libvirt.spec.in | 3 +- mingw32-libvirt.spec.in | 58 ++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 491f1b8..9853287 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -71,21 +71,23 @@ if [ -x /usr/bin/i686-pc-mingw32-gcc ]; then --host=i686-pc-mingw32 \ --prefix="$AUTOBUILD_INSTALL_ROOT/i686-pc-mingw32/sys-root/mingw" \ --enable-compile-warnings=error \ - --without-sasl \ - --without-avahi \ - --without-polkit \ - --without-python \ --without-xen \ --without-qemu \ + --without-openvz \ --without-lxc \ - --without-uml \ --without-vbox \ - --without-openvz \ + --without-xenapi \ + --without-uml \ + --without-sasl \ + --without-avahi \ + --without-polkit \ + --without-python \ + --without-libvirtd \ --without-phyp \ + --without-hyperv \ --without-netcf \ --without-audit \ --without-dtrace \ - --without-libvirtd make make install diff --git a/libvirt.spec.in b/libvirt.spec.in index 7c63710..03bd7c7 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -47,7 +47,7 @@ %define with_libxl 0%{!?_without_libxl:%{server_drivers}} %define with_vmware 0%{!?_without_vmware:%{server_drivers}} -# Then the hypervisor drivers that talk a native remote protocol +# Then the hypervisor drivers that talk via a native remote protocol %define with_phyp 0%{!?_without_phyp:1} %define with_esx 0%{!?_without_esx:1} %define with_hyperv 0%{!?_without_hyperv:1} @@ -711,7 +711,6 @@ of recent versions of Linux (and other OSes). %{?_without_python} \ %{?_without_libvirtd} \ %{?_without_uml} \ - %{?_without_one} \ %{?_without_phyp} \ %{?_without_esx} \ %{?_without_hyperv} \ diff --git a/mingw32-libvirt.spec.in b/mingw32-libvirt.spec.in index f651d11..57c67ae 100644 --- a/mingw32-libvirt.spec.in +++ b/mingw32-libvirt.spec.in @@ -5,6 +5,21 @@ %define __find_provides %{_mingw32_findprovides} %define __debug_install_post %{_mingw32_debug_install_post} +# The mingw build is client only. Set up defaults for hypervisor drivers +# that talk via a native remote protocol, and for which prereq mingw +# libraries exist. +%define with_phyp 0%{!?_without_phyp:1} +%define with_esx 0%{!?_without_esx:1} +%define with_hyperv 0%{!?_without_hyperv:0} # missing libwsman +%define with_xenapi 0%{!?_without_xenapi:1} + +# RHEL ships ESX but not PowerHypervisor, HyperV, or libxenserver (xenapi) +%if 0%{?rhel} +%define with_phyp 0 +%define with_xenapi 0 +%define with_hyperv 0 +%endif + Name: mingw32-libvirt Version: @VERSION@ Release: 3%{?dist}%{?extra_release} @@ -29,6 +44,13 @@ BuildRequires: pkgconfig # Need native version for msgfmt BuildRequires: gettext +%if %{with_phyp} +BuildRequires: mingw32-libssh2 +%endif +%if %{with_esx} +BuildRequires: mingw32-curl +%endif + BuildArch: noarch %{?_mingw32_debug_package} @@ -42,23 +64,43 @@ MinGW Windows libvirt virtualization library. %build +%if ! %{with_phyp} +%define _without_phyp --without-phyp +%endif + +%if ! %{with_esx} +%define _without_esx --without-esx +%endif + +%if ! %{with_hyperv} +%define _without_hyperv --without-hyperv +%endif + +%if ! %{with_xenapi} +%define _without_xenapi --without-xenapi +%endif + # XXX enable SASL in future %{_mingw32_configure} \ + --without-xen \ + --without-qemu \ + --without-openvz \ + --without-lxc \ + --without-vbox \ + %{?_without_xenapi} \ --without-sasl \ --without-avahi \ --without-polkit \ --without-python \ - --without-xen \ - --without-qemu \ - --without-lxc \ + --without-libvirtd \ --without-uml \ - --without-vbox \ - --without-openvz \ - --without-phyp \ + %{?_without_phyp} \ + %{?_without_esx} \ + %{?_without_hyperv} \ + --without-vmware \ --without-netcf \ --without-audit \ - --without-dtrace \ - --without-libvirtd + --without-dtrace make -- 1.7.4.4

2 2