- Devel - libvirt List Archives

[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