- Devel - libvirt List Archives

[libvirt] [PATCH 0/6] Add more options for clock sync
by Daniel P. Berrange 02 Feb '10

02 Feb '10

This series adds two new options for clock synchronization The two current options are: <clock offset='utc'/> <clock offset='localtime'/> This introduces a way to set a timezone for localtime <clock offset='localtime' 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'/> It is open to debate whether we need to distinguish this last one via the extra string 'variable', or just add 'adjustment' as an optional attribute for the existing term 'utc'. Opinions welcome

1 6

[libvirt] [PATCH] xen_hypervisor.c: avoid NULL deref for NULL domain argument
by Jim Meyering 02 Feb '10

02 Feb '10

When "domain" is NULL, don't deref NULL. Instead, just return -1, as in many other functions in this file, and as this function did up until a month ago. An alternative (taken 3 times in this file) is to do this: virXenErrorFunc (NULL, VIR_ERR_INTERNAL_ERROR, __FUNCTION__, "domain or conn is NULL", 0); return -1; I could go either way. >From 177556167775b806a29bcb1af7ba4294d1909912 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 26 Jan 2010 20:17:07 +0100 Subject: [PATCH] xen_hypervisor.c: avoid NULL deref for NULL domain argument * src/xen/xen_hypervisor.c (xenHypervisorGetVcpus): Don't attempt to diagnose an unlikely NULL-domain or NULL-domain->conn error. --- src/xen/xen_hypervisor.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 6d8accc..0257be2 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1,7 +1,7 @@ /* * xen_internal.c: direct access to Xen hypervisor level * - * Copyright (C) 2005, 2006, 2007, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2005-2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -3475,11 +3475,8 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, virVcpuInfoPtr ipt; int nbinfo, i; - if (domain == NULL || domain->conn == NULL) { - virXenErrorFunc (domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__, - "invalid argument", 0); + if (domain == NULL || domain->conn == NULL) return -1; - } priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || (domain->id < 0) || -- 1.7.0.rc0.140.gfbe7

2 4

[libvirt] libvirt: python bindings
by Sharadha Prabhakar (3P) 02 Feb '10

02 Feb '10

Hi, I'm writing XenServer APIs for libvirt and trying to test it on virt-Manager. I need to build the libvirt python bindings for that. I tried this for libvirt. ./configure -with-xenapi -with-python make make install I suppose this is supposed to build a .libs directory with libvirtmod.so in the ~/libvirt/python/ But I don't seem to have this at all after build. It was a clean build with no errors. I need libvirtmod.so to link with virt-manager. I'm using libvirt version 0.7.4 Can anyone help me with this. Am I missing something here Thanks in Advance, -SP

4 4

[libvirt] [PATCH] Ensure QEMU DAC security driver is activated at all times
by Daniel P. Berrange 02 Feb '10

02 Feb '10

If the primary security driver (SELinux/AppArmour) was disabled then the secondary QEMU DAC security driver was also disabled. This is mistaken, because the latter must be active at all times * src/qemu/qemu_driver.c: Ensure DAC driver is always active --- src/qemu/qemu_driver.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 16e9b56..a9313e7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -897,26 +897,28 @@ qemudSecurityInit(struct qemud_driver *qemud_drv) int ret; virSecurityDriverPtr security_drv; + qemuSecurityStackedSetDriver(qemud_drv); + qemuSecurityDACSetDriver(qemud_drv); + ret = virSecurityDriverStartup(&security_drv, qemud_drv->securityDriverName); if (ret == -1) { VIR_ERROR0(_("Failed to start security driver")); return -1; } - /* No security driver wanted to be enabled: just return */ + + /* No primary security driver wanted to be enabled: just setup + * the DAC driver on its own */ if (ret == -2) { + qemud_drv->securityDriver = &qemuDACSecurityDriver; VIR_INFO0(_("No security driver available")); - return 0; + } else { + qemud_drv->securityPrimaryDriver = security_drv; + qemud_drv->securitySecondaryDriver = &qemuDACSecurityDriver; + qemud_drv->securityDriver = &qemuStackedSecurityDriver; + VIR_INFO("Initialized security driver %s", security_drv->name); } - qemuSecurityStackedSetDriver(qemud_drv); - qemuSecurityDACSetDriver(qemud_drv); - - qemud_drv->securityPrimaryDriver = security_drv; - qemud_drv->securitySecondaryDriver = &qemuDACSecurityDriver; - qemud_drv->securityDriver = &qemuStackedSecurityDriver; - - VIR_INFO("Initialized security driver %s", security_drv->name); return 0; } -- 1.6.6

