- Devel - libvirt List Archives

[libvirt] Assigning a static IP to the Linux Container through libvirt.
by Kumar L Srikanth-B22348 05 Mar '10

05 Mar '10

Hi, I want to assign a static IP 172.16.1.20 to one of the containers created using libvirt. For that, I defined a network 'mynetwork' using net-define. The XML format for that network is shown below: <network> <name>mynetwork</name> <uuid>f76c44dd-bcef-6b45-4a50-00d575a369ad</uuid> <bridge name='virbr1' stp='on' delay='0' /> <ip address='172.16.1.20' netmask='255.255.255.0'> </ip> </network> I started the network 'mynetwork' using net-start. virsh # net-list --all Name State Autostart ----------------------------------------- default active yes mynetwork active no I created a domain 'vm1_fedora' with libvirt XML: <domain type='lxc' id='1'> <name>vm1_fedora</name> <memory>500000</memory> <os> <type>exe</type> <init>/bin/bash</init> </os> <vcpu>1</vcpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/libexec/libvirt_lxc</emulator> <filesystem type='mount'> <source dir='/root/lxc/fedora12'/> <target dir='/'/> </filesystem> <filesystem type='mount'> <source dir='/root/lxc/fedora12/dev'/> <target dir='/dev'/> </filesystem> <interface type='network'> <source network='mynetwork'/> </interface> <console type='pty' /> </devices> </domain> But, when I start that domain and see for 'ifconfig', I am not able to see the IP address 172.16.1.20 there. [root@localhost /]# ifconfig eth0 Link encap:Ethernet HWaddr 52:54:00:53:4D:BF inet6 addr: fe80::5054:ff:fe53:4dbf/64 Scope:Link UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:18 errors:0 dropped:0 overruns:0 frame:0 TX packets:6 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:840 (840.0 b) TX bytes:384 (384.0 b) lo Link encap:Local Loopback inet addr:127.0.0.1 Mask:255.0.0.0 inet6 addr: ::1/128 Scope:Host UP LOOPBACK RUNNING MTU:16436 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:0 RX bytes:0 (0.0 b) TX bytes:0 (0.0 b) can you please let me know, how can I assign a static IP to the Linux Container? Regards, Srikanth.

2 2

[libvirt] Assigning a Static IP to the Domain.
by Kumar L Srikanth-B22348 05 Mar '10

05 Mar '10

Hi, I am using libvirt version 0.7.6, to create a Linux Container. With the domain XML format, I am able to create a Domain and able to access. But, I am struck at assigning an IP address to the Domain. I want to assign a static IP to eth0 of the Domain and ale to ping that from the host machine. For that, I created a bridge br0 [which can be linked to the interface eth0 of the Domain created by libvirt] with the IP 172.16.1.10. I want to assign an IP 172.16.1.16 to the Domain. Can any one please guide me? Regards, Srikanth.

1 0

[libvirt] [PATCH] Change default for storage uid/gid from getuid()/getgid() to -1/-1
by Laine Stump 05 Mar '10

05 Mar '10

This allows the config to have a setting that means "leave it alone", eg when building a pool where the directory already exists the user may want the current uid/gid of the directory left intact. This actually gets us back to older behavior - before recent changes to the pool building code, we weren't as insistent about honoring the uid/gid settings in the XML, and virt-manager was taking advantage of this behavior. As a side benefit, removing calls to getuid/getgid from the XML parsing functions also seems like a good idea. And having a default that is different from a common/useful value (0 == root) is a good thing in general, as it removes ambiguity from decisions (at least one place in the code was checking for (perms.uid == 0) to see if a special uid was requested). Note that this will only affect newly created pools and volumes. Due to the way that the XML is parsed, then formatted for newly created volumes, all existing pools/volumes already have an explicit uid and gid set. src/conf/storage_conf.c: Remove calls to setuid/setgid for default values of uid/gid, and set them to -1 instead src/storage/storage_backend.c: src/storage/storage_backend_fs.c: Make account for the new default values of perms.uid and perms.gid. --- src/conf/storage_conf.c | 8 +++--- src/storage/storage_backend.c | 25 +++++++++++++---------- src/storage/storage_backend_fs.c | 39 +++++++++++++++++++++++++++---------- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 19a1db9..f59f109 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -539,8 +539,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, if (node == NULL) { /* Set default values if there is not <permissions> element */ perms->mode = defaultmode; - perms->uid = getuid(); - perms->gid = getgid(); + perms->uid = -1; + perms->gid = -1; perms->label = NULL; return 0; } @@ -564,7 +564,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, } if (virXPathNode("./owner", ctxt) == NULL) { - perms->uid = getuid(); + perms->uid = -1; } else { if (virXPathLong("number(./owner)", ctxt, &v) < 0) { virStorageReportError(VIR_ERR_XML_ERROR, @@ -575,7 +575,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, } if (virXPathNode("./group", ctxt) == NULL) { - perms->gid = getgid(); + perms->gid = -1; } else { if (virXPathLong("number(./group)", ctxt, &v) < 0) { virStorageReportError(VIR_ERR_XML_ERROR, diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 3742493..ee6a32e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -241,11 +241,10 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, uid = (vol->target.perms.uid != st.st_uid) ? vol->target.perms.uid : -1; gid = (vol->target.perms.gid != st.st_gid) ? vol->target.perms.gid : -1; if (((uid != -1) || (gid != -1)) - && (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0)) { + && (fchown(fd, uid, gid) < 0)) { virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), - vol->target.path, vol->target.perms.uid, - vol->target.perms.gid); + vol->target.path, uid, gid); goto cleanup; } if (fchmod(fd, vol->target.perms.mode) < 0) { @@ -356,10 +355,12 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } + uid_t uid = (vol->target.perms.uid == -1) ? getuid() : vol->target.perms.uid; + gid_t gid = (vol->target.perms.gid == -1) ? getgid() : vol->target.perms.gid; + if ((createstat = virFileOperation(vol->target.path, O_RDWR | O_CREAT | O_EXCL | O_DSYNC, - vol->target.perms.mode, - vol->target.perms.uid, vol->target.perms.gid, + vol->target.perms.mode, uid, gid, createRawFileOpHook, &hdata, VIR_FILE_OP_FORCE_PERMS | (pool->def->type == VIR_STORAGE_POOL_NETFS @@ -491,14 +492,14 @@ cleanup: static int virStorageBuildSetUIDHook(void *data) { virStorageVolDefPtr vol = data; - if ((vol->target.perms.gid != 0) + if ((vol->target.perms.gid != -1) && (setgid(vol->target.perms.gid) != 0)) { virReportSystemError(errno, _("Cannot set gid to %u before creating %s"), vol->target.perms.gid, vol->target.path); return -1; } - if ((vol->target.perms.uid != 0) + if ((vol->target.perms.uid != -1) && (setuid(vol->target.perms.uid) != 0)) { virReportSystemError(errno, _("Cannot set uid to %u before creating %s"), @@ -517,8 +518,11 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, int filecreated = 0; if ((pool->def->type == VIR_STORAGE_POOL_NETFS) - && (getuid() == 0) - && ((vol->target.perms.uid != 0) || (vol->target.perms.gid != 0))) { + && (((getuid() == 0) + && (vol->target.perms.uid != -1) + && (vol->target.perms.uid != 0)) + || ((vol->target.perms.gid != -1) + && (vol->target.perms.gid != getgid())))) { if (virRunWithHook(cmdargv, virStorageBuildSetUIDHook, vol, NULL) == 0) { /* command was successfully run, check if the file was created */ @@ -547,8 +551,7 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, && (chown(vol->target.path, uid, gid) < 0)) { virReportSystemError(errno, _("cannot chown %s to (%u, %u)"), - vol->target.path, vol->target.perms.uid, - vol->target.perms.gid); + vol->target.path, uid, gid); return -1; } if (chmod(vol->target.path, vol->target.perms.mode) < 0) { diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 8279d78..dfd417f 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -529,16 +529,28 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, /* Now create the final dir in the path with the uid/gid/mode * requested in the config. If the dir already exists, just set * the perms. */ - if ((err = virDirCreate(pool->def->target.path, - pool->def->target.perms.mode, - pool->def->target.perms.uid, - pool->def->target.perms.gid, - VIR_DIR_CREATE_FORCE_PERMS | VIR_DIR_CREATE_ALLOW_EXIST | - (pool->def->type == VIR_STORAGE_POOL_NETFS - ? VIR_DIR_CREATE_AS_UID : 0)) != 0)) { - virReportSystemError(err, _("cannot create path '%s'"), - pool->def->target.path); - goto error; + + struct stat st; + + if ((stat(pool->def->target.path, &st) < 0) + || (pool->def->target.perms.uid != -1)) { + + uid_t uid = (pool->def->target.perms.uid == -1) + ? getuid() : pool->def->target.perms.uid; + gid_t gid = (pool->def->target.perms.gid == -1) + ? getgid() : pool->def->target.perms.gid; + + if ((err = virDirCreate(pool->def->target.path, + pool->def->target.perms.mode, + uid, gid, + VIR_DIR_CREATE_FORCE_PERMS | + VIR_DIR_CREATE_ALLOW_EXIST | + (pool->def->type == VIR_STORAGE_POOL_NETFS + ? VIR_DIR_CREATE_AS_UID : 0)) != 0)) { + virReportSystemError(err, _("cannot create path '%s'"), + pool->def->target.path); + goto error; + } } ret = 0; error: @@ -777,8 +789,13 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } + uid_t uid = (vol->target.perms.uid == -1) + ? getuid() : vol->target.perms.uid; + gid_t gid = (vol->target.perms.gid == -1) + ? getgid() : vol->target.perms.gid; + if ((err = virDirCreate(vol->target.path, vol->target.perms.mode, - vol->target.perms.uid, vol->target.perms.gid, + uid, gid, VIR_DIR_CREATE_FORCE_PERMS | (pool->def->type == VIR_STORAGE_POOL_NETFS ? VIR_DIR_CREATE_AS_UID : 0))) != 0) { -- 1.6.6.1

3 4

[libvirt] [PATCH 0/9] Storage volume wiping API
by David Allan 04 Mar '10

04 Mar '10

Here's the revised volume wiping API patchset (thanks to Dan for the improved name). I'm sending a replacement patchset since the name change made for a very large and messy incremental. This set contains everybody's feedback, which was all very helpful. Many thanks. Dave David Allan (9): Add public API for volume wiping Define the internal driver API for vol wiping Add vol wiping to ESX storage driver struct Implement the public API for vol wiping Define wire protocol format for vol wiping Implement RPC client for vol wiping Implement the remote dispatch bits of vol wiping Simplified version of volume wiping based on feedback from the list. Virsh support for vol wiping daemon/remote.c | 32 +++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++ daemon/remote_dispatch_table.h | 5 + include/libvirt/libvirt.h.in | 2 + src/driver.h | 5 + src/esx/esx_storage_driver.c | 1 + src/libvirt.c | 47 ++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 27 ++++ src/remote/remote_protocol.c | 11 ++ src/remote/remote_protocol.h | 9 ++ src/remote/remote_protocol.x | 8 +- src/storage/storage_driver.c | 224 +++++++++++++++++++++++++++++++++++ tools/virsh.c | 42 +++++++ 15 files changed, 422 insertions(+), 1 deletions(-)

1 9

[libvirt] Libvirt Auth
by Marco Mornati 04 Mar '10

04 Mar '10

Hello guys, I've created a plugin for Hudson (CI) to manage virtmachines using libvirt java (start vm for a build and stop just after). I've just a problem in authentication, in fact I cannot send uname and password using code, libvirt ask me for the credentials on CLI. someone could explain me How I can use authentican or where I can find an usage guide or example? Thanks a lot Bye Marco

3 8

[libvirt] FW: Libvirt and openvz?
by Thomas Graves 04 Mar '10

04 Mar '10

I just recently started using openvz and I'm trying to use libvirt/virsh with it. I followed the directions on page: http://libvirt.org/drvopenvz.html But I get an error trying to run define with the example config file: virsh define virt.conf error: Failed to define domain from virt.conf error: unknown OS type exe sudo virsh -c openvz:/// list error: could not connect to openvz:/// error: failed to connect to the hypervisor I'm using libvirt 0.7.2 with latest rhel5.4 openvz. Does anyone have more example configuration files or other setup instructions? Does Libvirt allow you to specify other resource limits and controls that openvz has - like vzctl set --devnodes device:rw or vzctl set kmemsize? Thanks, Tom

1 1

[libvirt] [PATCH] fix rawhide/F13 --no-add-needed/dlopen-related build failures
by Jim Meyering 04 Mar '10

04 Mar '10

