- Devel - libvirt List Archives

[libvirt] [PATCH] command: avoid double close bugs
by Eric Blake 31 May '12

31 May '12

KAMEZAWA Hiroyuki reported a nasty double-free bug when virCommand is used to convert a string into input to a child command. The problem is that the poll() loop of virCommandProcessIO would close() the write end of the pipe in order to let the child see EOF, then the caller virCommandRun() would also close the same fd number, with the second close possibly nuking an fd opened by some other thread in the meantime. This in turn can have all sorts of bad effects. This is based on his first attempt at a patch, at https://bugzilla.redhat.com/show_bug.cgi?id=823716 * src/util/command.c (_virCommand): Drop inpipe member. (virCommandProcessIO): Add argument, to avoid closing caller's fd without informing caller. (virCommandRun, virCommandNewArgs): Adjust clients. --- src/util/command.c | 18 ++++++------------ 1 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index eaa9f16..bf219e4 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -83,7 +83,6 @@ struct _virCommand { char **errbuf; int infd; - int inpipe; int outfd; int errfd; int *outfdptr; @@ -788,7 +787,6 @@ virCommandNewArgs(const char *const*args) cmd->handshakeNotify[1] = -1; cmd->infd = cmd->outfd = cmd->errfd = -1; - cmd->inpipe = -1; cmd->pid = -1; virCommandAddArgSet(cmd, args); @@ -1676,7 +1674,7 @@ virCommandTranslateStatus(int status) * Manage input and output to the child process. */ static int -virCommandProcessIO(virCommandPtr cmd) +virCommandProcessIO(virCommandPtr cmd, int *inpipe) { int infd = -1, outfd = -1, errfd = -1; size_t inlen = 0, outlen = 0, errlen = 0; @@ -1687,7 +1685,7 @@ virCommandProcessIO(virCommandPtr cmd) * via pipe */ if (cmd->inbuf) { inlen = strlen(cmd->inbuf); - infd = cmd->inpipe; + infd = *inpipe; } /* With out/err buffer, the outfd/errfd have been filled with an @@ -1807,12 +1805,9 @@ virCommandProcessIO(virCommandPtr cmd) } } else { inoff += done; - if (inoff == inlen) { - int tmpfd ATTRIBUTE_UNUSED; - tmpfd = infd; - if (VIR_CLOSE(infd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); - } + if (inoff == inlen && VIR_CLOSE(*inpipe) < 0) + VIR_DEBUG("ignoring failed close on fd %d", infd); + infd = -1; } } } @@ -1938,7 +1933,6 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) return -1; } cmd->infd = infd[0]; - cmd->inpipe = infd[1]; } /* If caller requested the same string for stdout and stderr, then @@ -1981,7 +1975,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) } if (string_io) - ret = virCommandProcessIO(cmd); + ret = virCommandProcessIO(cmd, &infd[1]); if (virCommandWait(cmd, exitstatus) < 0) ret = -1; -- 1.7.7.6

2 11

[libvirt] error: this function is not supported by the connection driver : virConnectNumOfStoragePool
by Onkar 30 May '12

30 May '12

Hello, I configured libvirt like this ./configure --prefix=/ --exec-prefix=/usr/ --libdir=/usr/lib64/ --with-storage-dir --with-storage-dir --with-storage-dir --with-storage-dir --with-storage-dir --with-storage-dir --with-storage-dir --with-storage-dir --with-qemu --with-libvirtd --with-selinux --with-network --with-libblkid and then, make -j4 make install and then # virsh list --all Id Name State ---------------------------------------------------- - vm1 shut off # virsh pool-list --all error: Failed to list active pools error: this function is not supported by the connection driver: virConnectNumOfStoragePools getting this error , any clues on why is this error ? Regards, Onkar

1 0

[libvirt] [PATCH] build: include augeas-gentest.pl and libvirt_qemu_probes.h into dist file
by Wen Congyang 30 May '12

30 May '12

We generate *.aug from *.aug.in by augeas-gentest.pl, so this script should be included in dist file. We split QEMU dtrace probes into separate file, but we forget include libvirt_qemu_probes.h into dist file. --- src/Makefile.am | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index c8efa5b..c418897 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1120,6 +1120,7 @@ check-local: check-augeas check-augeas: check-augeas-qemu check-augeas-lxc check-augeas-sanlock AUG_GENTEST = $(top_srcdir)/build-aux/augeas-gentest.pl +EXTRA_DIST += $(AUG_GENTEST) if WITH_QEMU qemu/test_libvirtd_qemu.aug: qemu/test_libvirtd_qemu.aug.in qemu/qemu.conf $(AUG_GENTEST) @@ -1289,7 +1290,8 @@ if WITH_REMOTE $(REMOTE_DRIVER_GENERATED): libvirt_probes.h endif WITH_REMOTE -BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp +BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp \ + libvirt_qemu_probes.h if WITH_QEMU libvirt_driver_qemu_la_LIBADD += libvirt_qemu_probes.lo -- 1.7.1