2 1

[libvirt] [PATCH 00/13] Fix device hotplug with QEMU and -device
by Daniel P. Berrange 02 Feb '10

02 Feb '10

The current libvirt code enables use of -device when launching QEMU but forgot to update the equivalent code for hotpluging devices to an already running QEMU. This series implements the latter, doing three core things - Assign a unique PCI slot to the new device - Assign a unique device alias to the new device - Call the device_add function for attaching the device As a minor flaw in the whole plan, it turns out the new -netdev code in QEMU 0.12 does not support hotplugging at all. So this series also switching network setup back to using the old VLAN based syntax. Hopefully we can re-introduce use of -netdev with QEMU 0.13, since it gives important performance benefits.

3 45

[libvirt] [PATCH] udev: Don't let strtoul parse USB busnum and devnum as octal
by Matthias Bolte 02 Feb '10

02 Feb '10

udevGetUintProperty was called with base set to 0 for busnum and devnum. With base 0 strtoul parses the number as octal if it start with a 0. But busnum and devnum are decimal and udev returns them padded with leading zeros. So strtoul parses them as octal. This works for certain decimal values like 001-007, but fails for values like 008. Change udevProcessUSBDevice to use base 10 for busnum and devnum. --- src/node_device/node_device_udev.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 734f3f7..0c0f29a 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -500,14 +500,14 @@ static int udevProcessUSBDevice(struct udev_device *device, if (udevGetUintProperty(device, "BUSNUM", &data->usb_dev.bus, - 0) == PROPERTY_ERROR) { + 10) == PROPERTY_ERROR) { goto out; } if (udevGetUintProperty(device, "DEVNUM", &data->usb_dev.device, - 0) == PROPERTY_ERROR) { + 10) == PROPERTY_ERROR) { goto out; } -- 1.6.3.3

2 3

[libvirt] [PATCH] libvirtd.c: avoid closing a negative socket file descriptor
by Jim Meyering 02 Feb '10

02 Feb '10

A lot like the last one... >From e32826290b960790b0ec4f50b195f424fa42348f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 2 Feb 2010 11:27:25 +0100 Subject: [PATCH] libvirtd.c: avoid closing a negative socket file descriptor * daemon/libvirtd.c (qemudListenUnix): Close socket only if non-negative. --- daemon/libvirtd.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3070dfc..f7df0c6 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1,7 +1,7 @@ /* * libvirtd.c: daemon start of day, guest process & i/o management * - * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006-2010, 2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -582,7 +582,7 @@ static int qemudListenUnix(struct qemud_server *server, return 0; cleanup: - if (sock->fd) + if (0 <= sock->fd) close(sock->fd); VIR_FREE(sock); return -1; -- 1.7.0.rc1.149.g0b0b7

3 2

[libvirt] [PATCH] lxc_controller.c: don't ignore failed "accept"
by Jim Meyering 02 Feb '10

02 Feb '10

coverity complained (rightly) about the risk of closing a negative file descriptor. However, the real problem was the missing test for a failed "accept" call. I'm not 100% sure that a failed accept call deserves to provoke a "goto cleanup", but doing that is consistent with what the nearby code does upon epoll_ctl failure. >From 8bfd81f0a8a9cb3fd9b575e9c2f5ab9969a2910f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 2 Feb 2010 11:55:19 +0100 Subject: [PATCH] lxc_controller.c: don't ignore failed "accept" * src/lxc/lxc_controller.c (lxcControllerMain): A failed accept could lead to passing a negative file descriptor to various other functions, which would in turn report EBADF, rather that whatever error prompted the initial failure. --- src/lxc/lxc_controller.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 6304815..682f874 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -349,6 +349,11 @@ static int lxcControllerMain(int monitor, if (numEvents > 0) { if (epollEvent.data.fd == monitor) { int fd = accept(monitor, NULL, 0); + if (fd < 0) { + virReportSystemError(NULL, errno, "%s", + _("accept(monitor,...) failed")); + goto cleanup; + } if (client != -1) { /* Already connected, so kick new one out */ close(fd); continue; -- 1.7.0.rc1.149.g0b0b7