Diego, I saw no follow-up here, http://thread.gmane.org/gmane.comp.emulators.libvirt/21398/focus=21671 so I've fixed your two patches to address Eric's review feedback, and confirmed that they solve the problem on Rawhide. >From 55c182d89cd6e5417eafe9f4bdf2e0ef7f8721f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20Elio=20Petten=C3=B2?= <flameeyes(a)gmail.com> Date: Thu, 4 Mar 2010 16:45:02 +0100 Subject: [PATCH 1/2] build: avoid dlopen-related link failure on rawhide/F13 Instead of using AC_CHECK_LIB and hardcoding -ldl, search for the library needed to get dlopen() and then use the cached value. --- configure.ac | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 682b8b5..d5d62ea 100644 --- a/configure.ac +++ b/configure.ac @@ -1700,7 +1700,7 @@ if test "x$with_driver_modules" = "xyes" ; then old_libs="$LIBS" fail=0 AC_CHECK_HEADER([dlfcn.h],[],[fail=1]) - AC_CHECK_LIB([dl], [dlopen],[],[fail=1]) + AC_SEARCH_LIBS([dlopen], [dl], [], [fail=1]) test $fail = 1 && AC_MSG_ERROR([You must have dlfcn.h / dlopen() support to build driver modules]) @@ -1709,7 +1709,10 @@ if test "x$with_driver_modules" = "xyes" ; then fi if test "$with_driver_modules" = "yes"; then DRIVER_MODULE_CFLAGS="-export-dynamic" - DRIVER_MODULE_LIBS="-ldl" + case $ac_cv_search_dlopen in + no*) DRIVER_MODULE_LIBS= ;; + *) DRIVER_MODULE_LIBS=$ac_cv_search_dlopen ;; + esac AC_DEFINE_UNQUOTED([WITH_DRIVER_MODULES], 1, [whether to build drivers as modules]) fi AM_CONDITIONAL([WITH_DRIVER_MODULES], [test "$with_driver_modules" != "no"]) -- 1.7.0.1.475.gaf89b >From ca7a9f299b0bd415f8434b3c9e6c70528d2f85cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20Elio=20Petten=C3=B2?= <flameeyes(a)gmail.com> Date: Thu, 4 Mar 2010 16:48:10 +0100 Subject: [PATCH 2/2] build: vbox: avoid build failure when linking with --no-add-needed With the recent changes to the linking defaults in Fedora 13 (namely enabling --no-add-needed behaviour by default), we have to pass the dlopen()-providing libraries directly at the link of the module; use the same AC_SEARCH_LIBS function as used before to look for it and add it to the Makefile. --- configure.ac | 6 ++++++ src/Makefile.am | 1 + 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index d5d62ea..923d7b8 100644 --- a/configure.ac +++ b/configure.ac @@ -303,6 +303,12 @@ fi AM_CONDITIONAL([WITH_OPENVZ], [test "$with_openvz" = "yes"]) if test "x$with_vbox" = "xyes"; then + AC_SEARCH_LIBS([dlopen], [dl], [], [AC_MSG_ERROR([Unable to find dlopen()])]) + case $ac_cv_search_dlopen in + no*) DLOPEN_LIBS= ;; + *) DLOPEN_LIBS=$ac_cv_search_dlopen ;; + esac + AC_SUBST([DLOPEN_LIBS]) AC_DEFINE_UNQUOTED([WITH_VBOX], 1, [whether VirtualBox driver is enabled]) fi AM_CONDITIONAL([WITH_VBOX], [test "$with_vbox" = "yes"]) diff --git a/src/Makefile.am b/src/Makefile.am index 2051e5d..67f8b6d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -463,6 +463,7 @@ libvirt_driver_vbox_la_CFLAGS = \ if WITH_DRIVER_MODULES libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version endif +libvirt_driver_vbox_la_LIBADD = $(DLOPEN_LIBS) libvirt_driver_vbox_la_SOURCES = $(VBOX_DRIVER_SOURCES) endif -- 1.7.0.1.475.gaf89b

2 1

[libvirt] FW: Bringing up LXC container from virsh command prompt.
by Kumar L Srikanth-B22348 04 Mar '10

04 Mar '10

Hi Daniel, One more thing is that in my host machine vibr0 inteface is missing. Because of the the 'default' network is in 'inactive' state and if I tried to activate, it's giving the following error: error: Failed to start network default error: internal error '/usr/sbin/dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/default.pid --conf-file= --listen-address 192.168.122.1 --except-interface lo --dhcp-range 192.168.122.2,192.168.122.254' exited with non-zero status 2 and signal 0: dnsmasq: failed to bind listening socket for 192.168.122.1: Address already in use Can you please guide me in this regard? Regards, Srikanth. -----Original Message----- From: Kumar L Srikanth-B22348 Sent: Thursday, March 04, 2010 4:43 PM To: 'Daniel P. Berrange' Cc: libvir-list(a)redhat.com; Reddy B Veera-B22347 Subject: RE: Bringing up LXC container from virsh command prompt. Hi, I want to create eth0 network interface in the container, which is linked to br0 bridge in the host machine. For that, I have created a bridge 'br0' with the IP '172.16.1.10'. Now, I want to create a network interface eth0 in the container with the IP '172.16.1.17'. I created the following network XML for the above requirement. <network> <name>mynetwork</name> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> <bridge name="br0" stp="on" delay="5"/> <forward mode="nat" dev="eth0"/> <ip address="172.16.1.17" netmask="255.255.255.0"/> </network> I am able to define the network, but unable to start the network. The following error is coming when I start the network with the following command: #virsh -c lxc:/// net-start mynetwork error: Failed to start network mynetwork error: cannot create bridge 'br0': File exists Can you please guide me to create a network? My network interfaces in the host machine is shown below: br0 Link encap:Ethernet HWaddr 00:50:BF:27:AA:60 inet addr:172.16.1.10 Bcast:172.16.255.255 Mask:255.255.0.0 inet6 addr: fe80::250:bfff:fe27:aa60/64 Scope:Link UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:80728 errors:0 dropped:0 overruns:0 frame:0 TX packets:170078 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:0 RX bytes:54037401 (51.5 MiB) TX bytes:123567268 (117.8 MiB) eth0 Link encap:Ethernet HWaddr 00:50:BF:27:AA:60 inet6 addr: fe80::250:bfff:fe27:aa60/64 Scope:Link UP BROADCAST RUNNING PROMISC MULTICAST MTU:1500 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:288 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 b) TX bytes:30108 (29.4 KiB) Interrupt:21 Base address:0x1140 eth1 Link encap:Ethernet HWaddr 00:16:76:4B:91:DC inet addr:192.168.1.10 Bcast:192.168.1.255 Mask:255.255.255.0 UP BROADCAST MULTICAST MTU:1500 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 b) TX bytes:0 (0.0 b) eth2 Link encap:Ethernet HWaddr 00:11:95:FC:8D:62 inet addr:10.232.113.80 Bcast:10.232.113.255 Mask:255.255.254.0 inet6 addr: fe80::211:95ff:fefc:8d62/64 Scope:Link UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:563988 errors:0 dropped:0 overruns:0 frame:0 TX packets:391293 errors:1100 dropped:0 overruns:0 carrier:1100 collisions:188525 txqueuelen:1000 RX bytes:565906426 (539.6 MiB) TX bytes:234291753 (223.4 MiB) Interrupt:22 Base address:0xa000 lo Link encap:Local Loopback inet addr:127.0.0.1 Mask:255.0.0.0 inet6 addr: ::1/128 Scope:Host UP LOOPBACK RUNNING MTU:16436 Metric:1 RX packets:23001 errors:0 dropped:0 overruns:0 frame:0 TX packets:23001 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:0 RX bytes:3478027 (3.3 MiB) TX bytes:3478027 (3.3 MiB) Regards, Srikanth. -----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Thursday, March 04, 2010 3:41 PM To: Kumar L Srikanth-B22348 Cc: libvir-list(a)redhat.com; Reddy B Veera-B22347 Subject: Re: Bringing up LXC container from virsh command prompt. On Thu, Mar 04, 2010 at 03:38:43PM +0530, Kumar L Srikanth-B22348 wrote: > So, again I need to add one network interface in the libvirt XML?. It > will not take the interface which was created earlier while creating > container using lxc-tools?. > But, in the container which was created with lxc-tools(lxc-create, > lxc-start ..etc), I am not getting any such issues. The lxc-XXX tools are completely unrelated. They have a configuration file where you specify what network interfaces are created. Likewise if you use libvirt you need to specify what network interfaces are to be created. > Why I am getting these issues in virsh command prompt, when I am > mounting the same rootfs that was used while creating container using > lxc-tools. You need to setup $PATH in your container. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] Bringing up LXC container from virsh command prompt.
by Kumar L Srikanth-B22348 04 Mar '10

04 Mar '10

Hi, I am new to libvirt virsh console. Currently I am working on Linux Containers(LXC). After lot of exploration, I am able to create a Linux Container(fedora12) with the help of lxc-tools(lxc-create, lxc-start, ...etc). I am able to start that container and use it's console. I am able to assign an IP address to the container, by creating a bridge in the host machine and assigning it to the container. Here, my goal is to bring up this container(fedora12) through libvirt virsh command prompt. For that I created a domain with the following XML file: [vm1_fedora_container.xml] <domain type='lxc' id='1'> <name>vm1_fedora</name> <memory>500000</memory> <os> <type>exe</type> <init>/bin/bash</init> </os> <vcpu>1</vcpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/libexec/libvirt_lxc</emulator> <filesystem type='mount'> <source dir='/root/lxc/fedora12'/> /*This is the rootfs directory for my linux container(fedora12)*/ <target dir='/'/> /* I mounted it to '/' directory in the Domain. */ </filesystem> <console type='pty' /> </devices> </domain> I executed the following virsh commands: # virsh --connect lxc:/// define vm1_fedora_container.xml Domain vm1_fedora defined from /root/lxc_devel/fedora.xml # virsh --connect lxc:/// list --all Id Name State ---------------------------------- - vm1_fedora shut off # virsh --connect lxc:/// start vm1_fedora Domain vm1_fedora started # virsh --connect lxc:/// list --all Id Name State ---------------------------------- 7805 vm1_fedora running # virsh --connect lxc:/// console vm1_fedora Connected to domain vm1_fedora Escape character is ^] [root@localhost /]# I am able to view the console of my container(fedora12), but when I execute 'ifconfig' in that console, I am getting the following error: bash: ifconfig: command not found When I execute the same 'ifconfig' command through lxc-tools (like lxc_start -n fedora12 /bin/bash), I am able to get the assigned IP address. Can you please suggest me where did I go wrong? Regards, Srikanth.

2 6

[libvirt] [PATCH 0/9] Vol Zeroing API
by David Allan 04 Mar '10

04 Mar '10

This patch set contains a new API for zeroing out volumes. I called it zero out rather than overwrite because it doesn't overwrite anything in the case of sparse files. It does guarantee that any subsequent reads from the volume will return zeroes. The ESX driver change is part of the patch. Without it, the struct is missing its initializer and fails to build. I'm not sure what our position on the style of struct initializers is; there are both forms in the code. If this style is heresy, we should add a statement to that effect in HACKING. Dave David Allan (9): Fix ESX storage driver struct initializer Add public API for volume zeroing Define the internal driver API for vol zeroing Implement the public API for vol zeroing Define wire protocol format for vol zeroing Implement RPC client for vol zeroing Implement the remote dispatch bits of vol zeroing Simplified version of volume zeroing based on feedback from the list. Virsh support for vol zeroing daemon/remote.c | 32 +++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++ daemon/remote_dispatch_table.h | 5 + include/libvirt/libvirt.h.in | 2 + src/driver.h | 5 + src/esx/esx_storage_driver.c | 39 +------ src/libvirt.c | 47 ++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 27 +++++ src/remote/remote_protocol.c | 11 ++ src/remote/remote_protocol.h | 9 ++ src/remote/remote_protocol.x | 8 +- src/storage/storage_driver.c | 218 +++++++++++++++++++++++++++++++++++ tools/virsh.c | 42 +++++++ 15 files changed, 418 insertions(+), 37 deletions(-)

7 27

[libvirt] [PATCH] Add TCK script for verifying disk based storage pools
by Daniel P. Berrange 04 Mar '10

04 Mar '10

