- Devel - libvirt List Archives

[libvirt] PATCH: Fix leak in storage driver
by Daniel P. Berrange 30 Jan '09

30 Jan '09

A recent change to keep the storage pools active upon shutdown, exposed a minor flaw in the code which free's a virStoragePoolObj instance. It never free's the associated volumes, since it presumed you'd never free a pool, which was still active. A bogus assumption, causing us to leak memory upon daemon shutdown, thus annoying valgrind. Daniel diff -r 3e95abd6df89 src/storage_conf.c --- a/src/storage_conf.c Fri Jan 30 11:01:10 2009 +0000 +++ b/src/storage_conf.c Fri Jan 30 11:01:29 2009 +0000 @@ -296,6 +296,8 @@ virStoragePoolObjFree(virStoragePoolObjP if (!obj) return; + virStoragePoolObjClearVols(obj); + virStoragePoolDefFree(obj->def); virStoragePoolDefFree(obj->newDef); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

3 3

[libvirt] PATCH: Fix leak in libvirtd daemon when reporting errors
by Daniel P. Berrange 30 Jan '09

30 Jan '09

When reporting libvirt errors back to the client, we forgot to free the memory associated with the remote_error object - principally the char * message strings. So every error reported would leak some memory. This patch free's the memory Daniel diff -r 1bd5f9fd5393 qemud/remote.c --- a/qemud/remote.c Fri Jan 30 11:01:29 2009 +0000 +++ b/qemud/remote.c Fri Jan 30 11:01:52 2009 +0000 @@ -363,6 +363,7 @@ rpc_error: remoteDispatchGenericError(&rerr); if (!xdr_remote_error (&xdr, &rerr)) goto fatal_error; + xdr_free((xdrproc_t)xdr_remote_error, (char *)&rerr); } /* Write the length word. */ -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

3 2

[libvirt] PATCH: Fix passing of ifname to QEMU
by Daniel P. Berrange 30 Jan '09

30 Jan '09

When using the type='ethernet' network device configuration for a guest we pass a script, and optional interface name to QEMU. If ifname is omitted, then QEMU allocates one itself. The problem was we were passing an ifname of '(null)' by mistake. This patch corrects that problem and adds a test for it. Daniel diff -r b6a065030fa6 src/qemu_conf.c --- a/src/qemu_conf.c Fri Jan 30 12:28:00 2009 +0000 +++ b/src/qemu_conf.c Fri Jan 30 13:07:11 2009 +0000 @@ -1147,11 +1147,18 @@ int qemudBuildCommandLine(virConnectPtr case VIR_DOMAIN_NET_TYPE_ETHERNET: { char arg[PATH_MAX]; - if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d", - net->ifname, - net->data.ethernet.script, - vlan) >= (PATH_MAX-1)) - goto error; + if (net->ifname) { + if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d", + net->ifname, + net->data.ethernet.script, + vlan) >= (PATH_MAX-1)) + goto error; + } else { + if (snprintf(arg, PATH_MAX-1, "tap,script=%s,vlan=%d", + net->data.ethernet.script, + vlan) >= (PATH_MAX-1)) + goto error; + } ADD_ARG_LIT(arg); } diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,macaddr=00:11:22:33:44:55,vlan=0 -net tap,ifname=nic02,script=/etc/qemu-ifup,vlan=0 -serial none -parallel none -usb diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <interface type='ethernet'> + <mac address='00:11:22:33:44:55'/> + <script path='/etc/qemu-ifup'/> + <target dev='nic02'/> + </interface> + </devices> +</domain> diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth.args --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor pty -pidfile /nowhere/QEMUGuest1.pid -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,macaddr=00:11:22:33:44:55,vlan=0 -net tap,script=/etc/qemu-ifup,vlan=0 -serial none -parallel none -usb diff -r b6a065030fa6 tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml Fri Jan 30 13:07:11 2009 +0000 @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <interface type='ethernet'> + <mac address='00:11:22:33:44:55'/> + <script path='/etc/qemu-ifup'/> + </interface> + </devices> +</domain> diff -r b6a065030fa6 tests/qemuxml2argvtest.c --- a/tests/qemuxml2argvtest.c Fri Jan 30 12:28:00 2009 +0000 +++ b/tests/qemuxml2argvtest.c Fri Jan 30 13:07:11 2009 +0000 @@ -224,6 +224,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_UUID | QEMUD_CMD_FLAG_DOMID); DO_TEST("net-user", 0); DO_TEST("net-virtio", 0); + DO_TEST("net-eth", 0); + DO_TEST("net-eth-ifname", 0); DO_TEST("serial-vc", 0); DO_TEST("serial-pty", 0); diff -r b6a065030fa6 tests/qemuxml2xmltest.c --- a/tests/qemuxml2xmltest.c Fri Jan 30 12:28:00 2009 +0000 +++ b/tests/qemuxml2xmltest.c Fri Jan 30 13:07:11 2009 +0000 @@ -111,6 +111,8 @@ mymain(int argc, char **argv) DO_TEST("misc-no-reboot"); DO_TEST("net-user"); DO_TEST("net-virtio"); + DO_TEST("net-eth"); + DO_TEST("net-eth-ifname"); DO_TEST("sound"); DO_TEST("serial-vc"); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

