- Devel - libvirt List Archives

[libvirt] [PATCH v2] qemu: Use ShutdownVMDaemon for all startup cleanup paths
by Cole Robinson 20 May '10

20 May '10

The current cleanup: in StartVMDaemon path is a poor duplication. qemuShutdownVMDaemon can handle teardown for inactive VMs, so let's use it. v2: Remove old abort: label, only use cleanup: Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/qemu/qemu_driver.c | 37 ++++++++++--------------------------- 1 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d387e69..3823dbd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3516,29 +3516,29 @@ static int qemudStartVMDaemon(virConnectPtr conn, DEBUG0("Waiting for monitor to show up"); if (qemudWaitForMonitor(driver, vm, pos) < 0) - goto abort; + goto cleanup; DEBUG0("Detecting VCPU PIDs"); if (qemuDetectVcpuPIDs(driver, vm) < 0) - goto abort; + goto cleanup; DEBUG0("Setting any required VM passwords"); if (qemuInitPasswords(conn, driver, vm, qemuCmdFlags) < 0) - goto abort; + goto cleanup; /* If we have -device, then addresses are assigned explicitly. * If not, then we have to detect dynamic ones here */ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { DEBUG0("Determining domain device PCI addresses"); if (qemuInitPCIAddresses(driver, vm) < 0) - goto abort; + goto cleanup; } DEBUG0("Setting initial memory amount"); qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorSetBalloon(priv->mon, vm->def->memory) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); - goto abort; + goto cleanup; } if (migrateFrom == NULL) { @@ -3549,7 +3549,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); qemuDomainObjExitMonitorWithDriver(driver, vm); - goto abort; + goto cleanup; } } qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -3557,7 +3557,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, DEBUG0("Writing domain status to disk"); if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) - goto abort; + goto cleanup; if (logfile != -1) close(logfile); @@ -3565,26 +3565,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, return 0; cleanup: - /* We jump here if we failed to start the VM for any reason - * XXX investigate if we can kill this block and safely call - * qemudShutdownVMDaemon even though no PID is running */ - qemuDomainReAttachHostDevices(driver, vm->def); - - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityAllLabel) - driver->securityDriver->domainRestoreSecurityAllLabel(vm, 0); - if (driver->securityDriver && - driver->securityDriver->domainReleaseSecurityLabel) - driver->securityDriver->domainReleaseSecurityLabel(vm); - qemuRemoveCgroup(driver, vm, 1); - if (logfile != -1) - close(logfile); - vm->def->id = -1; - return -1; - -abort: - /* We jump here if we failed to initialize the now running VM - * killing it off and pretend we never started it */ + /* We jump here if we failed to start the VM for any reason, or + * if we failed to initialize the now running VM. kill it off and + * pretend we never started it */ qemudShutdownVMDaemon(driver, vm, 0); if (logfile != -1) -- 1.6.6.1

2 2

[libvirt] [PATCH] daemon: A few initscript corrections
by Cole Robinson 20 May '10

20 May '10

Fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=565238 - Avahi service is called 'avahi-daemon' - chkconfig descriptions must use \ for line continuations Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- daemon/libvirtd.init.in | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in index d4dc98b..809433e 100644 --- a/daemon/libvirtd.init.in +++ b/daemon/libvirtd.init.in @@ -9,7 +9,7 @@ # Should-Start: $named # Should-Start: xend # Should-Start: hal -# Should-Start: avahi +# Should-Start: avahi-daemon # Required-Stop: $network messagebus # Should-Stop: $named # Default-Start: 3 4 5 @@ -24,8 +24,8 @@ # libvirtd: guest and virtual network management daemon # # chkconfig: 345 97 03 -# description: This is a daemon for managing guest instances -# and libvirt virtual networks +# description: This is a daemon for managing guest instances \ +# and libvirt virtual networks \ # See http://libvirt.org # # processname: libvirtd -- 1.6.6.1

2 2

Re: [libvirt] libvirt and Ruby
by Chris Lalancette 20 May '10

20 May '10