2 3

[libvirt] [PATCH] build: silence warning from autoconf
by Eric Blake 30 May '12

30 May '12

Autoconf 2.60 and later insist on using ${datarootdir}, rather than the derived ${datadir} (although the latter defaults to the former, it is possible to set configure arguments so that they differ): config.status: creating libvirt.pc config.status: WARNING: 'libvirt.pc.in' seems to ignore the --datarootdir setting This patch follows the autoconf manual's suggestions for how to support 2.59 (RHEL 5) and newer simultaneously. * libvirt.pc.in (datarootdir): Define, so ${datadir} will not ignore datarootdir when using newer autoconf. --- https://www.gnu.org/software/autoconf/manual/autoconf.html#Changed-Director… libvirt.pc.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/libvirt.pc.in b/libvirt.pc.in index 3d999a8..616575d 100644 --- a/libvirt.pc.in +++ b/libvirt.pc.in @@ -2,6 +2,7 @@ prefix=@prefix@ exec_prefix=@exec_prefix@ libdir=@libdir@ includedir=@includedir@ +datarootdir=@datarootdir@ libvirt_api=@datadir@/libvirt/api/libvirt-api.xml libvirt_qemu_api=@datadir@/libvirt/api/libvirt-qemu-api.xml -- 1.7.7.6

2 9

[libvirt] [snmp PATCH 0/2] Cleanup for libvirt-snmp
by Martin Kletzander 30 May '12

30 May '12

This series cleans up the libvirt-snmp codebase with some visual (whitespaces) and semantic (memory leaks) cleanups. Martin Kletzander (2): cleanup: whitespaces removed from ends of lines Stop event thread on server stop autogen.sh | 4 +- docs/libvirtMib_subagent.pod | 4 +- src/LIBVIRT-MIB.txt | 4 +- src/libvirtGuestTable-README-FIRST.txt | 6 +- src/libvirtGuestTable-README-libvirtGuestTable.txt | 16 ++-- src/libvirtGuestTable.c | 16 ++-- src/libvirtGuestTable.h | 22 +++--- src/libvirtGuestTable_data_access.c | 8 +- src/libvirtGuestTable_data_get.c | 8 +- src/libvirtGuestTable_data_set.c | 22 +++--- src/libvirtGuestTable_enums.h | 24 +++--- src/libvirtGuestTable_interface.c | 82 ++++++++++---------- src/libvirtGuestTable_interface.h | 2 +- src/libvirtGuestTable_oids.h | 20 +++--- src/libvirtMib_subagent.c | 22 +++--- src/libvirtSnmp.c | 20 +++++- 16 files changed, 150 insertions(+), 130 deletions(-) -- 1.7.8.6

2 5

[libvirt] [PATCH] file: Log closing filedescriptors
by Jiri Denemark 30 May '12

30 May '12

--- src/util/virfile.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/util/virfile.c b/src/util/virfile.c index db3d737..bc7f2c9 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -35,6 +35,7 @@ #include "configmake.h" #include "memory.h" #include "virterror_internal.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_NONE #define virFileError(code, ...) \ @@ -51,6 +52,15 @@ int virFileClose(int *fdptr, bool preserve_errno) if (preserve_errno) saved_errno = errno; rc = close(*fdptr); + if (rc < 0) { + if (errno != EBADF) { + char ebuf[1024] ATTRIBUTE_UNUSED; + VIR_DEBUG("Failed to close fd %d: %d", + *fdptr, virStrerror(errno, ebuf, sizeof(ebuf))); + } + } else { + VIR_DEBUG("Closed fd %d", *fdptr); + } *fdptr = -1; if (preserve_errno) errno = saved_errno; -- 1.7.10.2

2 3

[libvirt] [PATCH] command: Fix debug message during handshake
by Jiri Denemark 30 May '12

30 May '12