3 2

[libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
by john cooper 30 Jan '09

30 Jan '09

We have found certain application scenarios where overriding of the default qemu host cache mode provides a substantial improvement in guest performance. In particular, disabling host caching of the file/dev backing a guest drive. A summary of performance metrics may be found below. Attached is a simple patch which adds general support for a "cache=*" attribute to be passed to qemu from libvirt as part of a xml <driver> element. It is parsed in a domain's xml definition as well as being generated during output of the same. For example: <disk type='file' device='disk'> <source file='/guest_roots/fc8.root'/> <target dev='hda'/> <driver name='qemu' type='qwerty' cache='none'/> </disk> where both the existing "type=*" and added "cache=*" attributes are optional and independent. Note this is only intended to serve as an internal hook allowing override of qemu's default cache behavior. At the present it is not proposed as an end-user documented/visible feature. -john Mark Wagner wrote: > This data is provided is provided by the Red Hat Performance > team. It is intended to support the request for adding the > required functionality to libvirt to allow for setting a > storage parameter to control the caching behavior of QEMU. > This value would be passed on the QEMU command line. > > > Our testing of commercial databases in a larger, Enterprise > configuration (MSA, 16 cores, etc) has shown that the default > QEMU caching behavior is not always the best. This set of data > compares the drop off in performance of both writethrough > and writeback cache compared to the nocache option. The main > metric is transactions per minute (tpm). The other "axis" is > the number of simulated users (x1000) > > % nocache TPM Results > K Users WT WB > 10 60% 64% > 20 66% 71% > 40 68% 72% > 60 71% 75% > 80 74% 79% > 100 76% 83% > > From the above set of data, it is clear that the default behavior > of QEMU is the worst performer out of the three cache options for > this type of use case. It is also clear that we at minimum, 25% > of the possible TPM performance just due to the cache setting. > > > > -- john.cooper(a)redhat.com

4 15

[libvirt] [PATCH] Fix virsh migrateuri handling
by john.levon＠sun.com 30 Jan '09

30 Jan '09

# HG changeset patch # User john.levon(a)sun.com # Date 1233286638 28800 # Node ID 0934f4c2e1d446b1902d9ffcf14febf964af1e6a # Parent 6ee939f57a02bf9d332f094e07180e0149e85924 Fix virsh migrateuri handling Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/virsh.c b/src/virsh.c --- a/src/virsh.c +++ b/src/virsh.c @@ -2254,11 +2254,9 @@ cmdMigrate (vshControl *ctl, const vshCm goto done; } - migrateuri = vshCommandOptString (cmd, "migrateuri", &found); - if (!found) migrateuri = NULL; - - dname = vshCommandOptString (cmd, "dname", &found); - if (!found) migrateuri = dname; + migrateuri = vshCommandOptString (cmd, "migrateuri", NULL); + + dname = vshCommandOptString (cmd, "dname", NULL); if (vshCommandOptBool (cmd, "live")) flags |= VIR_MIGRATE_LIVE;

3 2

[libvirt] [PATCH] Fix yet another printf("%s", NULL) case
by john.levon＠sun.com 30 Jan '09

30 Jan '09

# HG changeset patch # User john.levon(a)sun.com # Date 1233282885 28800 # Node ID 6ee939f57a02bf9d332f094e07180e0149e85924 # Parent c60439e564f90b579c07f6349f8f0810a5da1032 Fix yet another printf("%s", NULL) case Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/libvirt.c b/src/libvirt.c --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2691,7 +2691,7 @@ virDomainMigrate (virDomainPtr domain, char *dom_xml = NULL; int cookielen = 0, ret, version = 0; DEBUG("domain=%p, dconn=%p, flags=%lu, dname=%s, uri=%s, bandwidth=%lu", - domain, dconn, flags, dname, uri, bandwidth); + domain, dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); virResetLastError();

2 1

[libvirt] [PATCH] Fix getpwuid_r() usage
by john.levon＠sun.com 30 Jan '09

30 Jan '09

# HG changeset patch # User john.levon(a)sun.com # Date 1233280613 28800 # Node ID 59c231ad7e158acead0c236c3a52f60718b84605 # Parent b203988038c14f1160cbd250a48eda40d4613eca Fix getpwuid_r() usage Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/util.c b/src/util.c --- a/src/util.c +++ b/src/util.c @@ -1477,7 +1477,7 @@ char *virGetUserDirectory(virConnectPtr char *strbuf; char *ret; struct passwd pwbuf; - struct passwd *pw; + struct passwd *pw = NULL; size_t strbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { @@ -1485,7 +1485,14 @@ char *virGetUserDirectory(virConnectPtr return NULL; } - if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0) { + /* + * From the manpage (terrifying but true): + * + * ERRORS + * 0 or ENOENT or ESRCH or EBADF or EPERM or ... + * The given name or uid was not found. + */ + if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0 || pw == NULL) virReportSystemError(conn, errno, _("Failed to find user record for uid '%d'"), uid);

3 2

[libvirt] (resend) Problems with virt-manager checking access on virtual images.
by Daniel J Walsh 30 Jan '09

30 Jan '09

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Met with Cole this morning and we talked about how SELinux can cause people headaches when installing virtual images from random locations. User downloads a iso image to his home directory and then uses virt-manager to install it. Problem is when the user has the whole thing configured, virt-manager tells libvirt to install. It executes qemu and SELinux prevents qemu from reading the iso image because it is labeled user_home_t and qemu is not allowed to read the contents of the home directory. qemu blows up with permission denied and the user is at a loss to what just happened. As we talked we realised this is not just an SELinux problem, but would also happen if a use had an nfs homedir or potentially a samba home directory where root was not allowed access. Also pam_namespace would cause problems, in the /tmp or /home/dwalsh would not be the same for root as they are for the user. One solution to the SELinux problem is to have a label that virt-manager could apply to the iso image (virt_content_ro_t). This would allow qemu to access the file as long as it had search access to the path to the image. solving most of these problems. But the user could still have an access problem that would be tough to diagnose. We came up with the idea of a running a simple helper application to check read access to the image file. During the install, virt-manager could tell libvirt to verify access by executing "qemuaccess /home/dwalsh/windows.iso". If this executable was labeled qemu_exec_t like the other qemu images the same SELinux transitions would happen and we could instantly figure out if SELinux was going to cause problems. As a side benefit we could also check if NFS or samba would cause a problems. If qemuaccess failed, virt-manager could put up a diagnostic message suggesting SELinux, NFS, or Samba might be a problem, and the user could move the iso image to some directory like /var/lib/libvirt/isos/, where libirt would have access. I have attached a version that could solve the problem. Comments? Dan -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkmC9NAACgkQrlYvE4MpobO8egCgpOlWtlSSrC+TPK41fWC9YPWg xwoAn2zYpk5ODoGhl5PXnwkltBKVjO1m =PYqR -----END PGP SIGNATURE-----

2 4

[libvirt] PATCH: Avoid crashing valgrind in LXC driver
by Daniel P. Berrange 30 Jan '09

30 Jan '09

The LXC driver makes use of new clone flags for creating containers. It creates a dummy container which immediately exits in order to test for availabilty of this feature in the kernel. Unfortunately valgrind has no knowledge of these new clone flags, gets very very unhappy and then reports bogus memory leaks, bogus illegal instructions, and then often SEGV's itself. Not cool. While obviously valgrind needs fixing, I looked at its code, and it doesn't seem easy, so this patch adds a quick check for a LD_PRELOAD environemnt variable which contains a library whose name contains 'vgpreload'. If it sees this, LXC driver totally disables itself. This lets me reliably valgrind the libvirtd daemon again. Second, I also move the lxcProbe() call in the lxcOpen() method down a little, so we only probe if we are actually about to try opening the connection. This avoids some unneccessary container creation checks for most virConnectOpen scenarios. Daniel diff -r 6c8e581563aa src/lxc_driver.c --- a/src/lxc_driver.c Fri Jan 30 11:00:43 2009 +0000 +++ b/src/lxc_driver.c Fri Jan 30 11:01:10 2009 +0000 @@ -80,14 +80,14 @@ static virDrvOpenStatus lxcOpen(virConne virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - if (!lxcProbe()) - goto declineConnection; - if (lxc_driver == NULL) goto declineConnection; /* Verify uri was specified */ if (conn->uri == NULL) { + if (!lxcProbe()) + goto declineConnection; + conn->uri = xmlParseURI("lxc:///"); if (!conn->uri) { virReportOOMError(conn); @@ -96,8 +96,11 @@ static virDrvOpenStatus lxcOpen(virConne } else if (conn->uri->scheme == NULL || STRNEQ(conn->uri->scheme, "lxc")) { goto declineConnection; + } else if (!lxcProbe()) { + goto declineConnection; } + conn->privateData = lxc_driver; return VIR_DRV_OPEN_SUCCESS; @@ -1119,6 +1122,13 @@ static int lxcStartup(void) { uid_t uid = getuid(); unsigned int i; + char *ld; + + /* Valgrind gets very annoyed when we clone containers, so + * disable LXC when under valgrind */ + ld = getenv("LD_PRELOAD"); + if (ld && strstr(ld, "vgpreload")) + return -1; /* Check that the user is root */ if (0 != uid) { -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] libvirt for rhel-5?
by Farkas Levente 30 Jan '09

30 Jan '09

hi, is there any plan to be able to buils libvirt on rhel-5 ie. epel-5? currently all virt and kvm releated packages can be build on epel except libvirt since this dbus problem. thanks in advance. -------------------------------------- gcc -I../gnulib/lib -I../gnulib/lib -I../include -I../include -I../src -I/usr/include/libxml2 -Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -Wno-sign-compare -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fasynchronous-unwind-tables -DLOCAL_STATE_DIR=\"/var\" -DSYSCONF_DIR=\"/etc\" -DQEMUD_PID_FILE=\"\" -DREMOTE_PID_FILE=\"/var/run/libvirtd.pid\" -DGETTEXT_PACKAGE=\"libvirt\" -D_REENTRANT -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -Wno-sign-compare -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fasynchronous-unwind-tables -o .libs/libvirtd libvirtd-event.o libvirtd-qemud.o libvirtd-remote.o libvirtd-remote_protocol.o libvirtd-mdns.o ../gnulib/lib/.libs/libgnu.a ../src/.libs/libvirt_driver_qemu.a ../src/.libs/libvirt_driver_lxc.a ../src/.libs/libvirt_driver_uml.a ../src/.libs/libvirt_driver_storage.a ../src/.libs/libvirt_driver_network.a ../src/.libs/libvirt_driver_nodedev.a -L/lib64 -lhal -ldbus-1 ../src/.libs/libvirt.so -lxml2 -lz -lm -lselinux -lgnutls -lsasl2 -lxenstore -lavahi-common -lavahi-client -lpthread ../src/.libs/libvirt_driver_nodedev.a(libvirt_driver_nodedev_la-node_device_hal.o): In function `add_dbus_watch': /home/robot/rpm/BUILD/libvirt-0.5.1/src/node_device_hal.c:578: undefined reference to `dbus_watch_get_unix_fd' collect2: ld returned 1 exit status make[2]: *** [libvirtd] Error 1 make[2]: Leaving directory `/home/robot/rpm/BUILD/libvirt-0.5.1/qemud' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/robot/rpm/BUILD/libvirt-0.5.1' make: *** [all] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.39202 (%build) -------------------------------------- -- Levente "Si vis pacem para bellum!"

3 6