This adds a reasonably comprehensive test script for working with disk based storage pools. It checks - Prevent re-formating an existing partition table (currently missing this check in libvirt) - Formating of a partiton table - Wiping a partition table (currently missing in libvirt) - Creating multiple storage volumes - Deleteing storage volumes * conf/default.cfg: add config entry for specifying host block devices that a test script can use * lib/Sys/Virt/TCK.pm: Add API to get a host block device * lib/Sys/Virt/TCK/StoragePoolBuilder.pm: Fix typo in device XML formatting. Allow pool source format to be specified * scripts/storage/110-disk-pool.t: Test which creates a storage pool, creates some volumes, then deletes it all again. --- conf/default.cfg | 9 ++ lib/Sys/Virt/TCK.pm | 7 ++ lib/Sys/Virt/TCK/StoragePoolBuilder.pm | 13 +++- scripts/storage/110-disk-pool.t | 127 ++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 1 deletions(-) create mode 100644 scripts/storage/110-disk-pool.t diff --git a/conf/default.cfg b/conf/default.cfg index 5b85cbc..288540f 100644 --- a/conf/default.cfg +++ b/conf/default.cfg @@ -127,3 +127,12 @@ host_pci_devices = ( # } # Can list more than one PCI device if many are available ) + +# List of host block devices that the test suite can trash all +# data on, for purposes of testing. You have been warned ! +# Only full block devices allowed, no single partitions, since +# the test suite itself needs to create partitions. +# The disks should be at *least* 512 MB in size +host_block_devices = ( +# /dev/vdb +) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index f101937..d32b03d 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -805,4 +805,11 @@ sub get_host_pci_device { return ($domain, $bus, $slot, $function); } +sub get_host_block_device { + my $self = shift; + my $devindex = @_ ? shift : 0; + + return $self->config("host_block_devices/[$devindex]", undef); +} + 1; diff --git a/lib/Sys/Virt/TCK/StoragePoolBuilder.pm b/lib/Sys/Virt/TCK/StoragePoolBuilder.pm index f92b21a..c9ef2dd 100644 --- a/lib/Sys/Virt/TCK/StoragePoolBuilder.pm +++ b/lib/Sys/Virt/TCK/StoragePoolBuilder.pm @@ -77,6 +77,14 @@ sub target { return $self; } +sub format { + my $self = shift; + + $self->{format} = shift; + + return $self; +} + sub as_xml { my $self = shift; @@ -98,7 +106,7 @@ sub as_xml { } if ($self->{source}->{device}) { foreach my $dev (@{$self->{source}->{device}}) { - $w->emptyTag("dev", path => $dev); + $w->emptyTag("device", path => $dev); } } if ($self->{source}->{adapter}) { @@ -107,6 +115,9 @@ sub as_xml { if ($self->{source}->{name}) { $w->dataElement("name", $self->{source}->{name}); } + if ($self->{format}) { + $w->emptyTag("format", type => $self->{format}); + } $w->endTag("source"); $w->startTag("target"); diff --git a/scripts/storage/110-disk-pool.t b/scripts/storage/110-disk-pool.t new file mode 100644 index 0000000..faa13c5 --- /dev/null +++ b/scripts/storage/110-disk-pool.t @@ -0,0 +1,127 @@ +# -*- perl -*- +# +# Copyright (C) 2009-2010 Red Hat +# Copyright (C) 2009-2010 Daniel P. Berrange +# +# This program is free software; You can redistribute it and/or modify +# it under the GNU General Public License as published by the Free +# Software Foundation; either version 2, or (at your option) any +# later version +# +# The file "LICENSE" distributed along with this file provides full +# details of the terms and conditions +# + +=pod + +=head1 NAME + +storage/110-disk-pool.t - test disk storage pools + +=head1 DESCRIPTION + +The test case validates that it is possible to use all core +functions of the disk pool type. + +=cut + +use strict; +use warnings; + +use Test::More tests => 20; + +use Sys::Virt::TCK; +use Test::Exception; +use File::stat; + +my $tck = Sys::Virt::TCK->new(); +my $conn = eval { $tck->setup(); }; +BAIL_OUT "failed to setup test harness: $@" if $@; +END { + $tck->cleanup if $tck; +} + + +SKIP: { + my $dev = $tck->get_host_block_device(); + + skip "no host block device available", 20 unless defined $dev; + + # Blow away partition table (if any) + open DEV, ">$dev" or die "cannot write to $dev: $!"; + my $data = " "x 512; + + print DEV $data; + close DEV or die "cannot save $dev: $!"; + + my $poolxml = $tck->generic_pool("disk", "tck") + ->source_device($dev) + ->format("dos") + ->target("/dev/") + ->as_xml; + + diag "Defining persistent storage pool $poolxml"; + my $pool; + ok_pool(sub { $pool = $conn->define_storage_pool($poolxml) }, "define persistent storage pool"); + + # Since we blew away the partition table we should not be able to + # start the pool yet + ok_error(sub { $pool->create }, "unable to start un-built storage pool"); + + lives_ok(sub { $pool->build(0) }, "built storage pool"); + + # We should get an error if trying to build a pool which already + # has a partition table written. + ok_error(sub { $pool->build(0) }, "prevent rebuilding existing storage pool"); + + lives_ok(sub { $pool->create }, "started storage pool"); + + my @vols = $pool->list_volumes(); + + is($#vols, -1, "no storage volumes in new pool"); + + my $poolinfo = $pool->get_info(); + + ok($poolinfo->{available} > 1000, "there is some space available in the pool"); + + my $volbase = $dev; + $volbase =~ s,/dev/,,; + + my $vol1xml = $tck->generic_volume($volbase . "1", undef, 1024*1024*256)->as_xml; + my $vol2xml = $tck->generic_volume($volbase . "2", undef, 1024*1024*64)->as_xml; + + diag "Vol $vol1xml $vol2xml"; + my $vol1; + my $vol2; + ok_volume(sub { $vol1 = $pool->create_volume($vol1xml) }, "create disk partition"); + ok_volume(sub { $vol2 = $pool->create_volume($vol2xml) }, "create disk partition"); + + for my $vol (($vol1, $vol2)) { + my $path = xpath($vol, "string(/volume/target/path)"); + my $st = stat($path); + + ok($st, "path $path exists"); + } + + @vols = $pool->list_volumes(); + is($#vols, 1, "two storage volumes in pool"); + + lives_ok(sub { $vol1->delete(0) }, "deleted volume"); + + @vols = $pool->list_volumes(); + is($#vols, 0, "one storage volume in pool"); + + lives_ok(sub { $vol2->delete(0) }, "deleted volume"); + + @vols = $pool->list_volumes(); + is($#vols, -1, "zero storage volume in pool"); + + lives_ok(sub { $pool->destroy() }, "destroyed pool"); + lives_ok(sub { $pool->delete(0) }, "deleted pool"); + + # Since we blew away the partition table we should not be able to + # start the pool anymore + ok_error(sub { $pool->create }, "unable to start un-built storage pool"); + + lives_ok(sub { $pool->undefine() }, "undefined pool"); +} -- 1.6.5.2

3 3

[libvirt] Problem building on Fedora Rawhide
by Laine Stump 04 Mar '10

04 Mar '10

I just checked out a clean tree on a Fedora Rawhide system (up to date as of today), and ran "./autogen.sh && make", and ended up with the following error. Am I missing something, or is configure missing something? make[3]: Entering directory `/home/laine/devel/libvirt/daemon' CC libvirtd-event.o CC libvirtd-libvirtd.o CC libvirtd-remote.o CC libvirtd-dispatch.o CC libvirtd-stream.o CC libvirtd-remote_protocol.o CCLD libvirtd ../src/.libs/libvirt.so: undefined reference to `g_pfnGetFunctions' ../src/.libs/libvirt.so: undefined reference to `g_pVBoxFuncs' ../src/.libs/libvirt.so: undefined reference to `VBoxCGlueInit'

1 0

[libvirt] [PATCH] Fix parser checking of storage pool device
by Daniel P. Berrange 03 Mar '10

03 Mar '10

The storage backend implementations all presume that the XML parser is validating correctness of the source specification. The check for a source device was lost at some point. This allowed for a potential crash in the disk backend. Re-introduce the sanity check * src/conf/storage_conf.c: Re-add check for source device --- src/conf/storage_conf.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 19a1db9..b0f326f 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -689,6 +689,15 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) { } } + /* If DEVICE is the only source type, then its required */ + if (options->flags == VIR_STORAGE_POOL_SOURCE_DEVICE) { + if (!ret->source.ndevice) { + virStorageReportError(VIR_ERR_XML_ERROR, + "%s", _("missing storage pool source device name")); + goto cleanup; + } + } + if ((ret->target.path = virXPathString("string(./target/path)", ctxt)) == NULL) { virStorageReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool target path")); -- 1.6.5.2

3 2

[libvirt] [PATCH] Fix mis-leading error message in pool delete API
by Daniel P. Berrange 03 Mar '10

03 Mar '10

When trying to delete a pool the error message claimed the volume could not be deleted. * src/storage/storage_driver.c: Error message referred to volumes instead of pools --- src/storage/storage_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6b1045a..f3be6de 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -822,7 +822,7 @@ storagePoolDelete(virStoragePoolPtr obj, if (!backend->deletePool) { virStorageReportError(VIR_ERR_NO_SUPPORT, - "%s", _("pool does not support volume delete")); + "%s", _("pool does not support pool deletion")); goto cleanup; } if (backend->deletePool(obj->conn, pool, flags) < 0) -- 1.6.5.2

3 2

[libvirt] [PATCH 0/2] virsh: consistent N_() usage
by Eric Blake 03 Mar '10

03 Mar '10

As discussed here[1], we aren't being very smart about the use of gettext_noop(). In particular, most packages use N_() to mean gettext_noop rather than a synonym to _(); furthermore, our current definition of N_() meant that we were effectively calling gettext(gettext("string")), which has undefined results. Patch 1/2 also fixes a potential crash due to a translation bug, by telling xgettext to enforce that the translation of a string later handed to printf does not violate printf constraints. [1] https://www.redhat.com/archives/libvir-list/2010-March/msg00197.html [PATCH 1/2] virsh: fix existing N_ uses [PATCH 2/2] virsh: use N_ rather than gettext_noop .../0008-Step-8-of-8-Add-virsh-support.patch | 21 +- src/internal.h | 2 +- tools/virsh.c | 895 ++++++++++---------- 3 files changed, 459 insertions(+), 459 deletions(-)

1 2

[libvirt] [PATCH] Add a define for NFS_SUPER_MAGIC
by Chris Lalancette 03 Mar '10

03 Mar '10

Commit 3c12a67b766cce51b47861ccde2be41de369f832 added a dependency on the NFS_SUPER_MAGIC macro, which is defined in linux/magic.h. Unfortunately linux/magic.h is not available in RHEL-5, and causes a compile error. Just define it locally, since this is something that can't change. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu/qemu_driver.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ccb9ef9..efc3be1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -49,8 +49,10 @@ #ifdef __linux__ #include <sys/vfs.h> -#include <linux/magic.h> -#endif +#ifndef NFS_SUPER_MAGIC +#define NFS_SUPER_MAGIC 0x6969 +#endif /* NFS_SUPER_MAGIC */ +#endif /* __linux__ */ #include "virterror_internal.h" #include "logging.h" -- 1.6.6.1

4 5

[libvirt] [PATCH] util: ensure safe{read, write, zero} return is checked
by Eric Blake 03 Mar '10

03 Mar '10

Based on a warning from coverity. The safe* functions guarantee complete transactions on success, but don't guarantee freedom from failure. * src/util/util.h (saferead, safewrite, safezero): Add ATTRIBUTE_RETURN_CHECK. * src/remote/remote_driver.c (remoteIO, remoteIOEventLoop): Ignore some failures. (remoteIOReadBuffer): Adjust error messages on read failure. * daemon/event.c (virEventHandleWakeup): Ignore read failure. --- Coverity claimed that safewrite was checked 37 out of 46 times, but only flagged one instance of a CHECKED_RETURN report, against remote_driver.c. I didn't spot the 9 unchecked calls that coverity's numbers would imply. Meanwhile, adding the attribute ensures that we don't forget this in the future. Please double-check if my use of ignore_value on some of the previously unchecked calls makes sense, or if we need a more invasive patch to deal with real failure. daemon/event.c | 5 +++-- src/remote/remote_driver.c | 13 +++++++++---- src/util/util.h | 9 ++++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/daemon/event.c b/daemon/event.c index 2218a3e..36e9dd3 100644 --- a/daemon/event.c +++ b/daemon/event.c @@ -1,8 +1,8 @@ /* * event.c: event loop for monitoring file handles * + * Copyright (C) 2007, 2010 Red Hat, Inc. * Copyright (C) 2007 Daniel P. Berrange - * Copyright (C) 2007 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -35,6 +35,7 @@ #include "event.h" #include "memory.h" #include "util.h" +#include "ignore-value.h" #define EVENT_DEBUG(fmt, ...) DEBUG(fmt, __VA_ARGS__) @@ -630,7 +631,7 @@ static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED, { char c; virEventLock(); - saferead(fd, &c, sizeof(c)); + ignore_value (saferead(fd, &c, sizeof(c))); virEventUnlock(); } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index def4617..47d8c56 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7809,7 +7809,9 @@ remoteIOReadBuffer(virConnectPtr conn, char errout[1024] = "\0"; if (priv->errfd != -1) { - saferead(priv->errfd, errout, sizeof(errout)); + if (saferead(priv->errfd, errout, sizeof(errout)) < 0) { + strncpy (errout, _("unknown reason"), sizeof errout); + } } virReportSystemError(errno, @@ -7818,7 +7820,9 @@ remoteIOReadBuffer(virConnectPtr conn, } else { char errout[1024] = "\0"; if (priv->errfd != -1) { - saferead(priv->errfd, errout, sizeof(errout)); + if (saferead(priv->errfd, errout, sizeof(errout)) < 0) { + strncpy (errout, _("unknown reason"), sizeof errout); + } } errorf (in_open ? NULL : conn, @@ -8431,7 +8435,8 @@ remoteIOEventLoop(virConnectPtr conn, if (fds[1].revents) { DEBUG0("Woken up from poll by other thread"); - saferead(priv->wakeupReadFD, &ignore, sizeof(ignore)); + ignore_value (saferead(priv->wakeupReadFD, &ignore, + sizeof(ignore))); } if (ret < 0) { @@ -8575,7 +8580,7 @@ remoteIO(virConnectPtr conn, priv->waitDispatch = thiscall; /* Force other thread to wakup from poll */ - safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore)); + ignore_value (safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore))); DEBUG("Going to sleep %d %p %p", thiscall->proc_nr, priv->waitDispatch, thiscall); /* Go to sleep while other thread is working... */ diff --git a/src/util/util.h b/src/util/util.h index cc05abe..34b77fa 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -2,6 +2,7 @@ /* * utils.h: common, generic utility functions * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * @@ -31,9 +32,11 @@ #include <sys/select.h> #include <sys/types.h> -int saferead(int fd, void *buf, size_t count); -ssize_t safewrite(int fd, const void *buf, size_t count); -int safezero(int fd, int flags, off_t offset, off_t len); +int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK; +ssize_t safewrite(int fd, const void *buf, size_t count) + ATTRIBUTE_RETURN_CHECK; +int safezero(int fd, int flags, off_t offset, off_t len) + ATTRIBUTE_RETURN_CHECK; enum { VIR_EXEC_NONE = 0, -- 1.6.6.1

1 1

[libvirt] [PATCH] Fix typo in QEMU migration command name
by Daniel P. Berrange 03 Mar '10

03 Mar '10

The QMP code was running query-migration instead of query-migrate. This doesn't work so well * src/qemu/qemu_monitor_json.c: s/query-migration/query-migrate/ --- src/qemu/qemu_monitor_json.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a94dd0..9c0245c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1108,7 +1108,7 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *total) { int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-migration", + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL); virJSONValuePtr reply = NULL; -- 1.6.5.2

2 1

[libvirt] [PATCH] Add TCK script for host PCI device hotplug/unplug
by Daniel P. Berrange 03 Mar '10

03 Mar '10

This adds support for testing host PCI device hotplug/unplug in libvirt drivers. This requires that the host have a usable IOMMU & hardware virt, and that the device in question can be reset safely. Since the TCK can't assume there are any host PCI devices that can be safely messed around with, the person running the test suite must first list one or more devices in the config file. If no devices are listed, the test will automatically skip all parts, rather than failing. * conf/default.cfg: Config entry to specify PCI devices that the TCK can mess around with * lib/Sys/Virt/TCK.pm: API for getting a host PCI device from the config * scripts/domain/250-pci-host-hotplug.t: Test case for PCI device hotplug/unplug. --- conf/default.cfg | 15 ++++ lib/Sys/Virt/TCK.pm | 18 +++++ scripts/domain/250-pci-host-hotplug.t | 117 +++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 0 deletions(-) create mode 100644 scripts/domain/250-pci-host-hotplug.t diff --git a/conf/default.cfg b/conf/default.cfg index f9c2b5c..5b85cbc 100644 --- a/conf/default.cfg +++ b/conf/default.cfg @@ -112,3 +112,18 @@ host_usb_devices = ( # } # Can list more than one USB device if many are available ) + + +# Host PCI devices that the test suite can safely mess around with +# without risk of breaking the host OS. Using a NIC device is the +# most reliable option. Definitely don't try listing a VGA device. +host_pci_devices = ( +# Slot is only required entry, all others default to 0 +# { +# domain = 0000 +# bus = 00 +# slot = 07 +# function = 0 +# } +# Can list more than one PCI device if many are available +) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index ce03935..f101937 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -787,4 +787,22 @@ sub get_host_usb_device { return ($bus, $device, $vendor, $product); } +sub get_host_pci_device { + my $self = shift; + my $devindex = @_ ? shift : 0; + + my $devs = $self->config("host_pci_devices", []); + + if ($devindex > $#{$devs}) { + return (); + } + + my $domain = $self->config("host_pci_devices/[$devindex]/domain", 0); + my $bus = $self->config("host_pci_devices/[$devindex]/bus", 0); + my $slot = $self->config("host_pci_devices/[$devindex]/slot"); + my $function = $self->config("host_pci_devices/[$devindex]/function", 0); + + return ($domain, $bus, $slot, $function); +} + 1; diff --git a/scripts/domain/250-pci-host-hotplug.t b/scripts/domain/250-pci-host-hotplug.t new file mode 100644 index 0000000..9ccd036 --- /dev/null +++ b/scripts/domain/250-pci-host-hotplug.t @@ -0,0 +1,117 @@ +# -*- perl -*- +# +# Copyright (C) 2009 Red Hat +# Copyright (C) 2009 Daniel P. Berrange +# +# This program is free software; You can redistribute it and/or modify +# it under the GNU General Public License as published by the Free +# Software Foundation; either version 2, or (at your option) any +# later version +# +# The file "LICENSE" distributed along with this file provides full +# details of the terms and conditions +# + +=pod + +=head1 NAME + +domain/250-pci-host-hotplug.t - verify hot plug & unplug of a host PCI device + +=head1 DESCRIPTION + +The test case validates that it is possible to hotplug a pci +host device to a running domain, and then unplug it again. +This requires that the TCK configuration file have at least +one host PCI device listed. + +This first searches for the node device, then detachs it from +the host OS. Next it performs a reset. Then does the hotplug +and unplug, before finally reattaching to the host. + +=cut + +use strict; +use warnings; + +use Test::More tests => 10; + +use Sys::Virt::TCK; +use Test::Exception; + +my $tck = Sys::Virt::TCK->new(); +my $conn = eval { $tck->setup(); }; +BAIL_OUT "failed to setup test harness: $@" if $@; +END { + $tck->cleanup if $tck; +} + + +my $xml = $tck->generic_domain("tck")->as_xml; + +diag "Creating a new transient domain"; +my $dom; +ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); + + +my ($domain, $bus, $slot, $function) = $tck->get_host_pci_device(); + +SKIP: { + # Must have one item be non-zero + unless ($domain || $bus || $slot || $function) { + skip "No host pci device available in configuration file", 9; + } + + my $nodedev; + my @devs = $conn->list_node_devices("pci"); + foreach my $dev (@devs) { + my $thisxml = $dev->get_xml_description(); + + my $xp = XML::XPath->new(xml => $thisxml); + + #warn $thisxml; + my $ndomain = $xp->find("string(/device/capability[\@type='pci']/domain[text()])")->value(); + my $nbus = $xp->find("string(/device/capability[\@type='pci']/bus[text()])")->value(); + my $nslot = $xp->find("string(/device/capability[\@type='pci']/slot[text()])")->value(); + my $nfunction = $xp->find("string(/device/capability[\@type='pci']/function[text()])")->value(); + + if ($ndomain == $domain && + $nbus == $bus && + $nslot == $slot && + $nfunction == $function) { + $nodedev = $dev; + last; + } + } + + ok(defined $nodedev, "found PCI device $domain:$bus:$slot.$function on host"); + + lives_ok(sub { $nodedev->dettach() }, "detached device from host OS"); + lives_ok(sub { $nodedev->reset() }, "reset the host PCI device"); + + my $devxml = + "<hostdev mode='subsystem' type='pci' managed='no'>\n" . + " <source>\n" . + " <address domain='$domain' bus='$bus' slot='$slot' function='$function'/>\n" . + " </source>\n" . + "</hostdev>\n"; + + my $initialxml = $dom->get_xml_description; + + diag "Attaching the new dev $devxml"; + lives_ok(sub { $dom->attach_device($devxml); }, "PCI dev has been attached"); + + my $newxml = $dom->get_xml_description; + ok($newxml =~ m|<hostdev|, "new XML has extra PCI dev present"); + + diag "Detaching the new dev $devxml"; + lives_ok(sub { $dom->detach_device($devxml); }, "PCI dev has been detached"); + + lives_ok(sub { $nodedev->reset() }, "reset the host PCI device"); + lives_ok(sub { $nodedev->reattach() }, "reattached device to host OS"); + + my $finalxml = $dom->get_xml_description; + + is($initialxml, $finalxml, "final XML has removed the disk") +} + -- 1.6.5.2

2 1

[libvirt] [PATCH] uml: avoid crash on partial read
by Eric Blake 03 Mar '10

03 Mar '10

Coverity detected a potential dereference of uninitialized memory if recvfrom got cut short. * src/uml/uml_driver.c (umlMonitorCommand): Validate complete read prior to dereferencing res. --- The patch borrows ideas from macvtap.c, the only other file in libvirt that currently uses recvfrom. I did not analyze whether this is a security hole, where a malicious UDP packet could intentionally force the dereferencing of uninitialized memory to misbehave in a controlled manner. src/uml/uml_driver.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index bbea429..eec239f 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -733,14 +733,24 @@ static int umlMonitorCommand(virConnectPtr conn, } do { + ssize_t nbytes; addrlen = sizeof(addr); - if (recvfrom(priv->monitor, &res, sizeof res, 0, - (struct sockaddr *)&addr, &addrlen) < 0) { + nbytes = recvfrom(priv->monitor, &res, sizeof res, 0, + (struct sockaddr *)&addr, &addrlen) < 0; + if (nbytes < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; virReportSystemError(errno, _("cannot read reply %s"), cmd); goto error; } + if (nbytes < sizeof res) { + virReportSystemError(errno, + _("incomplete reply %s"), + cmd); + goto error; + } if (VIR_REALLOC_N(retdata, retlen + res.length) < 0) { virReportOOMError(); -- 1.6.6.1

3 10

[libvirt] [PATCH] don't let a bogus packet trigger over-allocation and segfault
by Jim Meyering 03 Mar '10

03 Mar '10

Another not-really-urgent fix: >From 4b56f03dee82657be3af5c79c826ae3091fbf522 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 3 Mar 2010 16:50:02 +0100 Subject: [PATCH] don't let a bogus packet trigger over-allocation and segfault * src/xen/proxy_internal.c (xenProxyDomainDumpXML): An invalid packet could include a too-large "ans.len" value, which would make us allocate too much memory and then copy data from beyond the end of "ans", possibly evoking a segfault. Ensure that the value we use is no larger than the remaining portion of "ans". Also, change unnecessary memmove to memcpy (src and dest obviously do not overlap, so no need to use memmove). --- src/xen/proxy_internal.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c index 8e1c226..bd234ec 100644 --- a/src/xen/proxy_internal.c +++ b/src/xen/proxy_internal.c @@ -978,26 +978,27 @@ xenProxyDomainDumpXML(virDomainPtr domain, int flags ATTRIBUTE_UNUSED) memset(&req, 0, sizeof(req)); req.command = VIR_PROXY_DOMAIN_XML; req.data.arg = domain->id; req.len = sizeof(req); ret = xenProxyCommand(domain->conn, &req, &ans, 0); if (ret < 0) { return(NULL); } - if (ans.len <= sizeof(virProxyPacket)) { + if (ans.len <= sizeof(virProxyPacket) + || ans.len > sizeof (ans) - sizeof(virProxyPacket)) { virProxyError(domain->conn, VIR_ERR_OPERATION_FAILED, __FUNCTION__); return (NULL); } xmllen = ans.len - sizeof(virProxyPacket); if (VIR_ALLOC_N(xml, xmllen+1) < 0) { virReportOOMError(); return NULL; } - memmove(xml, &ans.extra.dinfo, xmllen); + memcpy(xml, &ans.extra.dinfo, xmllen); xml[xmllen] = '\0'; return(xml); } /** * xenProxyDomainGetOSType: * @domain: a domain object -- 1.7.0.1.464.g0adc7

2 3

[libvirt] [PATCH] virFileReadLimFD: diagnose maxlen <= 0, rather than passing it on...
by Jim Meyering 03 Mar '10

03 Mar '10

Not urgent. >From 35a24209934cb983ca52e74f580b3ea963d92f57 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 3 Mar 2010 11:42:11 +0100 Subject: [PATCH] virFileReadLimFD: diagnose maxlen <= 0, rather than passing it on... to saferead_lim, which interprets it as a size_t. * src/util/util.c (virFileReadLimFD): Do not malfunction when maxlen < -1. Return -1,EINVAL in that case. Handle maxlen==0 in the same manner. --- src/util/util.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 34c585d..7a3a3c4 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1030,10 +1030,17 @@ saferead_lim (int fd, size_t max_len, size_t *length) /* A wrapper around saferead_lim that maps a failure due to exceeding the maximum size limitation to EOVERFLOW. */ -int virFileReadLimFD(int fd, int maxlen, char **buf) +int +virFileReadLimFD(int fd, int maxlen, char **buf) { size_t len; - char *s = saferead_lim (fd, maxlen+1, &len); + char *s; + + if (maxlen <= 0) { + errno = EINVAL; + return -1; + } + s = saferead_lim (fd, maxlen+1, &len); if (s == NULL) return -1; if (len > maxlen || (int)len != len) { -- 1.7.0.1.464.g0adc7

2 1

[libvirt] [PATCH] qemu restore: don't let corrupt input provoke unwarranted OOM
by Jim Meyering 03 Mar '10

03 Mar '10

As the log says, we must not use a just-read (and unverified) header.xml_len as the size of buffer to allocate. While looking at this, I noticed a small problem with virFileReadLimFD. Passing it a limit < -1 would cause int->size_t trouble with saferead_lim. Separate patch coming up. >From a4906ba24b576f3219234c519c88d102df2173b2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 3 Mar 2010 11:27:16 +0100 Subject: [PATCH] qemu restore: don't let corrupt input provoke unwarranted OOM * src/qemu/qemu_driver.c (qemudDomainRestore): A corrupt save file (in particular, a too-large header.xml_len value) would cause an unwarranted out-of-memory error. Do not trust the just-read header.xml_len. Instead, merely use that as a hint, and read/allocate up to that number of bytes from the file. Also verify that header.xml_len is positive; if it were negative, passing it to virFileReadLimFD could cause trouble. --- src/qemu/qemu_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f7c459..5c9d003 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4725,44 +4725,45 @@ static int qemudDomainRestore(virConnectPtr conn, if (saferead(fd, &header, sizeof(header)) != sizeof(header)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read qemu header")); goto cleanup; } if (memcmp(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)) != 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("image magic is incorrect")); goto cleanup; } if (header.version > QEMUD_SAVE_VERSION) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("image version is not supported (%d > %d)"), header.version, QEMUD_SAVE_VERSION); goto cleanup; } - if (VIR_ALLOC_N(xml, header.xml_len) < 0) { - virReportOOMError(); + if (header.xml_len <= 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("invalid XML length: %d"), header.xml_len); goto cleanup; } - if (saferead(fd, xml, header.xml_len) != header.xml_len) { + if (virFileReadLimFD(fd, header.xml_len, &xml) != header.xml_len) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML")); goto cleanup; } /* Create a domain from this XML */ if (!(def = virDomainDefParseString(driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to parse XML")); goto cleanup; } if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def))) { -- 1.7.0.1.464.g0adc7

2 1

[libvirt] [PATCH 0/1] - for comments/help only, nearly working root-squash-domain-save
by Laine Stump 03 Mar '10

03 Mar '10

(This patch requires that you first apply the virFileOperation patches that I sent to the list yesterday.) I'm hoping someone else's keen eye can see what I'm failing to see here. The patch following this message makes domain save "work" on root-squash NFS (both the libvirt header and the QEMU state are saved to the file), but the files that it saves don't restore properly, even when simply saving to a local directory w/o forking - the guest screen comes up black, or reboots immediately, or some other such thing. I've looked at the data files and the code, and can't see what difference is causing this - at least the header written by libvirt seems to be the same, and the part written by qemu starts in the same place and looks similar. If anyone's got an idea, please let me know! I'd like to get this figured out and a working patch submitted before the day is done.

3 6

[libvirt] [PATCH 0/4] Misc hotplug fixes for QEMU
by Daniel P. Berrange 03 Mar '10

03 Mar '10

This is a series of fixes for hotplug in QEMU

1 4

[libvirt] [PATCH 0/1] Support domain restore from root-squash NFS
by Laine Stump 03 Mar '10

03 Mar '10

Most of the explanation is in the commit message with the patch proper. When reveiwing please take pains to point out any places where I may not be diligent enough cleaning up loose ends, especially in terms of assuring that the child process always terminates properly no matter what happens. The (unrelated) race condition in qemudDomainRestore() remains - I solved it locally by putting a sleep(10) just before qemuMonitorStartCPUs(), but it needs something with a bit higher comfort factor ;-)

5 11

[libvirt] [PATCH] virsh: silence compiler warning
by Eric Blake 03 Mar '10

03 Mar '10

gcc warns: virsh.c:1879: warning: '0' flag ignored with '-' flag in gnu_printf format * tools/virsh.c (cmdDomjobinfo): Delete useless flag. --- tools/virsh.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a6a637d..65487ed 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1876,27 +1876,27 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-17s %-12llu ms\n", _("Time remaining:"), info.timeRemaining); if (info.dataTotal || info.dataRemaining || info.dataProcessed) { val = prettyCapacity(info.dataProcessed, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("Data processed:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("Data processed:"), val, unit); val = prettyCapacity(info.dataRemaining, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("Data remaining:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("Data remaining:"), val, unit); val = prettyCapacity(info.dataTotal, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("Data total:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("Data total:"), val, unit); } if (info.memTotal || info.memRemaining || info.memProcessed) { val = prettyCapacity(info.memProcessed, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("Memory processed:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("Memory processed:"), val, unit); val = prettyCapacity(info.memRemaining, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("Memory remaining:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("Memory remaining:"), val, unit); val = prettyCapacity(info.memTotal, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("Memory total:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("Memory total:"), val, unit); } if (info.fileTotal || info.fileRemaining || info.fileProcessed) { val = prettyCapacity(info.fileProcessed, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("File processed:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("File processed:"), val, unit); val = prettyCapacity(info.fileRemaining, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("File remaining:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("File remaining:"), val, unit); val = prettyCapacity(info.fileTotal, &unit); - vshPrint(ctl, "%-17s %-0.3lf %s\n", _("File total:"), val, unit); + vshPrint(ctl, "%-17s %-.3lf %s\n", _("File total:"), val, unit); } } else { ret = FALSE; -- 1.6.6.1

3 4

[libvirt] [PATCH] Add TCK script for host USB device hotplug/unplug
by Daniel P. Berrange 03 Mar '10

03 Mar '10

This adds support for testing host USB device hotplug/unplug in libvirt drivers. This currently fails in all known libvirt drivers :-) The QEMU driver only support the hotplug action, not unplug, hence the failure. Since the TCK can't assume there are any host USB devices that can be safely messed around with, the person running the test suite must first list one or more devices in the config file. If no devices are listed, the test will automatically skip all parts, rather than failing. * conf/default.cfg: Config entry to specify USB devices that the TCK can mess around with * lib/Sys/Virt/TCK.pm: API for getting a host USB device from the config * scripts/domain/240-usb-host-hotplug.t: Test case for USB device hotplug/unplug. --- conf/default.cfg | 13 +++++ lib/Sys/Virt/TCK.pm | 18 +++++++ scripts/domain/240-usb-host-hotplug.t | 89 +++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 0 deletions(-) create mode 100644 scripts/domain/240-usb-host-hotplug.t diff --git a/conf/default.cfg b/conf/default.cfg index 556225e..f9c2b5c 100644 --- a/conf/default.cfg +++ b/conf/default.cfg @@ -99,3 +99,16 @@ kernels = ( # ostype = exe # } ) + +# Host USB devices that the test suite can safely mess around with +# without risk of breaking the host OS +host_usb_devices = ( +# Must list either vendor+product, or bus+dev, or both +# { +# vendor = 0x0627 +# product = 0x0001 +# bus = 001 +# device = 002 +# } +# Can list more than one USB device if many are available +) diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm index 9e6b554..ce03935 100644 --- a/lib/Sys/Virt/TCK.pm +++ b/lib/Sys/Virt/TCK.pm @@ -769,4 +769,22 @@ sub xpath { return $xp->find($path); } +sub get_host_usb_device { + my $self = shift; + my $devindex = @_ ? shift : 0; + + my $devs = $self->config("host_usb_devices", []); + + if ($devindex > $#{$devs}) { + return (); + } + + my $bus = $self->config("host_usb_devices/[$devindex]/bus", undef); + my $device = $self->config("host_usb_devices/[$devindex]/device", undef); + my $vendor = $self->config("host_usb_devices/[$devindex]/vendor", undef); + my $product = $self->config("host_usb_devices/[$devindex]/product", undef); + + return ($bus, $device, $vendor, $product); +} + 1; diff --git a/scripts/domain/240-usb-host-hotplug.t b/scripts/domain/240-usb-host-hotplug.t new file mode 100644 index 0000000..4729300 --- /dev/null +++ b/scripts/domain/240-usb-host-hotplug.t @@ -0,0 +1,89 @@ +# -*- perl -*- +# +# Copyright (C) 2009 Red Hat +# Copyright (C) 2009 Daniel P. Berrange +# +# This program is free software; You can redistribute it and/or modify +# it under the GNU General Public License as published by the Free +# Software Foundation; either version 2, or (at your option) any +# later version +# +# The file "LICENSE" distributed along with this file provides full +# details of the terms and conditions +# + +=pod + +=head1 NAME + +domain/240-usb-host-hotplug.t - verify hot plug & unplug of a host USB device + +=head1 DESCRIPTION + +The test case validates that it is possible to hotplug a usb +host device to a running domain, and then unplug it again. +This requires that the TCK configuration file have at least +one host USB device listed. + +=cut + +use strict; +use warnings; + +use Test::More tests => 5; + +use Sys::Virt::TCK; +use Test::Exception; + +my $tck = Sys::Virt::TCK->new(); +my $conn = eval { $tck->setup(); }; +BAIL_OUT "failed to setup test harness: $@" if $@; +END { + $tck->cleanup if $tck; +} + + +my $xml = $tck->generic_domain("tck")->as_xml; + +diag "Creating a new transient domain"; +my $dom; +ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain object"); + + +my ($bus, $device, $vendor, $product) = $tck->get_host_usb_device(); + +SKIP: { + # Must have one of the pairs at least + unless (($bus && $device) || ($vendor && $product)) { + skip "No host usb device available in configuration file", 4; + } + + my $devxml = "<hostdev mode='subsystem' type='usb'>\n" . + "<source>\n"; + if ($bus && $device) { + $devxml .= "<address bus='$bus' device='$device'/>\n" + } + if ($vendor && $product) { + $devxml .= "<vendor id='$vendor'/>\n"; + $devxml .= "<product id='$product'/>\n"; + } + $devxml .= "</source>\n" . + "</hostdev>\n"; + + my $initialxml = $dom->get_xml_description; + + diag "Attaching the new dev $devxml"; + lives_ok(sub { $dom->attach_device($devxml); }, "USB dev has been attached"); + + my $newxml = $dom->get_xml_description; + ok($newxml =~ m|<hostdev|, "new XML has extra USB dev present"); + + diag "Detaching the new dev $devxml"; + lives_ok(sub { $dom->detach_device($devxml); }, "USB dev has been detached"); + + + my $finalxml = $dom->get_xml_description; + + is($initialxml, $finalxml, "final XML has removed the disk") +} + -- 1.6.5.2

1 0

[libvirt] [PATCH] web docs -- macvtap mode explanation
by Stefan Berger 03 Mar '10

03 Mar '10

This adds more information about the different macvtap device modes. Regards, Stefan

1 0

[libvirt] Xen Daemon error : internal error domain information incomplete, missing cpu_cap
by "J. Oriol Fitó" 03 Mar '10

03 Mar '10

Hi all, I'm using VirtMonitor on top of Xen and I'm wondering when the 'cpu_cap' feature will be available. Best regards, Oriol

1 0

[libvirt] [PATCH] esx: don't ignore failure on close
by Eric Blake 03 Mar '10

03 Mar '10

Another warning caught by coverity. Continue to perform best-effort closing and resource release, but warn the caller about the failure. * src/esx/esx_driver.c (esxClose): Return an error on failure to close. --- src/esx/esx_driver.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index e125a09..45b389f 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2,6 +2,7 @@ /* * esx_driver.c: core driver methods for managing VMware ESX hosts * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2009, 2010 Matthias Bolte <matthias.bolte(a)googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max(a)rfc2324.org> * @@ -559,16 +560,19 @@ static int esxClose(virConnectPtr conn) { esxPrivate *priv = conn->privateData; + int result = 0; - esxVI_EnsureSession(priv->host); - - esxVI_Logout(priv->host); + if (esxVI_EnsureSession(priv->host) < 0 || + esxVI_Logout(priv->host) < 0) { + result = -1; + } esxVI_Context_Free(&priv->host); if (priv->vCenter != NULL) { - esxVI_EnsureSession(priv->vCenter); - - esxVI_Logout(priv->vCenter); + if (esxVI_EnsureSession(priv->vCenter) < 0 || + esxVI_Logout(priv->vCenter) < 0) { + result = -1; + } esxVI_Context_Free(&priv->vCenter); } @@ -579,7 +583,7 @@ esxClose(virConnectPtr conn) conn->privateData = NULL; - return 0; + return result; } -- 1.6.6.1

3 2

[libvirt] [PATCH 1/2] Use AC_SEARCH_LIBS to find the library to use for dlopen().
by Diego Elio Pettenò 03 Mar '10

03 Mar '10

Instead of using AC_CHECK_LIB and hardcoding -ldl, search for the library needed to get dlopen() and then use the cached value. --- configure.ac | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 5b1eb5f..a3f28c4 100644 --- a/configure.ac +++ b/configure.ac @@ -1705,7 +1705,7 @@ if test "x$with_driver_modules" = "xyes" ; then old_libs="$LIBS" fail=0 AC_CHECK_HEADER([dlfcn.h],[],[fail=1]) - AC_CHECK_LIB([dl], [dlopen],[],[fail=1]) + AC_SEARCH_LIBS([dlopen], [dl], [], [fail=1]) test $fail = 1 && AC_MSG_ERROR([You must have dlfcn.h / dlopen() support to build driver modules]) @@ -1714,7 +1714,7 @@ if test "x$with_driver_modules" = "xyes" ; then fi if test "$with_driver_modules" = "yes"; then DRIVER_MODULES_CFLAGS="-export-dynamic" - DRIVER_MODULES_LIBS="-ldl" + DRIVER_MODULES_LIBS="$ac_cv_search_dlopen" AC_DEFINE_UNQUOTED([WITH_DRIVER_MODULES], 1, [whether to build drivers as modules]) fi AM_CONDITIONAL([WITH_DRIVER_MODULES], [test "$with_driver_modules" != "no"]) -- 1.7.0

4 5

[libvirt] [PATCH] HACKING: add a section on preprocessor conventions
by Eric Blake 03 Mar '10

03 Mar '10

* HACKING: Document recently-discussed style issues. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Turn recent discussions: https://www.redhat.com/archives/libvir-list/2010-March/msg00058.html https://www.redhat.com/archives/libvir-list/2010-March/msg00120.html into policy, so I don't keep debating style points ;) Can wait till after 0.7.7 (particularly since the vararg macro series is also waiting). HACKING | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/HACKING b/HACKING index be8725d..50b24ee 100644 --- a/HACKING +++ b/HACKING @@ -102,6 +102,17 @@ Usually they're in macro definitions or strings, and should be converted anyhow. +Preprocessor +============ +For variadic macros, stick with C99 syntax: + +#define vshPrint(_ctl, ...) fprintf(stdout, __VA_ARGS__) + +Use parenthesis when checking if a macro is defined: + +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE) + + C types ======= Use the right type. -- 1.6.6.1

2 1

[libvirt] Update on 0.7.7 release schedule
by Daniel Veillard 02 Mar '10

02 Mar '10

As you can have noticed we still have patches queued up and not yet pushed, plus added a new interface yesterday. I'm travelling all of this week, so my bandwidth is limited. Plus February is a short month and we skipped a bit for 0.7.6, so time to validate and push patches has been shorter than usual. So I suggest to still push patches today and tomorrow for features, and start the freeze on thusday evening, and delaying the release to mid of next week. But still try to keep the expected cycle for 0.7.8 with a freeze on March 19 and a release on the 26 :-) Hope this doesn't disrupt other people schedules ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 1

[libvirt] [PATCH] Fix detection of errors in QEMU device_add command
by Daniel P. Berrange 02 Mar '10

02 Mar '10

The code assumed that 'device_add' returned an empty string upon success. This is not true, it sometimes prints random debug info. THus we need to check for an explicit fail string * src/qemu/qemu_monitor_text.c: Fix error checking of the device_add monitor command --- src/qemu/qemu_monitor_text.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7604ae8..c60e840 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2122,9 +2122,11 @@ int qemuMonitorTextAddDevice(qemuMonitorPtr mon, goto cleanup; } - if (STRNEQ(reply, "")) { + /* If the command failed qemu prints: + * Could not add ... */ + if (strstr(reply, "Could not add ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("adding %s device failed: %s"), devicestr, reply); + _("adding %s device failed"), devicestr); goto cleanup; } -- 1.6.5.2

2 1

[libvirt] [PATCH] Fix safezero()
by Jiri Denemark 02 Mar '10

02 Mar '10

Various safezero() implementations used either -1, errno or -errno return values. This patch fixes them all to return -1 and set errno appropriately. There was also a bug in size parameter passed to safewrite() which could result in an attempt to write gigabytes out of a megabyte buffer. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/storage/storage_backend.c | 4 ++-- src/util/util.c | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 3742493..849f01b 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -316,7 +316,7 @@ static int createRawFileOpHook(int fd, void *data) { if ((r = safezero(fd, 0, hdata->vol->allocation - remain, bytes)) != 0) { ret = errno; - virReportSystemError(r, _("cannot fill file '%s'"), + virReportSystemError(errno, _("cannot fill file '%s'"), hdata->vol->target.path); goto cleanup; } @@ -327,7 +327,7 @@ static int createRawFileOpHook(int fd, void *data) { if ((r = safezero(fd, 0, 0, remain)) != 0) { ret = errno; - virReportSystemError(r, _("cannot fill file '%s'"), + virReportSystemError(errno, _("cannot fill file '%s'"), hdata->vol->target.path); goto cleanup; } diff --git a/src/util/util.c b/src/util/util.c index cf7bba5..34c585d 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -146,11 +146,11 @@ int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len) */ r = ftruncate(fd, offset + len); if (r < 0) - return -errno; + return -1; buf = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset); if (buf == MAP_FAILED) - return -errno; + return -1; memset(buf, 0, len); munmap(buf, len); @@ -167,24 +167,26 @@ int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len) unsigned long long remain, bytes; if (lseek(fd, offset, SEEK_SET) < 0) - return errno; + return -1; /* Split up the write in small chunks so as not to allocate lots of RAM */ remain = len; bytes = 1024 * 1024; r = VIR_ALLOC_N(buf, bytes); - if (r < 0) - return -ENOMEM; + if (r < 0) { + errno = ENOMEM; + return -1; + } while (remain) { if (bytes > remain) bytes = remain; - r = safewrite(fd, buf, len); + r = safewrite(fd, buf, bytes); if (r < 0) { VIR_FREE(buf); - return r; + return -1; } /* safewrite() guarantees all data will be written */ -- 1.7.0

3 3

[libvirt] [PATCH] lxc: detect socket failure
by Eric Blake 02 Mar '10

02 Mar '10

Coverity warned about a potential close(-1). * src/lxc/lxc_controller.c (lxcControllerMain): React to failure to accept. --- src/lxc/lxc_controller.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 95970cc..318106e 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1,5 +1,6 @@ /* + * Copyright (C) 2010 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_controller.c: linux container process controller * @@ -348,8 +349,13 @@ static int lxcControllerMain(int monitor, numEvents = epoll_wait(epollFd, &epollEvent, 1, timeout); if (numEvents > 0) { if (epollEvent.data.fd == monitor) { int fd = accept(monitor, NULL, 0); + if (fd < 0) { + virReportSystemError(errno, "%s", + _("epoll_ctl(client) failed")); + goto cleanup; + } if (client != -1) { /* Already connected, so kick new one out */ close(fd); continue; } -- 1.6.6.1

2 1

[libvirt] [PATCH] build: silence coverity warning in node_device
by Eric Blake 02 Mar '10

02 Mar '10

All other uses of get_str_prop in this file that ignored failure explicitly cast to void. * src/node_device/node_device_hal.c (dev_create): Silence coverity warning. --- src/node_device/node_device_hal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 6cc2864..4bf445d 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -463,7 +463,7 @@ static void dev_create(const char *udi) goto cleanup; /* Some devices don't have a path in sysfs, so ignore failure */ - get_str_prop(ctx, udi, "linux.sysfs_path", &devicePath); + (void)get_str_prop(ctx, udi, "linux.sysfs_path", &devicePath); dev = virNodeDeviceAssignDef(&driverState->devs, def); -- 1.6.6.1

3 2

[libvirt] [PATCH] Fix USB/PCI device address aliases in QEMU hotplug driver
by Daniel P. Berrange 02 Mar '10

02 Mar '10

The USB/PCI device hotplug code for the QEMU driver was forgetting to allocate a unique device alias. * src/qemu/qemu_driver.c: Fill in device alias for USB/PCI devices --- src/qemu/qemu_driver.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c2bd46d..5ac3316 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5878,6 +5878,8 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, } if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) + goto error; if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0) goto error; @@ -5926,9 +5928,12 @@ static int qemudDomainAttachHostUsbDevice(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; - if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && - !(devstr = qemuBuildUSBHostdevDevStr(hostdev))) - goto error; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) + goto error; + if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) + goto error; + } if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) { virReportOOMError(); -- 1.6.6.1

2 1

[libvirt] [PATCH 0/6] Add more guest clock mods
by Daniel P. Berrange 02 Mar '10

02 Mar '10

An update of http://www.redhat.com/archives/libvir-list/2010-February/msg00104.html The two current options are: <clock offset='utc'/> <clock offset='localtime'/> This introduces a way to set a specific timezone <clock offset='timezone' timezone='Europe/Paris'/> And a way to set a completely arbitrary time, by giving the number of seconds offset from UTC. <clock offset='variable' adjustment='123456'/> Since last time, I removed the 'timezone' attribute from the existing offset='localtime' option, and introduced the new explicit option offset='timezone'. That way we don't change semantics of the existing options at all.

4 22

[libvirt] [PATCH 0/6] SCSI-Disk-Hotremove
by Wolfgang Mauerer 02 Mar '10

02 Mar '10

[PATCH 0/6] SCSI-Disk-Hotremove This series adds support for hot-removing SCSI disks when qemu supports the drive/device model. It also contains fixes for SCSI controller hotremoval and the build process. Cheers, Wolfgang --- Siemens AG, Corporate Technology, CT T TC 4 Corporate Competence Centre Embedded Linux Makefile.am | 7 +-- qemu/qemu_driver.c | 97 +++++++++++++++++++++++++++++++++++++++++++---- qemu/qemu_monitor.c | 13 ++++++ qemu/qemu_monitor.h | 3 + qemu/qemu_monitor_json.c | 24 +++++++++++ qemu/qemu_monitor_json.h | 3 + qemu/qemu_monitor_text.c | 40 +++++++++++++++++++ qemu/qemu_monitor_text.h | 3 + 8 files changed, 180 insertions(+), 10 deletions(-)

4 19

[libvirt] libvirt/dnsmasq integration.
by Simon Kelley 01 Mar '10

01 Mar '10

As the principal maintainer of dnsmasq, I'm seeing increasing reports of problems on systems which run both dnsmasq and libvirt. I'm fairly sure I understand what's going on in these cases, and I have a few proposals for changes in libvir and dnsmasq that should fix things. The problem is that libvirt runs a private instance of dnsmasq: on machines which are also running a "system" dnsmasq daemon, this can cause problems. Some background: dnsmasq can run in two modes. Default mode: dnsmasq binds the wildcard address and does network magic to determine which interface request packets actually come from, so that the results can be sent back with the correct source address. This has the advantage that network interfaces can come and go and change IP address and dnsmasq will keep working. It's possible to restrict dnsmasq to only reply to requests on some interfaces; requests from other interfaces will be read by dnsmasq and then silently dropped. Telling dnsmasq to use an interface which doesn't exist but might in the future will result in a logged warning, but dnsmasq will still start and when the interface comes up it will work. Bind-interfaces mode: This is the traditional way to do UDP servers. At startup dnsmasq enumerates all the extant interfaces and then opens a socket for each one, listening on the interfaces's IP address. Interfaces may be skipped if excluded by the --interface and --except-interface flags, and any interface specified in --interface which doesn't exist at start-up will generate a fatal error. In almost all cases, default mode is better: --bind-interfaces is only there to cope with old platforms which don't support enough socket options to do default mode. The only time when --bind-interfaces works better is when it's desirable to run more than one instance of dnsmasq or have dnsmasq co-exist with another DNS server. This is not possible in default mode, but it does work in bind-interfaces mode, providing than _all_ instances of dnsmasq are in bind-interfaces mode, and that they listen on a disjoint set of interfaces. Therefore, to allow multiple dnsmasq instances libvirt's private dnsmasq instance is started in bind-interfaces mode: that forces one of the dnsmasq instances to do bind-interfaces. Many of the Linux distibution dnsmasq packages have now implemented an /etc/dnsmasq.d directory where configuration fragments can be dropped. Their libvirt packages are putting a file there which contains a bind-interfaces command, so that the "system" dnsmasq is automatically forced into the same mode, and the two can co-exist. This works, sort-of, but there some disadvantages. Installing libvirt drops the configuration change for the system dnsmasq, but the packages frequently don't restart the system daemon, so that things transiently fail until everything has rebooted. Much worse, the system dnsmasq is forced into bind-interfaces mode and then service to transient interfaces (usb, ad-hoc wifi) no longer works, or, because those interfaces are mentioned in the dnsmasq configuration, dnsmasq now fails at start-up when the interfaces don't exist. I don't think there is a solution to this problem wholly within dnsmasq: the nature of the BSD-sockets universe means that there never be two instances of dnsmasq without bind-interfaces, and it's, at best, difficult to get round the limitations of that WRT transient network interfaces. There is, an alternative solution, which would involve changes to both dnsmasq, libvirt and their packaging. Hence this mail. My proposal is to get rid of the necessity for two dnsmasq instances. Libvirt should check for the existance of a "system" dnsmasq and, if the system daemon exists, libvirt should drop the required configuration into /etc/dnsmasq.d and then restart it. If the system daemon is not installed or enabled, libvirt can start a private instance as now. The difficulty with this scheme is that libvirt needs to create some configuration which enables the services it needs on the virtual network without disturbing, or being disturbed by, whatever configuration exists for the system daemon. That's not currently possible, but it can be made possible. I'm assuming that libvirt needs to provide a set of IP address / MAC address mappings, and range of IP addresses on a virtual network. It needs DHCP and DNS service on the virtual network. The dhcp-host IP/MAC mappings are a non-problem: they will be ignored for any other subnet where the IP addresses don't fit, and any other dhcp-hosts in the system configuration will be similarly ignored for DHCP on the virtual network subnet. The dhcp-range is more of a problem. Service to particular networks in dnsmasq is controlled by interface=<interface name"> lines in the configuration. If there are none of these, service is provided to all interfaces. If they exist, service is limited to the interfaces specified. The existence of any dhcp-range line in dnsmasq's configuration enables the DHCP server for any subnet unless explicitly limited to particular interfaces. So a default dnsmasq installation, (with no interface=<interface>) which provides DNS everywhere but DHCP nowhere would be turned into one which provided DHCP on every interface by libvirt adding a dhcp-range. Since there wouldn't be a suitable DHCP range for most subnets, this would only result in logged errors, but it is still not good. Worse, there's no good answer to the question 'should libvirt include interface=virt0"' in the configuration it supplies? If it does, then the "enable DHCP on all interfaces" problem is solved, but a default system configuration with no interface declaration is transformed from one which provides DNS everywhere to one which provides DNS only to the virtual interface. If libvirt doesn't provide "interface=virt0" and the system configuration includes interface declarations, then there will be no DNS or DHCP service to the virtual network. To solve this, I propose to add an optional interface name to the dhcp-range declaration. The semantics of this would be rather odd, but solve the problem perfectly. 1) for DHCP, if any other dhcp-range exists _without_ an interface name, them the interface name is ignored and and things behave as before, otherwise DHCP is only provided to interfaces mentioned in dhcp-range declarations. 2) for DNS, if there are no interface declarations, things work as before. If there are interface declarations, the interfaces mentioned in dhcp-ranges are added to the set which get DNS service. With these rules, it should be possible for libvirt to drop eg dhcp-range=interface:virt0,192.168.0.1,192.168.0.240 into the configuration of the system dnsmasq and get DHCP and DNS service for virt0, irrespective of any other configuration in the system dnsmasq, and doing so shouldn't affect the services supplied elsewhere. The code in libvirt to make this work looks like this: echo dhcp-range=interface:virt0,<ip range> >>/etc/dnsmasq.d/libvirt if <system dnsmasq is not installed or not enabled> dnsmasq --interface=virt0\ --bind-interfaces --conf-file=/etc/dnsmasq.d/libvirt else /etc/init.d/dnsmasq restart (The --bind-interfaces in the private-dnsmasq instance keeps dnsmasq from clashing with other nameservers eg BIND which may be running.) The system dnsmasq package has to ensure that /etc/dnsmasq.d is read for configuration fragments, and the dnsmasq package and the libvirt package will have to co-operate to manage transitions between private and system dnsmasq mode caused by package installation or removal. Does that make sense? It's a long and involved explanation to come to cold. I fear I may have over-simplified what libvirt is doing with dnsmasq, in which case please enlighten me and I'll modify my scheme to take that into account. If this looks good I can easily have the necessary dnsmasq changes in the next release. Cheers, Simon.

3 8

[libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input
by Jim Meyering 01 Mar '10

01 Mar '10

Clang warned about the potential NULL-dereference via the STREQ/strcmp below. models[i] could be NULL. Even "models" could be NULL, and the "allowed = ..." test makes that appear to be deliberately allowed. The change below is the least invasive and cleanest I could come up with, assuming there is no need to diagnose (e.g., by returning -1) the condition of a NULL models[i] pointer. >From 623ac546a93f3e5b554647b05524b5041a642530 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Sun, 28 Feb 2010 13:34:06 +0100 Subject: [PATCH] x86Decode: avoid NULL-dereference upon questionable input * src/cpu/cpu_x86.c (x86Decode): Don't dereference NULL when passed a NULL "models" pointer, or when passed a nonzero "nmodels" value and a corresponding NULL models[i]. --- src/cpu/cpu_x86.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 2194c32..b263629 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -957,89 +957,89 @@ x86Compare(virCPUDefPtr host, virCPUDefPtr cpu) { return x86Compute(host, cpu, NULL); } static virCPUCompareResult x86GuestData(virCPUDefPtr host, virCPUDefPtr guest, union cpuData **data) { return x86Compute(host, guest, data); } static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, const char **models, unsigned int nmodels) { int ret = -1; struct x86_map *map; const struct x86_model *candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; struct cpuX86cpuid *cpuid; int i; if (data == NULL || (map = x86LoadMap()) == NULL) return -1; candidate = map->models; while (candidate != NULL) { bool allowed = (models == NULL); for (i = 0; i < candidate->ncpuid; i++) { cpuid = x86DataCpuid(data, candidate->cpuid[i].function); if (cpuid == NULL || !x86cpuidMatchMasked(cpuid, candidate->cpuid + i)) goto next; } for (i = 0; i < nmodels; i++) { - if (STREQ(models[i], candidate->name)) { + if (models && models[i] && STREQ(models[i], candidate->name)) { allowed = true; break; } } if (!allowed) { VIR_DEBUG("CPU model %s not allowed by hypervisor; ignoring", candidate->name); goto next; } if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) goto out; if (cpuModel == NULL || cpuModel->nfeatures > cpuCandidate->nfeatures) { virCPUDefFree(cpuModel); cpuModel = cpuCandidate; } else virCPUDefFree(cpuCandidate); next: candidate = candidate->next; } if (cpuModel == NULL) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot find suitable CPU model for given data")); goto out; } cpu->model = cpuModel->model; cpu->nfeatures = cpuModel->nfeatures; cpu->features = cpuModel->features; VIR_FREE(cpuModel); ret = 0; out: x86MapFree(map); virCPUDefFree(cpuModel); return ret; } -- 1.7.0.1.414.g89213d

2 2

[libvirt] Problems with attach-device/detach-device using libvirt 0.7.6
by Rolf Eike Beer 01 Mar '10

01 Mar '10

Hi, first a short summary of what the situation is: I'm running qemu 0.12.2, libvirt 0.7.6 on a E5520 based machine with kernel 2.6.33. I also tried 2.6.32.7 as I was using that before and still had the old kernel around, but kernel version doesn't matter AFAICT. The host is a gentoo AMD64 installation, the client is SuSE 11.0 (both 32 and 64 bit tested). This system is our machine to run automated PCIe device tests on, so the idea is to move PCIe devices from the host to different vms, run some testcases there and then move them away again. This has worked before (I'm not absolutely sure, but I would bet a bit of money that it was libvirt 0.7.5). So, what I'm going to do is simply virsh # attach-device suse11.0-AMD64 dev-pci-AD1-CL.xml Device attached successfully virsh # detach-device suse11.0-AMD64 dev-pci-AD1-CL.xml Device detached successfully This is just was I expect and what I now get with the two patches attached. The original situation was somewhat different (i.e. broken): -first I got complains in the log stating: tried to create id "(null)" twice for "device" Well, adding two devices with the same id is bad, and if that id is a NULL pointer anyway IMHO it's better to not pass the parameter at all. That's what the "null-pci-id" patch does. -next, detaching failed. I found out that the command sent to qemu was " pci_del pci_addr=fc67add0:7f41:fe38b65a" which is obviously bogus. After some hours of digging through the command stack up and down I found that it was the memcpy addressed in the "pci_memcpy" patch. When the device is attached qemu just replies with an empty string. Afterwards from the guestAddr the address of the PCI device in the client is copied back to the info structure. Too bad that this structure was never initialized to anything useful (at least not for me). When I kick out the memcpy() everything works fine. Configuration files of one device and one guest are attached, the others look similar. Note: when the client PCI id is passed to qemu anyway it would be cool if that could be specified in the device XML. For USB devices, too. Eike Please keep me CC'ed as I'm not on the list.

2 3

[libvirt] [PATCH] openvzDomainDefineCmd: remove useless increment
by Jim Meyering 01 Mar '10

01 Mar '10

Another dead store. Refreshingly shallow. The function below starts like this: static int openvzDomainDefineCmd(virConnectPtr conn, const char *args[], int maxarg, virDomainDefPtr vmdef) { int narg; int veid; int max_veid; and there are no intervening uses of "max_veid". >From 7333ff9e0cf83d243dba752d293d9007ded5e9f8 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Mar 2010 21:38:06 +0100 Subject: [PATCH] openvzDomainDefineCmd: remove useless increment * src/openvz/openvz_driver.c (openvzDomainDefineCmd): Remove useless increment of "max_veid". --- src/openvz/openvz_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 5057b81..4673b6c 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -137,65 +137,65 @@ openvzDomainDefineCmd(virConnectPtr conn, narg = 0; ADD_ARG_LIT(VZCTL); ADD_ARG_LIT("--quiet"); ADD_ARG_LIT("create"); if ((fp = popen(VZLIST " -a -ovpsid -H 2>/dev/null", "r")) == NULL) { openvzError(NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); return -1; } max_veid = 0; while (!feof(fp)) { if (fscanf(fp, "%d\n", &veid) != 1) { if (feof(fp)) break; openvzError(NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse vzlist output")); goto cleanup; } if (veid > max_veid) { max_veid = veid; } } fclose(fp); if (max_veid == 0) { max_veid = 100; } else { max_veid++; } - sprintf(str_id, "%d", max_veid++); + sprintf(str_id, "%d", max_veid); ADD_ARG_LIT(str_id); ADD_ARG_LIT("--name"); ADD_ARG_LIT(vmdef->name); if (vmdef->nfss == 1 && vmdef->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) { ADD_ARG_LIT("--ostemplate"); ADD_ARG_LIT(vmdef->fss[0]->src); } #if 0 if ((vmdef->profile && *(vmdef->profile))) { ADD_ARG_LIT("--config"); ADD_ARG_LIT(vmdef->profile); } #endif ADD_ARG(NULL); return 0; no_memory: openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not put argument to %s"), VZCTL); return -1; cleanup: fclose(fp); return -1; #undef ADD_ARG #undef ADD_ARG_LIT } -- 1.7.0.1.414.g89213d

2 1

[libvirt] [PATCH] phypUUIDTable_Push: do not corrupt output stream
by Jim Meyering 01 Mar '10

01 Mar '10

This started with clang's report of the two dead stores. With the increment and decrement by "sent" being performed outside of the intended loop, the two assignments had no net effect. BTW, I presume these hard-coded names, from the same function, are somehow temporary and/or debugging-related: char remote_file[] = "/home/hscroot/libvirt_uuid_table"; char local_file[] = "./uuid_table"; >From a1104c4d1a95b10cab19bacf91ed073769f1d2d7 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Mar 2010 21:26:59 +0100 Subject: [PATCH] phypUUIDTable_Push: do not corrupt output stream upon partial write * src/phyp/phyp_driver.c (phypUUIDTable_Push): Move incr/decr of ptr/nread into the loop where those variables are used. Also, remove "exit" label and just-preceding "goto". --- src/phyp/phyp_driver.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 1e8ed30..3fc5a0c 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1982,54 +1982,51 @@ phypUUIDTable_Push(virConnectPtr conn) do { nread = fread(buffer, 1, sizeof(buffer), fd); if (nread <= 0) { if (feof(fd)) { /* end of file */ break; } else { VIR_ERROR("Failed to read from '%s'", local_file); goto err; } } ptr = buffer; sent = 0; do { /* write the same data over and over, until error or completion */ rc = libssh2_channel_write(channel, ptr, nread); if (LIBSSH2_ERROR_EAGAIN == rc) { /* must loop around */ continue; } else if (rc > 0) { /* rc indicates how many bytes were written this time */ sent += rc; } + ptr += sent; + nread -= sent; } while (rc > 0 && sent < nread); - ptr += sent; - nread -= sent; } while (1); - goto exit; - - exit: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); libssh2_channel_wait_closed(channel); libssh2_channel_free(channel); channel = NULL; } return 0; err: if (channel) { libssh2_channel_send_eof(channel); libssh2_channel_wait_eof(channel); libssh2_channel_wait_closed(channel); libssh2_channel_free(channel); channel = NULL; } return -1; } int phypUUIDTable_Pull(virConnectPtr conn) { -- 1.7.0.1.414.g89213d

2 1

[libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang
by Jim Meyering 01 Mar '10

01 Mar '10

Here's a case in which using an assertion appears to be the only way to tell clang that "client" really is non-NULL at that point. I'm sure clang's analyzers will eventually improve, and hence avoid this sort of false positive, so have marked this with a FIXME comment, to help ensure we eventually remove this otherwise unnecessary assertion. >From ba26bad7aec8713ded0fd3300e951eac3673cc72 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Mar 2010 15:19:48 +0100 Subject: [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang * daemon/libvirtd.c (qemudWorker): Assert. --- daemon/libvirtd.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9bdbecb..a357914 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -24,34 +24,35 @@ #include <config.h> #include <sys/types.h> #include <sys/wait.h> #include <sys/stat.h> #include <unistd.h> #include <fcntl.h> #include <limits.h> #include <sys/socket.h> #include <sys/un.h> #include <sys/poll.h> #include <netinet/in.h> #include <netinet/tcp.h> #include <netdb.h> #include <stdlib.h> #include <pwd.h> #include <stdio.h> +#include <assert.h> #include <stdarg.h> #include <syslog.h> #include <string.h> #include <errno.h> #include <getopt.h> #include <fnmatch.h> #include <grp.h> #include <signal.h> #include <netdb.h> #include "libvirt_internal.h" #include "virterror_internal.h" #define VIR_FROM_THIS VIR_FROM_QEMU #include "libvirtd.h" #include "dispatch.h" @@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data) virMutexLock(&server->lock); while (((client = qemudPendingJob(server)) == NULL) && !worker->quitRequest) { if (virCondWait(&server->job, &server->lock) < 0) { virMutexUnlock(&server->lock); return NULL; } } if (worker->quitRequest) { if (client) virMutexUnlock(&client->lock); virMutexUnlock(&server->lock); return NULL; } worker->processingCall = 1; virMutexUnlock(&server->lock); + /* Tell clang we know what we're doing. + FIXME: remove when clang improves. */ + assert (client); + /* We own a locked client now... */ client->refs++; /* Remove our message from dispatch queue while we use it */ msg = qemudClientMessageQueueServe(&client->dx); /* This function drops the lock during dispatch, * and re-acquires it before returning */ if (remoteDispatchClientRequest (server, client, msg) < 0) { VIR_FREE(msg); qemudDispatchClientFailure(client); client->refs--; virMutexUnlock(&client->lock); continue; } client->refs--; -- 1.7.0.1.414.g89213d

5 7

[libvirt] [RFC PATCH] build: consistently use C99 varargs macros
by Eric Blake 01 Mar '10

01 Mar '10

Prior to this patch, there was an inconsistent mix between GNU and C99. For consistency, and potential portability to other compilers, stick with the C99 vararg macro syntax. * src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU vararg macro syntax. * src/conf/domain_conf.c (virDomainReportError): Likewise. * src/conf/domain_event.c (eventReportError): Likewise. * src/conf/interface_conf.c (virInterfaceReportError): Likewise. * src/conf/network_conf.c (virNetworkReportError): Likewise. * src/conf/node_device_conf.h (virNodeDeviceReportError): Likewise. * src/conf/secret_conf.h (virSecretReportError): Likewise. * src/conf/storage_conf.h (virStorageReportError): Likewise. --- Any objections to expanding this into a full-blown patch series? This touches 8 files; I counted 43 more files that could be altered, using: $ git grep -l 'define.*[a-zA-Z]\.\.\.' src/conf/cpu_conf.c | 4 ++-- src/conf/domain_conf.c | 4 ++-- src/conf/domain_event.c | 5 +++-- src/conf/interface_conf.c | 4 ++-- src/conf/network_conf.c | 6 +++--- src/conf/node_device_conf.h | 5 +++-- src/conf/secret_conf.h | 6 +++--- src/conf/storage_conf.h | 6 +++--- 8 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index ed83188..612e376 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -31,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_CPU -#define virCPUReportError(code, fmt...) \ +#define virCPUReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST, "minimum", diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4420445..ea84863 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -232,9 +232,9 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, "private", "bridge") -#define virDomainReportError(code, fmt...) \ +#define virDomainReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) #ifndef PROXY diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b474b1c..e791cab 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1,6 +1,7 @@ /* * domain_event.c: domain event queue processing helpers * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -30,9 +31,9 @@ #define VIR_FROM_THIS VIR_FROM_NONE -#define eventReportError(conn, code, fmt...) \ +#define eventReportError(conn, code, ...) \ virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) /** diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a0d2dfa..bc9e958 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -45,9 +45,9 @@ static int virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDefPtr def, int level); -#define virInterfaceReportError(code, fmt...) \ +#define virInterfaceReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 6d3c3c0..39ebd62 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, "none", "nat", "route" ) -#define virNetworkReportError(code, fmt...) \ +#define virNetworkReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets, const unsigned char *uuid) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 3a5432c..cbaad9b 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -1,6 +1,7 @@ /* * node_device_conf.h: config handling for node devices * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -217,9 +218,9 @@ struct _virDeviceMonitorState { void *privateData; /* driver-specific private data */ }; -#define virNodeDeviceReportError(code, fmt...) \ +#define virNodeDeviceReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NODEDEV, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap); diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index da00920..afcf8f1 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -1,7 +1,7 @@ /* * secret_conf.h: internal <secret> XML handling API * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,9 +26,9 @@ #include "internal.h" #include "util.h" -#define virSecretReportError(code, fmt...) \ +#define virSecretReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_SECRET, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) VIR_ENUM_DECL(virSecretUsageType) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index c643984..5441c8e 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -1,7 +1,7 @@ /* * storage_conf.h: config handling for storage driver * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -318,9 +318,9 @@ static inline int virStoragePoolObjIsActive(virStoragePoolObjPtr pool) { return pool->active; } -#define virStorageReportError(code, fmt...) \ +#define virStorageReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_STORAGE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) + __FUNCTION__, __LINE__, __VA_ARGS__) int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, const char *configDir, -- 1.6.6.1

4 4

[libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs
by Stefan Berger 01 Mar '10

01 Mar '10

Hello! The following is a proposal to introduce network traffic filtering capabilities for the network traffic originating from and destined to virtual machines. Libvirt's capabilities will be extended to allow users to describe what traffic filtering rules are to be applied on a virtual machine (using XML) and libvirt then creates the appropriate ebtables and iptables rules and applies those on the host when the virtual machine starts up, resumes after suspension or resumes on a new host after migration. libvirt tears down the traffic filtering rules when the virtual machine shuts down, suspends, or a VM is migrated to another host. It will also be possible to modify the filtering rules while a virtual machine is running. In this architecture we apply the firewall rules on the outside of the virtual machines on the Linux host and make use of the fact that virtual machines can be configured by libvirt to have their network traffic pass through the host and the host exposes (tap) 'backend' interfaces on which the firewall rules can be applied. The application of the firewall rules is optional and those who do not want to introduce a raw network throughput performance hit on their system due to the evaluation of every packet passing through the filtering chains, do not have to use these capabilities. An initial implementation would be done for libvirt's Qemu support. As stated above, the application of firewall rules on virtual machines will require some form of XML description that lets the user choose what type of filtering is to be performed. In effect the user needs to be able to tell libvirt which ebtables and iptables rules to generate so that the appropriate filtering can be done. We realize that ebtables and iptables have lots of capabilities but think that we need to restrict which capabilities can actually be 'reached' through this XML. The following proposal is based on an XML as defined by the DMTF ( http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf, slide 10) with extensions for processing of ARP packets.It gives control over the evaluation of Ethernet (MAC) frames, ARP packet data and IP header information. A (draft) XML skeleton in this case could look as follows: <FilterEntry> <Action>DROP|ACCEPT</Action>  <TrafficType>incoming [to VM]|outgoing [from VM]</TrafficType> <Hdr8021Filter> <HdrSrcMACAddr8021> </HdrSrcMACAddr8021> <HdrSrcMACMask8021> </HdrSrcMACMask8021> <HdrDestMACAddr8021> </HdrDestMACAddr8021> <HdrDestMACMask8021> </HdrDestMACMask8021> <HdrProtocolID8021> numeric and/or string? </HdrProtocolID8021> <HdrPriorityValue8021></HdrPriorityValue8021> <HdrVLANID8021> </HdrVLANID8021> </Hdr8021Filter> <ARPFilter> <HWType> </HWType> <ProtocolType> </ProtocolType> <OPCode> </OPCode> <SrcMACAddr> </SrcMACAddr> <SrcIPAddr> </SrcIPAddr> <DestMACAddr> </DestMACAddr> <DestIPAddr> </DestIPAddr> </ARPFilter> <IPHeadersFilter> <HdrIPVersion> </HdrIPVersion> <HdrSrcAddress> </HdrSrcAddress> <HdrSrcMask> </HdrSrcMask> <HdrDestAddress> </HdrDestAddress> <HdrDestMask> </HdrDestMask> <HdrProtocolID> numeric and/or string? </HdrProtocolID> <HdrSrcPortStart> </HdrSrcPortStart> <HdrSrcPortEnd> </HdrSrcPortEnd> <HdrDestPortStart> </HdrDestPortStart> <HdrDestPortEnd> </HdrDestPortEnd> <HdrDSCP> </HdrDSCP> </IPHeadersFilter> </FilterEntry> Since the ebtables and iptables command cannot accept all possible parameters at the same time, only a certain subset of the above parameters may be set for any given filter command. Higher level application writers will likely use a library that lets them choose which features they would want to have enforced, such as no-broadcast or no-multicast, enforcement of MAC spoofing prevention or ARP poisoning prevention, which then generates this lower level XML rules in the appropriate order of the rules. Further, we will introduce filter pools where a collection of the above filter rules can be stored and referenced to by virtual machines' individual interface. A reference to such a filter pool entry will be given in the interface description and may look as in the following draft XML: <interface type='bridge'> <source bridge='virbr0'/> <script path='vif-bridge'/> <firewall> <reference profile='generic-layer2' ip_address='10.0.0.1'/> <reference profile='VM-specific-layer3'/> </firewall> </interface> The above XML has one reference to a generic layer2 filter template XML that should be usable by multiple virtual machines but would need to be customized with interface-specific parameters. In this case, the IP address of the interface is explicitly provided in order to make the filter XML template a concrete XML for the particular interface. The MAC address of the interface is not explicitly provided since it will already have been randomly generated by libvirt at the point when this layer2 filter XML needs to be converted into concrete ebtables commands/rules. We still need to determine how the format of a template should look like, though an idea would be to indicate a placeholder for a VM's MAC address using THIS_MAC and similarly for the IP address with THIS_IP as placeholder. Further, we would introduce the management of filter 'pools'. Considering existing functionality in libvirt and CLI commands for similar type of management functionality, the following CLI commands would be added: virsh nwfilter-create <file> virsh nwfilter-destroy <profile name> virsh nwfilter-list <options> virsh nwfilter-dumpxml <profile name> virsh nwfilter-update <filename> (performs an update on an existing profile replacing all rules) possibly also: virsh nwfilter-edit <profile name> virsh nwfilter-create-from <profile name> Please let us know what you think of this proposal. Regards Stefan, Gerhard and Vivek

8 25

[libvirt] [PATCH] qemudNetworkIfaceConnect: remove dead store
by Jim Meyering 01 Mar '10

01 Mar '10

clang reported this "dead store". As you can see, just a few lines before there's an identical assignment, so this may have been an editing error (didn't dig). As clang reports, that final assignment is a dead store, since there is no following use of "errobj". >From 92edbcf2ffa0dad25251723cd0749be2397a2321 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Mar 2010 15:59:42 +0100 Subject: [PATCH] qemudNetworkIfaceConnect: remove dead store * src/qemu/qemu_conf.c (qemudNetworkIfaceConnect): Remove extraneous virSaveLastError call, whose result was unused. --- src/qemu/qemu_conf.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1f8a9c3..eee6222 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1520,11 +1520,10 @@ qemudNetworkIfaceConnect(virConnectPtr conn, errobj = virSaveLastError(); virNetworkFree(network); virSetError(errobj); virFreeError(errobj); - errobj = virSaveLastError(); if (fail) return -1; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (!(brname = strdup(net->data.bridge.brname))) { -- 1.7.0.1.414.g89213d

2 2