January 2009 - Devel - libvirt List Archives

[libvirt] [PATCH] Fix "empty declaration" compiler error
by john.levon＠sun.com 31 Jan '09

31 Jan '09

# HG changeset patch # User John Levon <john.levon(a)sun.com> # Date 1233339660 28800 # Node ID 69992f43b6f634fa46c7ae2039666378ce378eea # Parent 1b00b26880ad7feae71bb8df9ae86cfcaa428458 Fix "empty declaration" compiler error Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/domain_conf.c b/src/domain_conf.c --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -95,7 +95,7 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DO "default", "none", "writethrough", - "writeback"); + "writeback") VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "mount",

2 1

[libvirt] Migration API broken for xenmigr://
by John Levon 31 Jan '09

31 Jan '09

2675 * Returns the new domain object if the migration was successful, 2676 * or NULL in case of error. Note that the new domain object 2677 * exists in the scope of the destination connection (dconn). This is obviously impossible for xenmigr:///. As a result, virsh migrate always returns an error code: # virsh migrate --live domu-220 xen:/// interpol 18:44:40.974: error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName libvir: Xen error : Domain not found: xenUnifiedDomainLookupByName thewhip:xend # echo $? 1 Since virDomainLookupByName() returns NULL. I will request again, can I remove the 'error' code from xenUnifiedDomainLookupByName? More importantly, how can this be fixed? Perhaps we can return 'domain' instead of 'ddomain' if a migrateuri was specified? Also, where does 'domain' get freed in the case where 'ddomain' is returned? Wish I'd spotted this when it went in :( regards john

2 6

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

31 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 31 Jan '09

31 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 31 Jan '09

31 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 31 Jan '09

31 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 31 Jan '09

31 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 31 Jan '09

31 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 31 Jan '09

31 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 31 Jan '09

31 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