Probably a result of copy&paste... --- src/util/command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/command.c b/src/util/command.c index 5b94f1e..8b581cf 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -2538,7 +2538,7 @@ int virCommandHandshakeNotify(virCommandPtr cmd) return -1; } - VIR_DEBUG("Notify handshake on %d", cmd->handshakeWait[0]); + VIR_DEBUG("Notify handshake on %d", cmd->handshakeNotify[1]); if (safewrite(cmd->handshakeNotify[1], &c, sizeof(c)) != sizeof(c)) { virReportSystemError(errno, "%s", _("Unable to notify child process")); VIR_FORCE_CLOSE(cmd->handshakeNotify[1]); -- 1.7.10.2

2 1

[libvirt] [PATCH] build: Fixed generating of libvirt_qemu_probes.h
by Martin Kletzander 30 May '12

30 May '12

I added libvirt_qemu_probes.h into BUILT_SOURCES. That makes it generated, but most probably it is not the clearest way how to do that, but it fixes the build. --- Feel free to clean this, but for the time being, I'm pushing this as a build breaker. src/Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index c8efa5b..1fde936 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1294,6 +1294,7 @@ BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp if WITH_QEMU libvirt_driver_qemu_la_LIBADD += libvirt_qemu_probes.lo nodist_libvirt_driver_qemu_la_SOURCES = libvirt_qemu_probes.h +BUILT_SOURCES += libvirt_qemu_probes.h endif tapsetdir = $(datadir)/systemtap/tapset -- 1.7.8.6

2 3

[libvirt] [PATCH] virCommand: Extend debug message for handshake
by Michal Privoznik 30 May '12

30 May '12

Currently, we are logging only one side of pipes we create in virCommandRequireHandshake(); This is enough in cases where pipe2() returns two consecutive FDs. However, it is not guaranteed and it may return any FDs. Therefore, it's wise to log the other ends as well. --- src/util/command.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index eaa9f16..5b94f1e 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -2443,8 +2443,10 @@ void virCommandRequireHandshake(virCommandPtr cmd) return; } - VIR_DEBUG("Transfer handshake wait=%d notify=%d", - cmd->handshakeWait[1], cmd->handshakeNotify[0]); + VIR_DEBUG("Transfer handshake wait=%d notify=%d, " + "keep handshake wait=%d notify=%d", + cmd->handshakeWait[1], cmd->handshakeNotify[0], + cmd->handshakeWait[0], cmd->handshakeNotify[1]); virCommandTransferFD(cmd, cmd->handshakeWait[1]); virCommandTransferFD(cmd, cmd->handshakeNotify[0]); cmd->handshake = true; -- 1.7.8.5

2 2

[libvirt] [PATCH libvirt-java] Return a byte[] array with secretGetValue
by Wido den Hollander 30 May '12

30 May '12

Signed-off-by: Wido den Hollander <wido(a)widodh.nl> --- src/main/java/org/libvirt/Secret.java | 11 +++++++++-- src/main/java/org/libvirt/jna/Libvirt.java | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/libvirt/Secret.java b/src/main/java/org/libvirt/Secret.java index e536cf4..a874925 100644 --- a/src/main/java/org/libvirt/Secret.java +++ b/src/main/java/org/libvirt/Secret.java @@ -5,6 +5,9 @@ import org.libvirt.jna.SecretPointer; import com.sun.jna.Native; import com.sun.jna.NativeLong; +import com.sun.jna.ptr.LongByReference; +import com.sun.jna.Pointer; +import java.nio.ByteBuffer; /** * A secret defined by libvirt @@ -109,9 +112,13 @@ public class Secret { * * @return the value of the secret, or null on failure. */ - public String getValue() throws LibvirtException { - String returnValue = libvirt.virSecretGetValue(VSP, new NativeLong(), 0); + public byte[] getValue() throws LibvirtException { + LongByReference value_size = new LongByReference(); + Pointer value = libvirt.virSecretGetValue(VSP, value_size, 0); processError(); + ByteBuffer bb = value.getByteBuffer(0, value_size.getValue()); + byte[] returnValue = new byte[bb.remaining()]; + bb.get(returnValue); return returnValue; } diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index 2c8c03d..3804d55 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -330,7 +330,7 @@ public interface Libvirt extends Library { public int virSecretGetUUID(SecretPointer virSecretPtr, byte[] uuidString); public int virSecretGetUUIDString(SecretPointer virSecretPtr, byte[] uuidString); public String virSecretGetUsageID(SecretPointer virSecretPtr); - public String virSecretGetValue(SecretPointer virSecretPtr, NativeLong value_size, int flags); + public Pointer virSecretGetValue(SecretPointer virSecretPtr, LongByReference value_size, int flags); public String virSecretGetXMLDesc(SecretPointer virSecretPtr, int flags); public SecretPointer virSecretLookupByUsage(ConnectionPointer virConnectPtr, int usageType, String usageID); public SecretPointer virSecretLookupByUUID(ConnectionPointer virConnectPtr, byte[] uuidBytes); -- 1.7.5.4

2 2