[libvirt] [PATCH] Fix safezero()
by Jiri Denemark
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
15 years, 1 month
[libvirt] [PATCH] lxc: detect socket failure
by Eric Blake
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
15 years, 1 month
[libvirt] [PATCH] build: silence coverity warning in node_device
by Eric Blake
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
15 years, 1 month
[libvirt] [PATCH] Fix USB/PCI device address aliases in QEMU hotplug driver
by Daniel P. Berrange
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
15 years, 1 month
[libvirt] [PATCH 0/6] Add more guest clock mods
by Daniel P. Berrange
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.
15 years, 1 month
[libvirt] [PATCH 0/6] SCSI-Disk-Hotremove
by Wolfgang Mauerer
[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(-)
15 years, 1 month
[libvirt] libvirt/dnsmasq integration.
by Simon Kelley
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.
15 years, 1 month
[libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input
by Jim Meyering
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
15 years, 1 month
[libvirt] Problems with attach-device/detach-device using libvirt 0.7.6
by Rolf Eike Beer
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.
15 years, 1 month
[libvirt] [PATCH] openvzDomainDefineCmd: remove useless increment
by Jim Meyering
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
15 years, 1 month