2 2

[libvirt] [PATCH] storage_backend.c: avoid closing a negative file descriptor
by Jim Meyering 02 Feb '10

02 Feb '10

This close(fd) is reachable with an "fd" of -1 via the "goto cleanup" just before &fd is first set. While closing(-1) is not a big problem, it is a failing syscall, and would show up on an strace audit, not to mention the coverity and maybe-clang warnings. >From c69369c445be53f12ec09a176fd477b9ff16bbff Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 2 Feb 2010 11:11:49 +0100 Subject: [PATCH] storage_backend.c: avoid closing a negative file descriptor * src/storage/storage_backend.c (virStorageBackendRunProgRegex): Don't close a negative (read-only) file descriptor. --- src/storage/storage_backend.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index bc656f2..84eb8aa 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1,7 +1,7 @@ /* * storage_backend.c: internal storage driver backend contract * - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2010 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1326,8 +1326,10 @@ virStorageBackendRunProgRegex(virConnectPtr conn, if (list) fclose(list); - else - close(fd); + else { + if (0 <= fd) + close(fd); + } while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR); -- 1.7.0.rc1.149.g0b0b7

3 3

[libvirt] [PATCH] avoid a probable EINVAL from lseek
by Jim Meyering 02 Feb '10

02 Feb '10

In src/qemu/qemu_driver.c, coverity reports this: Event negative_return_fn: Called negative-returning function "lseek(logfile, 0L, 2)" Event var_assign: NEGATIVE return value of "lseek" assigned to signed variable "pos" At conditional (1): "(pos = lseek(logfile, 0L, 2)) < 0" taking true path 2877 if ((pos = lseek(logfile, 0, SEEK_END)) < 0) 2878 VIR_WARN(_("Unable to seek to end of logfile: %s"), 2879 virStrerror(errno, ebuf, sizeof ebuf)); since later in that same function, a negative "pos" may be used like this: Event negative_returns: Tracked variable "pos" was passed to a negative sink. [details] 2930 if (qemudWaitForMonitor(conn, driver, vm, pos) < 0) 2931 goto abort; 2932 which is a legitimate problem, since qemudWaitForMonitor calls qemudLogReadFD, which calls lseek with that same "pos" value: Event neg_sink_parm_call: Parameter "pos" passed to negative sink "lseek" 560 if (lseek(fd, pos, SEEK_SET) < 0) { 561 virReportSystemError(conn, errno, 562 _("Unable to seek to %lld in %s"), 563 (long long) pos, logfile); 564 close(fd); 565 } One approach is to detect the negative offset in that final bit of code and skip the lseek: >From 0ef617935462c314ed0b44bcaa3dd5bf58ccbc1b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 1 Feb 2010 22:17:44 +0100 Subject: [PATCH] avoid a probable EINVAL from lseek * src/qemu/qemu_driver.c (qemudLogReadFD): Don't pass a negative offset (from a preceding failed attempt to seek to EOF) to this use of lseek. --- src/qemu/qemu_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 22593bf..676a27b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -558,8 +558,8 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char* name, off_t p close(fd); return -1; } - if (lseek(fd, pos, SEEK_SET) < 0) { - virReportSystemError(conn, errno, + if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) { + virReportSystemError(conn, pos < 0 ? 0 : errno, _("Unable to seek to %lld in %s"), (long long) pos, logfile); close(fd); -- 1.7.0.rc1.149.g0b0b7

3 7