Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by David Miller
From: Mike Frysinger <vapier(a)gentoo.org>
Date: Thu, 17 Jan 2013 23:14:31 -0500
> the kernel already exports many types with a __kernel_ prefix. i changed the
> kernel headers in Gentoo for a few releases (2.6.28 - 2.6.34) to do the same
> thing to pretty much all the networking headers. a few packages broke, but
> the number was low, and trivial to fix (a sed would do it most of the time).
>
> it's also trivial for userland packages to detect that they need to do this
> sort of thing in a local header by using linux/version.h and a set of defines
> to redirect the new structure name back to the old one.
>
> would be a lot cleaner to just break it and be done.
We are not at liberty to break something that has legitimately
compiled successfully for two decades.
Your __kernel_ prefix idea breaks compilation just as equally.
One thing you certainly don't have to be is happy about this header
file situation, but you cannot use that dissatisfaction as
justification to make the situation worse by breaking the build for
people outside of the world you directly control.
11 years, 11 months
Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by David Miller
From: Mike Frysinger <vapier(a)gentoo.org>
Date: Wed, 16 Jan 2013 12:04:56 -0500
> certainly true, but the current expectation is that you don't mix your ABIs.
> if you're programming with the C library API, then use the C library headers.
> if you're banging directly on the kernel, then use the kernel headers. not
> saying it's a perfect solution, but it works for the vast majority of use
> cases.
This isn't how real life works.
GLIBC itself brings in some of the kernel headers, as do various library
headers for libraries other than glibc.
So you can get these conflicting headers included indirectly, and it is
of no fault of any of the various parties involved.
We have to make them work when included at the same time somehow, and
this is totally unavoidable.
11 years, 11 months
[libvirt] [PATCH] build: fix build on BSD
by Eric Blake
A build on FreeBSD failed with:
util/virportallocator.c:108: error: storage size of 'addr' isn't known
util/virportallocator.c:123: error: 'INADDR_ANY' undeclared (first use in this function)
It turns out that while POSIX allows sockaddr_in to leak in through
other headers (the way Linux does it), conforming applications are
required to get it through netinet/in.h.
* src/util/virportallocator.c: Include header for struct
sockaddr_in.
---
Pushing under the build-breaker rule.
src/util/virportallocator.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 35f2157..590bb57 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -23,6 +23,7 @@
#include <sys/socket.h>
#include <arpa/inet.h>
+#include <netinet/in.h>
#include "viralloc.h"
#include "virbitmap.h"
--
1.8.0.2
11 years, 11 months
[libvirt] [PATCH v2.1 RESEND 00/11] Rework storage migration
by Michal Privoznik
This patch set re-implements migration with storage for enough new qemu.
Currently, you can migrate a domain to a host without need for shared storage.
This is done by setting 'blk' or 'inc' attribute (representing
VIR_MIGRATE_NON_SHARED_DISK and VIR_MIGRATE_NON_SHARED_INC flags respectively)
of 'migrate' monitor command. However, the qemu implementation is
buggy and applications are advised to switch to new impementation
which, moreover, offers some nice features, like migrating only explicitly
specified disks.
The new functionality is controlled via 'nbd-server-*' and 'drive-mirror'
commands. The flow is meant to look like this:
1) User invokes libvirt's migrate functionality.
2) libvirt checks that no block jobs are active on the source.
3) libvirt starts the destination QEMU and sets up the NBD server using the
nbd-server-start and nbd-server-add commands.
4) libvirt starts drive-mirror with a destination pointing to the remote NBD
server, for example nbd:host:port:exportname=diskname (where diskname is the
-drive id specified on the destination).
5) once all mirroring jobs reach steady state, libvirt invokes the migrate
command.
6) once migration completed, libvirt invokes the nbd-server-stop command on the
destination QEMU.
If we just skip the 2nd step and there is an active block-job, qemu will fail in
step 4. No big deal.
Since we try to NOT break migration and keep things compatible, this feature is
enabled iff both sides support it. Since there's obvious need for some data
transfer between src and dst, I've put it into qemuCookieMigration:
1) src -> dest: (QEMU_MIGRATION_PHASE_BEGIN3 -> QEMU_MIGRATION_PHASE_PREPARE)
<nbd>
<disk size='17179869184'/>
</nbd>
Hey destination, I know how to use this cool new feature. Moreover,
these are the disks I'll send you. Each one of them is X bytes big.
It's one of the prerequisite - the file (disk->src) on dst exists and has
at least the same size as on dst.
2) dst -> src: (QEMU_MIGRATION_PHASE_PREPARE -> QEMU_MIGRATION_PHASE_PERFORM3)
<nbd port='X'/>
Okay, I (destination) support this feature as well. I've created all
files as you (src) told me to and you can start rolling data. I am listening
on port X.
3) src -> dst: (QEMU_MIGRATION_PHASE_PERFORM3 -> QEMU_MIGRATION_PHASE_FINISH3)
<nbd port='-1'/>
Migration completed, destination, you may shut the NBD server down.
If either src or dst doesn't support NBD, it is not used and whole process fall
backs to old implementation.
diff to v1:
-Eric's and Daniel's suggestions worked in. To point out the bigger ones:
don't do NBD style when TUNNELLED requested, added 'b:writable' to
'nbd-server-add'
-drop '/qemu-migration/nbd/disk/@src' attribute from migration cookie.
As pointed out by Jirka, disk->src can be changed during migration (e.g. by
migration hook or by passed xml). So I've tried (as suggested on the list)
passing disk alias. However, since qemu hasn't been started on destination yet,
the aliases hasn't been generated yet. So we have to rely on ordering
completely.
diff to v2:
-rebase to reflect changes made by offline migration patch
-send initial nbd cookie only if needed
diff to previous v2.1:
-rebase to current HEAD
The patches 1,3 and 5 has been ACKed already.
Maybe, I'll need to drop 7/11 patch completely, once Dan's patches [1] are in.
1: https://www.redhat.com/archives/libvir-list/2013-January/msg00742.html
Michal Privoznik (11):
qemu: Introduce NBD_SERVER capability
Introduce NBD migration cookie
qemu: Introduce nbd-server-start command
qemu: Introduce nbd-server-add command
qemu: Introduce nbd-server-stop command
qemu_migration: Introduce qemuMigrationStartNBDServer
qemu_migration: Move port allocation to a separate func
qemu_migration: Implement qemuMigrationStartNBDServer()
qemu_migration: Implement qemuMigrationDriveMirror
qemu_migration: Check size prerequisites
qemu_migration: Stop NBD server at Finish phase
src/qemu/qemu_capabilities.c | 4 +-
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_migration.c | 624 ++++++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_monitor.c | 63 +++++
src/qemu/qemu_monitor.h | 7 +
src/qemu/qemu_monitor_json.c | 95 +++++++
src/qemu/qemu_monitor_json.h | 7 +
7 files changed, 768 insertions(+), 33 deletions(-)
--
1.8.0.2
11 years, 11 months
[libvirt] [RFC] Data in the <topology> element in the capabilities XML
by Peter Krempa
Hi everybody,
a while ago there was a discussion about changing the data that is
returned in the <topology> sub-element:
<capabilities>
<host>
<cpu>
<arch>x86_64</arch>
<model>SandyBridge</model>
<vendor>Intel</vendor>
<topology sockets='1' cores='2' threads='2'/>
The data provided here is as of today taken from the nodeinfo detection
code and thus is really wrong when the fallback mechanisms are used.
To get a useful count, the user has to multiply the data by the number
of NUMA nodes in the host. With the fallback detection code used for
nodeinfo the NUMA node count used to get the CPU count should be 1
instead of the actual number.
As Jiri proposed, I think we should change this output to separate
detection code that will not take into account NUMA nodes for this
output and will rather provide data as the "lspci" command does.
This change will make the data provided by the element standalone and
also usable in guest XMLs to mirror host's topology.
The meaning of the attributes of that element isn't really documented
anywhere, so as an additional precaution we should document that.
( http://libvirt.org/formatcaps.html )
For some additional background, please refer to the original discussion
here:
https://www.redhat.com/archives/libvir-list/2012-March/msg01123.html
Thanks in advance for your comments and suggestions.
Peter
11 years, 11 months
[libvirt] [PATCH v2] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The QEMU driver default max port is 65535, but it then increments
this by 1 to 65536. This maps to 0 in an unsigned short :-( This
was apparently done so that for() loops could use "< max" instead
of "<= max". Remove this insanity and just make the loop do the
right thing.
---
src/qemu/qemu_conf.c | 4 ----
src/util/virportallocator.c | 6 +++---
tests/virportallocatortest.c | 2 +-
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 680e8fb..b21392e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
filename, QEMU_REMOTE_PORT_MAX);
goto cleanup;
}
- /* increasing the value by 1 makes all the loops going through
- the bitmap (i = remotePortMin; i < remotePortMax; i++), work as
- expected. */
- driver->remotePortMax++;
if (driver->remotePortMin > driver->remotePortMax) {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 033aee4..d80347a 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start,
pa->start = start;
pa->end = end;
- if (!(pa->bitmap = virBitmapNew(end-start))) {
+ if (!(pa->bitmap = virBitmapNew((end-start)+1))) {
virReportOOMError();
virObjectUnref(pa);
return NULL;
@@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
*port = 0;
virObjectLock(pa);
- for (i = pa->start ; i < pa->end && !*port; i++) {
+ for (i = pa->start ; i <= pa->end && !*port; i++) {
int reuse = 1;
struct sockaddr_in addr;
bool used = false;
@@ -168,7 +168,7 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa,
virObjectLock(pa);
if (port < pa->start ||
- port >= pa->end) {
+ port > pa->end) {
virReportInvalidArg(port, "port %d must be in range (%d, %d)",
port, pa->start, pa->end);
goto cleanup;
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 93577d7..3f6edcc 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -59,7 +59,7 @@ int bind(int sockfd ATTRIBUTE_UNUSED,
static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
{
- virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5910);
+ virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5909);
int ret = -1;
unsigned short p1, p2, p3, p4, p5, p6, p7;
--
1.8.0.2
11 years, 11 months
[libvirt] [PATCH] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The QEMU driver default max port is 65535, but it then increments
this by 1 to 65536. This maps to 0 in an unsigned short :-( This
was apparently done so that for() loops could use "< max" instead
of "<= max". Remove this insanity and just make the loop do the
right thing.
---
src/qemu/qemu_conf.c | 4 ----
src/util/virportallocator.c | 4 ++--
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 680e8fb..b21392e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
filename, QEMU_REMOTE_PORT_MAX);
goto cleanup;
}
- /* increasing the value by 1 makes all the loops going through
- the bitmap (i = remotePortMin; i < remotePortMax; i++), work as
- expected. */
- driver->remotePortMax++;
if (driver->remotePortMin > driver->remotePortMax) {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 033aee4..5d07dd0 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start,
pa->start = start;
pa->end = end;
- if (!(pa->bitmap = virBitmapNew(end-start))) {
+ if (!(pa->bitmap = virBitmapNew((end-start)+1))) {
virReportOOMError();
virObjectUnref(pa);
return NULL;
@@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
*port = 0;
virObjectLock(pa);
- for (i = pa->start ; i < pa->end && !*port; i++) {
+ for (i = pa->start ; i <= pa->end && !*port; i++) {
int reuse = 1;
struct sockaddr_in addr;
bool used = false;
--
1.8.0.2
11 years, 11 months
[libvirt] [PATCH 0/5] Resolve REVERSE_INULL errors found by Coverity
by John Ferlan
This patch will resolve a set of "REVERSE_INULL (CWE-476):" errors found
by Coverity.
John Ferlan (5):
network: Resolve some issues around vlan copying
xen: Remove extraneous checks in xenStoreGetDomainInfo()
interface: Need to check ifacedef->mac not just ifacedef after
strdup()
xen: Need to check validity of domain->conn before deref privateData
openvz: Need to check 'vm' first before dereferencing 'def'
src/interface/interface_backend_udev.c | 2 +-
src/network/bridge_driver.c | 16 +++++++++++++---
src/openvz/openvz_driver.c | 2 +-
src/xen/xen_hypervisor.c | 3 ++-
src/xen/xs_internal.c | 2 +-
5 files changed, 18 insertions(+), 7 deletions(-)
--
1.7.11.7
11 years, 11 months
[libvirt] [PATCH] network: use bandwidth from portgroup when appropriate
by Laine Stump
The bandwidth plug and unplug functions were assuming that an
interface's bandwidth setting was always specified directly in the
domain's <interface> definition, but that's not necessarily true - it
could have been obtained from a <portgroup> definition in the network
definition. This patch fixes those functions to use
virDomainNetGetActualBandwidth(), which gets the bandwidth pointer
from iface->data.network.actual if it exists, otherwise returns
iface->bandwidth.
---
src/network/bridge_driver.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c3cb63d..6f3c839 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4518,7 +4518,7 @@ networkCheckBandwidth(virNetworkObjPtr net,
{
int ret = -1;
virNetDevBandwidthPtr netBand = net->def->bandwidth;
- virNetDevBandwidthPtr ifaceBand = iface->bandwidth;
+ virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
unsigned long long tmp_floor_sum = net->floor_sum;
unsigned long long tmp_new_rate = 0;
char ifmac[VIR_MAC_STRING_BUFLEN];
@@ -4597,6 +4597,7 @@ networkPlugBandwidth(virNetworkObjPtr net,
unsigned long long new_rate = 0;
ssize_t class_id = 0;
char ifmac[VIR_MAC_STRING_BUFLEN];
+ virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) {
/* helper reported error */
@@ -4625,11 +4626,8 @@ networkPlugBandwidth(virNetworkObjPtr net,
goto cleanup;
}
- plug_ret = virNetDevBandwidthPlug(net->def->bridge,
- net->def->bandwidth,
- &iface->mac,
- iface->bandwidth,
- class_id);
+ plug_ret = virNetDevBandwidthPlug(net->def->bridge, net->def->bandwidth,
+ &iface->mac, ifaceBand, class_id);
if (plug_ret < 0) {
ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id));
goto cleanup;
@@ -4638,11 +4636,11 @@ networkPlugBandwidth(virNetworkObjPtr net,
/* QoS was set, generate new class ID */
iface->data.network.actual->class_id = class_id;
/* update sum of 'floor'-s of attached NICs */
- net->floor_sum += iface->bandwidth->in->floor;
+ net->floor_sum += ifaceBand->in->floor;
/* update status file */
if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) {
ignore_value(virBitmapClearBit(net->class_id, class_id));
- net->floor_sum -= iface->bandwidth->in->floor;
+ net->floor_sum -= ifaceBand->in->floor;
iface->data.network.actual->class_id = 0;
ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id));
goto cleanup;
@@ -4666,6 +4664,7 @@ networkUnplugBandwidth(virNetworkObjPtr net,
{
int ret = 0;
unsigned long long new_rate;
+ virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface);
if (iface->data.network.actual &&
iface->data.network.actual->class_id) {
@@ -4680,13 +4679,13 @@ networkUnplugBandwidth(virNetworkObjPtr net,
if (ret < 0)
goto cleanup;
/* update sum of 'floor'-s of attached NICs */
- net->floor_sum -= iface->bandwidth->in->floor;
+ net->floor_sum -= ifaceBand->in->floor;
/* return class ID */
ignore_value(virBitmapClearBit(net->class_id,
iface->data.network.actual->class_id));
/* update status file */
if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) {
- net->floor_sum += iface->bandwidth->in->floor;
+ net->floor_sum += ifaceBand->in->floor;
ignore_value(virBitmapSetBit(net->class_id,
iface->data.network.actual->class_id));
goto cleanup;
--
1.7.11.7
11 years, 11 months
[libvirt] [PATCH] conf: don't fail to parse <boot> element when parsing a single device
by Laine Stump
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=895294
The symptom was that attempts to modify a network device using
virDomainUpdateDeviceFlags() would fail if the original device had a
<boot> element (e.g. "<boot order='1'/>"), even if the updated device
had the same <boot> element. Instead, the following error would be logged:
cannot modify network device boot index setting
It's true that it's not possible to change boot order (internally
known as bootIndex) of a live device; qemuDomainChangeNet checks for
that, but the problem was that the information it was checking was
incorrect.
Explanation:
When a complete domain is parsed, a global (to the domain) "bootMap"
is passed down to the parse for each device; the bootMap is used to
make sure that devices don't have conflicting settings for their boot
orders.
When a single device is parsed by itself (as in the case of
virDomainUpdateDeviceFlags), there is no global bootMap that would be
appropriate to send, so NULL is sent instead. However, although the
lowest level function that parses just the boot order *does* simply
skip the sanity check in that case, the next higher level
"virDomainDeviceInfoParseXML" function refuses to call down to the
lower "virDomainDeviceBootParseXML" if bootMap is NULL. So, the boot
order is never set in the "new" device object, and when it is compared
to the original (which does have a boot order), they don't match.
The fix is to patch virDomainDeviceInfoParseXML to not care about
bootMap, and just always call virDomainDeviceInfoBootParseXML whenever
there is a <boot> element. When we are only parsing a single device,
we don't care whether or not any specified boot order is consistent
with the rest of the domain; we will always do this check later (in
the current case, we do it by verifying that the net bootIndex exactly
matches the old bootIndex).
---
src/conf/domain_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 06b9c25..e4babaf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1,7 +1,7 @@
/*
* domain_conf.c: domain XML processing
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -2624,7 +2624,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
} else if (master == NULL &&
xmlStrEqual(cur->name, BAD_CAST "master")) {
master = cur;
- } else if (boot == NULL && bootMap &&
+ } else if (boot == NULL &&
(flags & VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) &&
xmlStrEqual(cur->name, BAD_CAST "boot")) {
boot = cur;
--
1.7.11.7
11 years, 11 months