On 05/20/2010 01:53 PM, Jaromír Červenka wrote: > Hi Chris, > > thank you for your answer. That will be great! :) I decided that I > will write this deamon with Ruby and with old ruby bindings at least, > so this is good news for me :) > > My friend from suse (Michal Hrusecky) did two patches till now - one > is for virNodeGetFreememory a the second implements > virConnectGetLibVersion. He sent these patches to David Lutterkort, > but without answer. Do you want them? > > Sorry for my EN :/ (please leave the list CC'ed; other people might be interested in the status of the Ruby bindings). Your English is just fine :). If you could send those patches to the libvirt list (and CC me), that would be great; I also have commit access to the ruby repository, so assuming they are OK I can add them. Thanks, -- Chris Lalancette

1 0

[libvirt] [PATCH] qemu: Genericize tapfds/ntapfds
by Alex Williamson 20 May '10

20 May '10

There doesn't seem to be anything specific to tap devices for this array of file descriptors which need to stay open of the guest to use. Rename then for others to make use of. Signed-off-by: Alex Williamson <alex.williamson(a)redhat.com> --- src/qemu/qemu_conf.c | 28 ++++++++++++++-------------- src/qemu/qemu_driver.c | 18 +++++++++--------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3e334dc..d7bc798 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3422,8 +3422,8 @@ int qemudBuildCommandLine(virConnectPtr conn, unsigned long long qemuCmdFlags, const char ***retargv, const char ***retenv, - int **tapfds, - int *ntapfds, + int **vmfds, + int *nvmfds, const char *migrateFrom, virDomainSnapshotObjPtr current_snapshot) { @@ -4116,7 +4116,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (tapfd < 0) goto error; - if (VIR_REALLOC_N(*tapfds, (*ntapfds)+1) < 0) { + if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { virNWFilterTearNWFilter(net); close(tapfd); goto no_memory; @@ -4124,7 +4124,7 @@ int qemudBuildCommandLine(virConnectPtr conn, last_good_net = i; - (*tapfds)[(*ntapfds)++] = tapfd; + (*vmfds)[(*nvmfds)++] = tapfd; if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; @@ -4136,7 +4136,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (tapfd < 0) goto error; - if (VIR_REALLOC_N(*tapfds, (*ntapfds)+1) < 0) { + if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { virNWFilterTearNWFilter(net); close(tapfd); goto no_memory; @@ -4144,7 +4144,7 @@ int qemudBuildCommandLine(virConnectPtr conn, last_good_net = i; - (*tapfds)[(*ntapfds)++] = tapfd; + (*vmfds)[(*nvmfds)++] = tapfd; if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; @@ -4157,12 +4157,12 @@ int qemudBuildCommandLine(virConnectPtr conn, network device */ int vhostfd = qemudOpenVhostNet(net, qemuCmdFlags); if (vhostfd >= 0) { - if (VIR_REALLOC_N(*tapfds, (*ntapfds)+1) < 0) { + if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { close(vhostfd); goto no_memory; } - (*tapfds)[(*ntapfds)++] = vhostfd; + (*vmfds)[(*nvmfds)++] = vhostfd; if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", vhostfd) >= sizeof(vhostfd_name)) goto no_memory; @@ -4653,12 +4653,12 @@ int qemudBuildCommandLine(virConnectPtr conn, error: for (i = 0; i <= last_good_net; i++) virNWFilterTearNWFilter(def->nets[i]); - if (tapfds && - *tapfds) { - for (i = 0; i < *ntapfds; i++) - close((*tapfds)[i]); - VIR_FREE(*tapfds); - *ntapfds = 0; + if (vmfds && + *vmfds) { + for (i = 0; i < *nvmfds; i++) + close((*vmfds)[i]); + VIR_FREE(*vmfds); + *nvmfds = 0; } if (qargv) { for (i = 0 ; i < qargc ; i++) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 65ca117..dd5bd24 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3253,8 +3253,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, const char **progenv = NULL; int i, ret; struct stat sb; - int *tapfds = NULL; - int ntapfds = 0; + int *vmfds = NULL; + int nvmfds = 0; unsigned long long qemuCmdFlags; fd_set keepfd; const char *emulator; @@ -3411,7 +3411,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, vm->def->id = driver->nextvmid++; if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON, qemuCmdFlags, &argv, &progenv, - &tapfds, &ntapfds, migrateFrom, + &vmfds, &nvmfds, migrateFrom, vm->current_snapshot) < 0) goto cleanup; @@ -3462,8 +3462,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, VIR_WARN("Unable to seek to end of logfile: %s", virStrerror(errno, ebuf, sizeof ebuf)); - for (i = 0 ; i < ntapfds ; i++) - FD_SET(tapfds[i], &keepfd); + for (i = 0 ; i < nvmfds ; i++) + FD_SET(vmfds[i], &keepfd); ret = virExecDaemonize(argv, progenv, &keepfd, &child, stdin_fd, &logfile, &logfile, @@ -3504,11 +3504,11 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (ret == -1) /* The VM failed to start; tear filters before taps */ virNWFilterTearVMNWFilters(vm); - if (tapfds) { - for (i = 0 ; i < ntapfds ; i++) { - close(tapfds[i]); + if (vmfds) { + for (i = 0 ; i < nvmfds ; i++) { + close(vmfds[i]); } - VIR_FREE(tapfds); + VIR_FREE(vmfds); } if (ret == -1) /* The VM failed to start */

4 3

[libvirt] [PATCH] qemu driver: fix version check typos
by Chris Wright 20 May '10

20 May '10

Looks like some cut'n paste error to me. Signed-off-by: Chris Wright <chrisw(a)redhat.com> --- src/qemu/qemu_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3e334dc..6d2ac6e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1291,13 +1291,13 @@ int qemudParseHelpStr(const char *qemu, ++p; minor = virParseNumber(&p); - if (major == -1 || *p != '.') + if (minor == -1 || *p != '.') goto fail; ++p; micro = virParseNumber(&p); - if (major == -1) + if (micro == -1) goto fail; SKIP_BLANKS(p);

4 8

[libvirt] How to enable vhost for virtIO NIC?
by Huang, Zhiteng 20 May '10

20 May '10

Hi gurus on the list, I'm trying to bring up a Linux with virtio vhost backend. My system is running on kernel 2.6.34-rc7, libvirt 0.8.0. Two questions regarding to vhost: 1) XML format for vhost I didn't find any documents on libvirt.org description the XML or QMU argument format for vhost. According to http://www.linux-kvm.org/page/VhostNet#vhost-net_driver_projects, I tried *domxml-from-native* to convert following qemu argument to XML qemu-system-x86_64 -m 1G disk-c.qcow2 \ -net nic,model=virtio,netdev=foo \ -netdev tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on But the output clearly ignore the network. <domain type='qemu'> <name>unnamed</name> <uuid>9cc877c1-7ac2-463d-3d3f-fa8f8918fe23</uuid> <memory>1048576</memory> <currentMemory>1048576</currentMemory> <vcpu>1</vcpu> <os> <type arch='x86_64'>hvm</type> </os> <features> <acpi/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>qemu-system-x86_64</emulator> <input type='mouse' bus='ps2'/> <graphics type='sdl'/> <video> <model type='cirrus' vram='9216' heads='1'/> </video> </devices> </domain> What's the problem here? 2) Virtio driver. It's said that vhost require guest kernel version > 2.6.31 because vhost requires MSI-X. How about frontend of virtio driver? Say I'm running a Windows 2008 (with MSI-X support) + virtio NIC, is it necessary to upgrade frontend driver too? Regards, HUANG, Zhiteng

3 4

[libvirt] [PATCH 0/4] qemu: Unify cleanup paths in qemuStartVMDaemon
by Cole Robinson 20 May '10

20 May '10

Many of the cleanup paths in qemuStartVMDaemon have the potential of squashing a legitimate startup error message. qemuShutdownVMDaemon already properly handles this, so let's unify all the cleanup to use it. This series fixes a few teardown issues, and changes ShutdownVMDaemon to not reject non-running VMs. This could be an issue if some caller was depending on this functionality, but I couldn't find one. Cole Robinson (4): qemu: Properly cleanup in security startup error path qemu: Remove explicit VNC XML cleanup qemu: Don't deny ShutdownVMDaemon for non-running VMs qemu: Use ShutdownVMDaemon for all startup cleanup paths src/qemu/qemu_driver.c | 31 +++++-------------------------- 1 files changed, 5 insertions(+), 26 deletions(-)

2 10

[libvirt] [PATCH 1/2] maint: avoid unwanted newline at end of diagnostic
by Jim Meyering 20 May '10

20 May '10

While I was in the virtual vicinity, Jiri mentioned the possibility of detecting one more minor formatting problem: the stray newline at the end of a diagnostic. Here are two corrections and the heuristic that caught them (and that will prevent introduction of some new ones -- but not if the end of the message is more than 2 lines after the function name). Currently there are no false positives. >From 122b1e31fb33c092a53802b56a0f2f5586c95bd5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 20 May 2010 11:12:17 +0200 Subject: [PATCH 1/2] maint: remove unwanted newline at end of diagnostic * src/xen/xend_internal.c (xenDaemonDomainDefineXML): Remove \n. * src/network/bridge_driver.c (networkAddMasqueradingIptablesRules): Likewise. --- src/network/bridge_driver.c | 2 +- src/xen/xend_internal.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3b9b4f4..5d7ef19 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -642,7 +642,7 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, network->def->network, network->def->forwardDev))) { virReportSystemError(err, - _("failed to add iptables rule to enable masquerading to '%s'\n"), + _("failed to add iptables rule to enable masquerading to '%s'"), network->def->forwardDev ? network->def->forwardDev : NULL); goto masqerr3; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index ea5addd..a203a8d 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -4731,7 +4731,7 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { VIR_FREE(sexpr); if (ret != 0) { virXendError(VIR_ERR_XEN_CALL, - _("Failed to create inactive domain %s\n"), def->name); + _("Failed to create inactive domain %s"), def->name); goto error; } -- 1.7.1.259.g3aef8 >From 532672f3a0c53213609dab49df2e7efcaf0eb594 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 20 May 2010 11:13:41 +0200 Subject: [PATCH 2/2] maint: prohibit newline at end of diagnostic * cfg.mk (sc_prohibit_newline_at_end_of_diagnostic): New rule. Idea proposed by Jiri Denemark. --- cfg.mk | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/cfg.mk b/cfg.mk index 2909e3e..b024c75 100644 --- a/cfg.mk +++ b/cfg.mk @@ -391,6 +391,20 @@ sc_libvirt_unmarked_diagnostics: { echo '$(ME): found unmarked diagnostic(s)' 1>&2; \ exit 1; } || : +# Like the above, but prohibit a newline at the end of a diagnostic. +# This is subject to false positives partly because it naively looks for +# `\n"', which may not be the end of the string, and also because it takes +# two lines of context (the -A2) after the line with the function name. +# FIXME: this rule might benefit from a separate function list, in case +# there are functions to which this one applies but that do not get marked +# diagnostics. +sc_prohibit_newline_at_end_of_diagnostic: + @grep -A2 -nE \ + '\<$(func_re) *\(' $$($(VC_LIST_EXCEPT)) \ + | grep '\\n"' \ + && { echo '$(ME): newline at end of message(s)' 1>&2; \ + exit 1; } || : + # Disallow trailing blank lines. sc_prohibit_trailing_blank_lines: @$(VC_LIST_EXCEPT) | xargs perl -ln -0777 -e \ -- 1.7.1.259.g3aef8

3 4

[libvirt] lxc_controller: don't ignore failed "accept"
by Jim Meyering 20 May '10

20 May '10

Initially I proposed a similar patch here: http://thread.gmane.org/gmane.comp.emulators.libvirt/20607 That thread languished, and Eric proposed a nearly identical patch: http://thread.gmane.org/gmane.comp.emulators.libvirt/21630 Here's a patch that should address the initial objection. Now, my only question is about which errno values to ignore. Obviously, EMFILE, EFAULT, etc. must not be ignored. I doubt EINTR and EBADF should be ignored, but haven't tried to prove the case. >From 6dcd15b0aa594f3d89e810e0a92aa75dab22903e Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 20 May 2010 14:30:36 +0200 Subject: [PATCH] lxc_controller.c: don't ignore failed "accept" * src/lxc/lxc_controller.c (ignorable_epoll_accept_errno): New function. (lxcControllerMain): Handle a failed accept carefully: most errno values indicate legitimate failure and must be fatal. However, ignore a special case: that in which an incoming client quits between the poll() indicating its presence, and our accept() which is trying to process it. --- src/lxc/lxc_controller.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 6b64372..75a45e9 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -269,6 +269,17 @@ typedef struct _lxcTtyForwardFd_t { int active; } lxcTtyForwardFd_t; +/* Return true if it is ok to ignore an accept-after-epoll syscall + that fails with the specified errno value. Else false. */ +static bool +ignorable_epoll_accept_errno(int erratum) +{ + return (errnum == EINVAL + || errnum == ECONNABORTED + || errnum == EAGAIN + || errnum == EWOULDBLOCK); +} + /** * lxcControllerMain * @monitor: server socket fd to accept client requests @@ -350,6 +361,18 @@ static int lxcControllerMain(int monitor, if (numEvents > 0) { if (epollEvent.data.fd == monitor) { int fd = accept(monitor, NULL, 0); + if (fd < 0) { + /* First reflex may be simply to declare accept failure + to be a fatal error. However, accept may fail when + a client quits between the above epoll_wait and here. + That case is not fatal, but rather to be expected, + if not common, so ignore it. */ + if (ignorable_epoll_accept_errno(errno)) + continue; + virReportSystemError(errno, "%s", + _("accept(monitor,...) failed")); + goto cleanup; + } if (client != -1) { /* Already connected, so kick new one out */ close(fd); continue; -- 1.7.1.262.g5ef3d

2 2

[libvirt] libvirt and Ruby
by Jaromír Červenka 20 May '10

20 May '10

Hello everyone, is there any possibility that the Ruby bindings for libvirt will be updated? I am currently createing a web interface for libivirt. This web app is written in Ruby on Rails. Now I am in phase when I have to create a deamon (which will do a "background jobs" and will comunicate with libvirt) and I would like to write this deamon in Ruby also, but interface for libvirt is old and not actual. The other possibility is write this deamon in C# (mono), 'cause from C# is very easy to call external functions from your library. Thank you for answer, -- Greetings, Jaromír Červenka Official openSUSE community member Web: http://www.cervajz.com/ Jabber: cervajz(a)cervajz.com MSN: jara.cervenka(a)seznam.cz Tel.: +420 607 592 687 Alt. e-mails: jaromir.cervenka(a)opensuse.org, jaromir.cervenka(a)speel.cz

2 1