[libvirt] [RFC][PATCH 2/2] Automatically recreate macvtap interface and reconnect to qemu
This patch implements a thread for each VM that iterates through the
list of macvtap interfaces, attempting to recreate each one and
reconnect it to qemu. See the PATCH 0 email for discussion.
---
Index: libvirt-0.7.6/src/qemu/qemu_driver.c
===
--- libvirt-0.7.6.orig/src/qemu/qemu_driver.c
+++ libvirt-0.7.6/src/qemu/qemu_driver.c
@@ -76,6 +76,7 @@
#include "xml.h"
#include "cpu/cpu.h"
#include "macvtap.h"
+#include "threads.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -96,6 +97,8 @@ struct _qemuDomainObjPrivate {
int *vcpupids;
qemuDomainPCIAddressSetPtr pciaddrs;
+
+pthread_t reconnectNetBackendsThread;
};
static int qemudShutdown(void);
@@ -143,6 +146,8 @@ static int qemudDomainDisconnectNetBacke
virDomainObjPtr vm,
virDomainNetDefPtr net);
+static void *qemudDomainReconnectNetBackends(void *opaque);
+
static struct qemud_driver *qemu_driver = NULL;
@@ -2794,6 +2799,10 @@ static int qemudStartVMDaemon(virConnect
if (virDomainSaveStatus(conn, driver->caps, driver->stateDir, vm) < 0)
goto abort;
+if (pthread_create(&priv->reconnectNetBackendsThread, NULL,
+ qemudDomainReconnectNetBackends, vm->def->uuid))
+goto abort;
+
return 0;
cleanup:
@@ -5627,7 +5636,7 @@ static int qemudDomainConnectNetBackend(
unsigned int qemuCmdFlags)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
-char *tapfd_name = NULL;
+char *hostnet_name = NULL;
int tapfd = -1;
char *netstr = NULL;
int ret = -1;
@@ -5664,27 +5673,30 @@ static int qemudDomainConnectNetBackend(
if (tapfd < 0)
return -1;
-if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0) {
+if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) {
virReportOOMError(conn);
close(tapfd);
return -1;
}
if (!(netstr = qemuBuildHostNetStr(conn, net, ' ',
- vlan, tapfd_name))) {
-VIR_FREE(tapfd_name);
+ vlan, hostnet_name))) {
+VIR_FREE(hostnet_name);
close(tapfd);
return -1;
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-if (qemuMonitorSendFileHandle(priv->mon, tapfd_name, tapfd) < 0)
+if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
+VIR_INFO0("Did not remove existing host network");
+
+if (qemuMonitorSendFileHandle(priv->mon, hostnet_name, tapfd) < 0)
goto out;
if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) {
-if (qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0)
-VIR_WARN(_("Failed to close tapfd with '%s'"), tapfd_name);
+if (qemuMonitorCloseFileHandle(priv->mon, hostnet_name) < 0)
+VIR_WARN(_("Failed to close tapfd with '%s'"), hostnet_name);
goto out;
}
@@ -5693,7 +5705,7 @@ static int qemudDomainConnectNetBackend(
out:
qemuDomainObjExitMonitorWithDriver(driver, vm);
VIR_FREE(netstr);
-VIR_FREE(tapfd_name);
+VIR_FREE(hostnet_name);
close(tapfd);
return ret;
}
@@ -8549,6 +8561,72 @@ out:
return ret;
}
+static void *qemudDomainReconnectNetBackends(void *opaque)
+{
+virConnectPtr conn = virConnectOpen("qemu:///system");
+struct qemud_driver *driver = conn->privateData;
+const unsigned char *uuid = opaque;
+
+while (1) {
+virDomainObjPtr vm;
+unsigned int qemuCmdFlags;
+int i;
+
+usleep(100);
+
+VIR_ERROR(_("qemuDomainReconnectNetBackends (%p %p)"),
+ conn, driver);
+
+qemuDriverLock(driver);
+vm = virDomainFindByUUID(&driver->domains, uuid);
+if (!vm) {
+VIR_ERROR0("qemuDomainReconnectNetBackends done");
+qemuDriverUnlock(driver);
+break;
+}
+
+VIR_ERROR(_("qemuDomainReconnectNetBackends (%s)"),
+ vm->def->name);
+
+if (qemudExtractVersionInfo(vm->def->emulator,
+NULL,
+&qemuCmdFlags) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm))
+goto endjob;
+
+for (i = 0 ; i < vm->def->nnets ; i++) {
+virDomainNetDefPtr net = vm->def->nets[i];
+
+VIR_ERROR(_("qemuDomainReconnectNetBackends (%s)"), net->ifname);
+
+if (net->type != VIR_DOMAIN_NET_TYPE_DIRECT)
+continue;
+
+if (qemudDomainConnectNetBackend(conn, driver, vm,
+ net, qemuCmdFlags) < 0)
+VIR_ERROR(_("qemudDomainConnectNetBackend failed (%s)"),
+
[libvirt] [RFC][PATCH 1/2] Separate backend setup from NIC device add/remove
This patch moves backend setup code from qemudDomainAttachNetDevice and
qemudDomainDetachNetDevice into separate functions. While the intent is
to allow the new functions to be called separately without affecting VM
NIC devices, this is arguably a worthwhile refactoring on its own.
Currently it's based on 0.7.6 with some macvtap patches. I can rebase
it against git if there's interest.
Signed-off-by: Ed Swierk
---
Move the code that creates and destroys the network backend and
connects it to qemu from the code that creates the virtual NIC.
Index: libvirt-0.7.6/src/qemu/qemu_driver.c
===
--- libvirt-0.7.6.orig/src/qemu/qemu_driver.c
+++ libvirt-0.7.6/src/qemu/qemu_driver.c
@@ -132,6 +132,17 @@ static int qemuDetectVcpuPIDs(virConnect
static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
virDomainDefPtr def);
+static int qemudDomainConnectNetBackend(virConnectPtr conn,
+struct qemud_driver *driver,
+virDomainObjPtr vm,
+virDomainNetDefPtr net,
+unsigned int qemuCmdFlags);
+
+static int qemudDomainDisconnectNetBackend(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainNetDefPtr net);
+
static struct qemud_driver *qemu_driver = NULL;
@@ -5609,19 +5620,17 @@ error:
}
-static int qemudDomainAttachNetDevice(virConnectPtr conn,
- struct qemud_driver *driver,
- virDomainObjPtr vm,
- virDomainNetDefPtr net,
- unsigned int qemuCmdFlags)
+static int qemudDomainConnectNetBackend(virConnectPtr conn,
+struct qemud_driver *driver,
+virDomainObjPtr vm,
+virDomainNetDefPtr net,
+unsigned int qemuCmdFlags)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
char *tapfd_name = NULL;
int tapfd = -1;
-char *nicstr = NULL;
char *netstr = NULL;
int ret = -1;
-virDomainDevicePCIAddress guestAddr;
int vlan;
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HOST_NET_ADD)) {
@@ -5630,142 +5639,132 @@ static int qemudDomainAttachNetDevice(vi
return -1;
}
+if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
+qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+ _("network device type '%s' cannot be attached: "
+ "qemu is not using a unix socket monitor"),
+ virDomainNetTypeToString(net->type));
+return -1;
+}
+
+if ((vlan = qemuDomainNetVLAN(net)) < 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s",
+ _("Unable to connect network backend without vlan"));
+goto out;
+}
+
if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
-if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
-qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
- _("network device type '%s' cannot be attached: "
- "qemu is not using a unix socket monitor"),
- virDomainNetTypeToString(net->type));
-return -1;
-}
-
-if ((tapfd = qemudNetworkIfaceConnect(conn, driver, net,
qemuCmdFlags)) < 0)
-return -1;
+tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags);
} else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
-qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
- _("direct device type '%s' cannot be attached: "
- "qemu is not using a unix socket monitor"),
- virDomainNetTypeToString(net->type));
-return -1;
-}
+tapfd = qemudPhysIfaceConnect(conn, net,
+ net->data.direct.linkdev,
+ net->data.direct.mode);
+}
+if (tapfd < 0)
+return -1;
-if ((tapfd = qemudPhysIfaceConnect(conn, net,
- net->data.direct.linkdev,
- net->data.direct.mode)) < 0)
-return -1;
+if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0) {
+virReportOOMError(conn);
+close(tapfd);
+return -1;
}
[libvirt] [RFC][PATCH 0/2] Dynamic backend setup for macvtap interfaces
Using a bridge to connect a qemu NIC to a host interface offers a fair amount of flexibility to reconfigure the host without restarting the VM. For example, if the bridge connects host interface eth0 to the qemu tap0 interface, eth0 can be hot-removed and hot-plugged without affecting the VM. Similarly, if the bridge connects host VLAN interface vlan0 to the qemu tap0 interface, the admin can easily replace vlan0 with vlan1 without the VM noticing. Using the macvtap driver instead of a kernel bridge, the host interface is much more tightly tied to the VM. Qemu communicates with the macvtap interface through a file descriptor, and the macvtap interface is bound permanently to a specific host interface when it is first created. What's more, if the underlying host interface disappears, the macvtap interface vanishes along with it, leaving the VM holding a file descriptor for a deleted file. To avoid race conditions during system startup, I would like libvirt to allow starting up the VM with a NIC even if the underlying host interface doesn't yet exist, deferring creation of the macvtap interface (analogous to starting up the VM with a tap interface bound to an orphan bridge). To support adding and removing a host interface without restarting the VM, I would like libvirt to react to the (re)appearance of the underlying host interface, creating a new macvtap interface and passing the new fd to qemu to reconnect to the NIC. (It would also be nice if libvirt allowed the user to change which underlying host interface the qemu NIC is connected to. I'm ignoring this issue for now, except to note that implementing the above features should make this easier.) The libvirt API already supports domainAttachDevice and domainDetachDevice to add or remove an interface while the VM is running. In the qemu implementation, these commands add or remove the VM NIC device as well as reconfiguring the host side. This works only if the OS and application running in the VM can handle PCI hotplug and dynamically reconfigure its network. I would like to isolate the VM from changes to the host network setup, whether you use macvtap or a bridge. The changes I think are needed to implement this include: 1. Refactor qemudDomainAttachNetDevice/qemudDomainDetachNetDevice, which currently handle both backend (host) setup and adding/removing the VM NIC device; move the backend setup code into separate functions that can called separately without affecting VM devices. 2. Implement a thread or task that watches for changes to the underlying host interface for each configured macvtap interface, and reacts by invoking the appropriate backend setup code. 3. Change qemudBuildCommandLine to defer backend setup if qemu supports the necessary features for doing it later (e.g. the host_net_add monitor command). 4. Implement appropriate error handling and reporting, and any necessary changes to the configuration schema. The following patches are a partial implementation of the above as a proof of concept. Patch 1 implements change (1) above, moving the backend setup code to new functions qemudDomainConnectNetBackend/qemudDomainDisconnectNetBackend, and calling these functions from the existing qemudDomainAttachNetDevice/qemudDomainDetachNetDevice. I think this change is useful on its own: it breaks up two monster functions into more manageable pieces, and eliminates some code duplication (e.g. the try_remove clause at the end of qemudDomainAttachNetDevice). Patch 2 is a godawful hack roughly implementing change (2) above (did I mention that this is a proof of concept?). It spawns a thread that simply tries reconnecting the backend of each macvtap interface once a second. As long as the interface is already up, the reconnection fails. If the macvtap interface goes away because the underlying host interface disappears, the reconnection fails until the host interface reappears. I ran into two major issues while implementing change (2): - Can we use the existing virEvent functions to invoke the reconnection process, triggered either by a timer or by an event from the host? It seems like this ought to work, but it appears that communication between libvirt and the qemu monitor relies on an event, and since all events run in the same thread, there's no way for an event to call the monitor. - Should the reconnection process use udev or hal to get notifications, or leverage the node device code which itself uses udev or hal? Currently there doesn't appear to be a way to get notifications of changes to node devices; if there were, we'd still need to address the threading issue. If we use node devices, what changes to the configuration schema would be needed to associate a macvtap interface with the underlying node device? I didn't attempt to implement changes (3) or (4) above but would welcome input on those items as well. --Ed -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH [0/3] Filesystem pool formatting
On 02/18/2010 05:58 PM, David Allan wrote: I finished these patches this afternoon, and I need to do a bit more testing before I'm comfortable having these patches committed, but I'm sending them along so I can get everybody's feedback overnight. In particular, I'm wondering if my move of the pool unref call is correct. Having done some additional testing and code review, I think these patches are ready to go. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] Fix daemon-conf invalid failures
On 02/19/2010 12:38 PM, Daniel Veillard wrote:
On Mon, Jan 25, 2010 at 02:46:38PM -0500, David Allan wrote:
The daemon-conf test would fail on my system if there was a system libvirtd
running. In the course of troubleshooting that problem, I discovered that the
daemon-conf script would always fail if run by itself because it found the line:
\# that each "PARAMETER = VALUE" line in this file have the parameter
which it mistook for a line containing a parameter. I have changed the test to avoid
mistaking a line containing \"PARAMETER = VALUE\" for a parameter line.
The corrupted config tests turned out to be failing because the test daemon was
discovering the pid file from the running daemon and exiting before it
processed the test config file. Specifying the pid file for the corrupt config
tests in the same way as for the valid config test solved that problem.
---
tests/daemon-conf |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/daemon-conf b/tests/daemon-conf
index 1eb4be1..10c1628 100755
--- a/tests/daemon-conf
+++ b/tests/daemon-conf
@@ -19,7 +19,7 @@ grep '^#define WITH_QEMU 1' "$CONFIG_HEADER"> /dev/null ||
conf="$abs_top_srcdir/daemon/libvirtd.conf"
# Ensure that each commented out PARAMETER = VALUE line has the expected form.
-grep '[a-z_] *= *[^ ]' "$conf" | grep -vE '^#[a-z_]+ = ' \
+grep -v '\"PARAMETER = VALUE\"' "$conf" | grep '[a-z_] *= *[^ ]' | grep -vE
'^#[a-z_]+ = ' \
&& { echo "$0: found unexpected lines (above) in $conf" 1>&2; exit 1; }
# Start with the sample libvirtd.conf file, uncommenting all real directives.
@@ -45,7 +45,7 @@ while :; do
esac
# Run libvirtd, expecting it to fail.
- $abs_top_builddir/daemon/libvirtd --config=$f 2> err&& fail=1
+ $abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=$f 2> err&&
fail=1
case $rhs in
# '"'*) msg='should be a string';;
Digging my folder, I found that old patch which hadn't been commited,
done now :-)
Daniel
Thanks Daniel, I've been meaning to push it for weeks and it keeps
slipping my mind.
Dave
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virFork: placate static analyzers: ignore pthread_sigmask return value
Laine Stump wrote: > On 02/19/2010 12:41 PM, Jim Meyering wrote: >> Since we check all other uses of pthread_sigmask, be consistent >> and clear: mark that we're deliberately ignoring this one. > > Ah, right. I didn't think of that (and my compiler didn't > complain. What target didn't I run / cflag didn't I set? I know you > sent mail about it awhile back, but I'm not firing on enough cylinders > right now to go looking for it ;-)) That was reported by coverity. I don't know if clang could detect it. gcc currently doesn't do enough static analysis to notice that, but could be made to warn if we gave that function the warn_unused_result attribute. Though in this case, it's really not important. No harm in ignoring, since the only possible failure appears to be EINVAL, which you'd get for passing it an invalid arg. I wrote the patch solely to suppress the false-positive warning. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virFork: placate static analyzers: ignore pthread_sigmask return value
On 02/19/2010 12:41 PM, Jim Meyering wrote:
Since we check all other uses of pthread_sigmask, be consistent
and clear: mark that we're deliberately ignoring this one.
Ah, right. I didn't think of that (and my compiler didn't complain. What
target didn't I run / cflag didn't I set? I know you sent mail about it
awhile back, but I'm not firing on enough cylinders right now to go
looking for it ;-))
> From 0cb4b2ffa51bb8c73243876221bff6d7edecd9cf Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Fri, 19 Feb 2010 18:40:14 +0100
Subject: [PATCH] virFork: placate static analyzers: ignore pthread_sigmask
return value
* src/util/util.c: Include "ignore-value.h".
(virFork): We really do want to ignore pthread_sigmask failure.
---
src/util/util.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 624e570..cf7bba5 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -69,6 +69,7 @@
#include "virterror_internal.h"
#include "logging.h"
#include "event.h"
+#include "ignore-value.h"
#include "buf.h"
#include "util.h"
#include "memory.h"
@@ -344,7 +345,7 @@ int virFork(pid_t *pid) {
if (*pid< 0) {
/* attempt to restore signal mask, but ignore failure, to
avoid obscuring the fork failure */
-pthread_sigmask(SIG_SETMASK,&oldmask, NULL);
+ignore_value (pthread_sigmask(SIG_SETMASK,&oldmask, NULL));
virReportSystemError(saved_errno,
"%s", _("cannot fork child process"));
goto cleanup;
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
ACK (if I'm allowed ;-)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh.c: avoid leak on OOM error path
Daniel Veillard wrote: > ACK Thanks. All pushed, except for the latest, "virFork: placate static analyzers..." No rush at all on that one, of course. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virFork: placate static analyzers: ignore pthread_sigmask return value
Since we check all other uses of pthread_sigmask, be consistent
and clear: mark that we're deliberately ignoring this one.
>From 0cb4b2ffa51bb8c73243876221bff6d7edecd9cf Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Fri, 19 Feb 2010 18:40:14 +0100
Subject: [PATCH] virFork: placate static analyzers: ignore pthread_sigmask
return value
* src/util/util.c: Include "ignore-value.h".
(virFork): We really do want to ignore pthread_sigmask failure.
---
src/util/util.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 624e570..cf7bba5 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -69,6 +69,7 @@
#include "virterror_internal.h"
#include "logging.h"
#include "event.h"
+#include "ignore-value.h"
#include "buf.h"
#include "util.h"
#include "memory.h"
@@ -344,7 +345,7 @@ int virFork(pid_t *pid) {
if (*pid < 0) {
/* attempt to restore signal mask, but ignore failure, to
avoid obscuring the fork failure */
-pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
virReportSystemError(saved_errno,
"%s", _("cannot fork child process"));
goto cleanup;
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] Fix daemon-conf invalid failures
On Mon, Jan 25, 2010 at 02:46:38PM -0500, David Allan wrote:
> The daemon-conf test would fail on my system if there was a system libvirtd
> running. In the course of troubleshooting that problem, I discovered that
> the daemon-conf script would always fail if run by itself because it found
> the line:
>
> \# that each "PARAMETER = VALUE" line in this file have the parameter
>
> which it mistook for a line containing a parameter. I have changed the test
> to avoid mistaking a line containing \"PARAMETER = VALUE\" for a parameter
> line.
>
> The corrupted config tests turned out to be failing because the test daemon
> was discovering the pid file from the running daemon and exiting before it
> processed the test config file. Specifying the pid file for the corrupt
> config tests in the same way as for the valid config test solved that problem.
> ---
> tests/daemon-conf |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/daemon-conf b/tests/daemon-conf
> index 1eb4be1..10c1628 100755
> --- a/tests/daemon-conf
> +++ b/tests/daemon-conf
> @@ -19,7 +19,7 @@ grep '^#define WITH_QEMU 1' "$CONFIG_HEADER" > /dev/null ||
> conf="$abs_top_srcdir/daemon/libvirtd.conf"
>
> # Ensure that each commented out PARAMETER = VALUE line has the expected
> form.
> -grep '[a-z_] *= *[^ ]' "$conf" | grep -vE '^#[a-z_]+ = ' \
> +grep -v '\"PARAMETER = VALUE\"' "$conf" | grep '[a-z_] *= *[^ ]' | grep
> -vE '^#[a-z_]+ = ' \
>&& { echo "$0: found unexpected lines (above) in $conf" 1>&2; exit 1; }
>
> # Start with the sample libvirtd.conf file, uncommenting all real directives.
> @@ -45,7 +45,7 @@ while :; do
>esac
>
># Run libvirtd, expecting it to fail.
> - $abs_top_builddir/daemon/libvirtd --config=$f 2> err && fail=1
> + $abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=$f 2> err
> && fail=1
>
>case $rhs in
> # '"'*) msg='should be a string';;
Digging my folder, I found that old patch which hadn't been commited,
done now :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] rework virFileCreate into virFileOperation w/hook function
On Thu, Feb 18, 2010 at 01:25:43PM -0500, Laine Stump wrote: > > (This patchset requires that the virFork() patchset I send a couple > hours ago be applied first, otherwise it will fail to apply). > > virFileCreate just didn't have what it takes to get the job done. It > turns out that there are other things that must be done to files as the > qemu (or whatever) user, so this patchset adds a hook function that > gets called during the child process. That solves a problem I found > with creating raw storage volumes on root-squash NFS, and will also be > used for an upcoming domain-save-on-root-squash-nfs patch. > > I'm now beginning to think that virFileCreate/virFileOperation is just > too narrow in scope, and it's getting too many options. Possibly it > will be better to just make a simpler virCallasUID() function that > gets a pointer to a function to execute in the child process as an > argument and does nothing but fork/setuid/call the function. That > function would then contain *all* of the file operations, > including creating/opening/closing the file. But that's too large of a > change to contemplate so soon before a release, and this functionality > is necessary for two important bug fixes (mentioned above), so... Okay, I agree that the functions are getting a bit too complex and some cleanup will be in order, but for the sake of the release I pushed the 2 patches which looks good to me, with the expectation that we can cleanup this next month, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not omit va_end(argptr) call
On Fri, Feb 19, 2010 at 05:54:53PM +0100, Jim Meyering wrote:
> Just realized I didn't post a version of this without
> the gnulib diff that Eric pointed out, so here you go:
>
> >From 4501145dd3529dc510e6cb52025a71dd0b111b4d Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Thu, 18 Feb 2010 21:25:01 +0100
> Subject: [PATCH] virBufferVSprintf: do not omit va_end(argptr) call
>
> * src/util/buf.c (virBufferVSprintf): Do not omit va_end(argptr).
> Improved-by: Daniel Veillard.
> ---
> src/util/buf.c |5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/buf.c b/src/util/buf.c
> index cc0a087..fc1217b 100644
> --- a/src/util/buf.c
> +++ b/src/util/buf.c
> @@ -245,12 +245,15 @@ virBufferVSprintf(const virBufferPtr buf, const char
> *format, ...)
> va_end(locarg);
>
> grow_size = (count > 1000) ? count : 1000;
> -if (virBufferGrow(buf, grow_size) < 0)
> +if (virBufferGrow(buf, grow_size) < 0) {
> +va_end(argptr);
> return;
> +}
>
> size = buf->size - buf->use - 1;
> va_copy(locarg, argptr);
> }
> +va_end(argptr);
> va_end(locarg);
> buf->use += count;
> buf->content[buf->use] = '\0';
Yup, ACK !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh.c: avoid leak on OOM error path
On Fri, Feb 19, 2010 at 06:09:13PM +0100, Jim Meyering wrote:
> No one really cares if we leak memory while dying, but who knows...
especially in virsh ...
> freeing that buffer may let us go down more gracefully.
>
> FYI, the leak is triggered when virFileReadAll succeeds
> (it allocates "buffer"), yet xmlNewDoc fails:
>
> if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
> return FALSE;
>
> doc = xmlNewDoc(NULL);
> if (doc == NULL)
> goto no_memory;
>
>
> >From 03c7a44e3d5b2e7c992bebc98fc8c6a7bf63881e Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Fri, 19 Feb 2010 18:03:41 +0100
> Subject: [PATCH] virsh.c: avoid leak on OOM error path
>
> * tools/virsh.c (cmdCPUBaseline): Also free "buffer" upon OOM.
> ---
> tools/virsh.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index dd916f3..8756a7a 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7139,6 +7139,7 @@ cleanup:
> return ret;
>
> no_memory:
> +VIR_FREE(buffer);
> vshError(ctl, "%s", _("Out of memory"));
> ret = FALSE;
> return ret;
ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh.c: avoid leak on OOM error path
Jim Meyering wrote:
> No one really cares if we leak memory while dying, but who knows...
> freeing that buffer may let us go down more gracefully.
>
> FYI, the leak is triggered when virFileReadAll succeeds
> (it allocates "buffer"), yet xmlNewDoc fails:
>
> if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
> return FALSE;
>
> doc = xmlNewDoc(NULL);
> if (doc == NULL)
> goto no_memory;
>
>
>>From 03c7a44e3d5b2e7c992bebc98fc8c6a7bf63881e Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Fri, 19 Feb 2010 18:03:41 +0100
> Subject: [PATCH] virsh.c: avoid leak on OOM error path
>
> * tools/virsh.c (cmdCPUBaseline): Also free "buffer" upon OOM.
> ---
> tools/virsh.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index dd916f3..8756a7a 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7139,6 +7139,7 @@ cleanup:
> return ret;
>
> no_memory:
> +VIR_FREE(buffer);
> vshError(ctl, "%s", _("Out of memory"));
> ret = FALSE;
> return ret;
> --
The above is correct, but there's another leak in the same function,
so I've amended the patch to also free the "list" buffer.
"list" is allocated in the for-loop.
If on the 2nd or subsequent iteration of that loop we take
the "goto no_memory", we'd leak that buffer.
>From 3d97412799c1b5bfedc647059c7d3b18e763f0bc Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Fri, 19 Feb 2010 18:03:41 +0100
Subject: [PATCH] virsh.c: avoid leak on OOM error path
* tools/virsh.c (cmdCPUBaseline): Also free "buffer" and "list" upon OOM.
---
tools/virsh.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index dd916f3..c8ae9f2 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7139,6 +7139,8 @@ cleanup:
return ret;
no_memory:
+VIR_FREE(list);
+VIR_FREE(buffer);
vshError(ctl, "%s", _("Out of memory"));
ret = FALSE;
return ret;
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] xend_internal.c: don't dereference NULL for unexpected input
On Fri, Feb 19, 2010 at 05:47:08PM +0100, Jim Meyering wrote:
> My recent xend_internal.c change introduced a bug.
> "val" could be NULL just before the STREQ comparisons.
> Here's a fix:
>
> >From 5cc834ab30a444cc35ddc9317379f4be3b5cf4c5 Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Fri, 19 Feb 2010 17:45:41 +0100
> Subject: [PATCH] xend_internal.c: don't dereference NULL for unexpected input
>
> * src/xen/xend_internal.c (xenDaemonDomainSetAutostart): Avoid a NULL
> dereference upon non-SEXPR_VALUE'd on_xend_start. This bug was
> introduced by commit 37ce5600c0bb1aed9e2f2888922388de4340ebd3.
> ---
> src/xen/xend_internal.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 1f8b106..9d95291 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -4411,7 +4411,7 @@ xenDaemonDomainSetAutostart(virDomainPtr domain,
> if (autonode) {
> const char *val = (autonode->u.s.car->kind == SEXPR_VALUE
> ? autonode->u.s.car->u.value : NULL);
> -if (!STREQ(val, "ignore") && !STREQ(val, "start")) {
> +if (!val || (!STREQ(val, "ignore") && !STREQ(val, "start"))) {
> virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR,
> "%s", _("unexpected value from on_xend_start"));
> goto error;
Okay, I didn't see that, the change was a bit complex :-) thanks for
double checking that quickly !
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh.c: avoid leak on OOM error path
No one really cares if we leak memory while dying, but who knows...
freeing that buffer may let us go down more gracefully.
FYI, the leak is triggered when virFileReadAll succeeds
(it allocates "buffer"), yet xmlNewDoc fails:
if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0)
return FALSE;
doc = xmlNewDoc(NULL);
if (doc == NULL)
goto no_memory;
>From 03c7a44e3d5b2e7c992bebc98fc8c6a7bf63881e Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Fri, 19 Feb 2010 18:03:41 +0100
Subject: [PATCH] virsh.c: avoid leak on OOM error path
* tools/virsh.c (cmdCPUBaseline): Also free "buffer" upon OOM.
---
tools/virsh.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index dd916f3..8756a7a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7139,6 +7139,7 @@ cleanup:
return ret;
no_memory:
+VIR_FREE(buffer);
vshError(ctl, "%s", _("Out of memory"));
ret = FALSE;
return ret;
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virBufferVSprintf: do not omit va_end(argptr) call
Just realized I didn't post a version of this without
the gnulib diff that Eric pointed out, so here you go:
>From 4501145dd3529dc510e6cb52025a71dd0b111b4d Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Thu, 18 Feb 2010 21:25:01 +0100
Subject: [PATCH] virBufferVSprintf: do not omit va_end(argptr) call
* src/util/buf.c (virBufferVSprintf): Do not omit va_end(argptr).
Improved-by: Daniel Veillard.
---
src/util/buf.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/util/buf.c b/src/util/buf.c
index cc0a087..fc1217b 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -245,12 +245,15 @@ virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
va_end(locarg);
grow_size = (count > 1000) ? count : 1000;
-if (virBufferGrow(buf, grow_size) < 0)
+if (virBufferGrow(buf, grow_size) < 0) {
+va_end(argptr);
return;
+}
size = buf->size - buf->use - 1;
va_copy(locarg, argptr);
}
+va_end(argptr);
va_end(locarg);
buf->use += count;
buf->content[buf->use] = '\0';
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] xend_internal.c: don't dereference NULL for unexpected input
My recent xend_internal.c change introduced a bug.
"val" could be NULL just before the STREQ comparisons.
Here's a fix:
>From 5cc834ab30a444cc35ddc9317379f4be3b5cf4c5 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Fri, 19 Feb 2010 17:45:41 +0100
Subject: [PATCH] xend_internal.c: don't dereference NULL for unexpected input
* src/xen/xend_internal.c (xenDaemonDomainSetAutostart): Avoid a NULL
dereference upon non-SEXPR_VALUE'd on_xend_start. This bug was
introduced by commit 37ce5600c0bb1aed9e2f2888922388de4340ebd3.
---
src/xen/xend_internal.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 1f8b106..9d95291 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4411,7 +4411,7 @@ xenDaemonDomainSetAutostart(virDomainPtr domain,
if (autonode) {
const char *val = (autonode->u.s.car->kind == SEXPR_VALUE
? autonode->u.s.car->u.value : NULL);
-if (!STREQ(val, "ignore") && !STREQ(val, "start")) {
+if (!val || (!STREQ(val, "ignore") && !STREQ(val, "start"))) {
virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR,
"%s", _("unexpected value from on_xend_start"));
goto error;
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] remote: Print ssh stderr on connection failure
On 02/18/2010 03:56 PM, Daniel Veillard wrote:
> On Thu, Feb 18, 2010 at 10:56:47AM -0500, Cole Robinson wrote:
>>
>> Signed-off-by: Cole Robinson
>> ---
>> src/remote/remote_driver.c | 38 +++---
>> 1 files changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>> index 13534ce..8914c39 100644
>> --- a/src/remote/remote_driver.c
>> +++ b/src/remote/remote_driver.c
>> @@ -154,6 +154,7 @@ struct private_data {
>> virMutex lock;
>>
>> int sock; /* Socket. */
>> +int errfd;/* File handle connected to remote stderr */
>> int watch; /* File handle watch */
>> pid_t pid; /* PID of tunnel process */
>> int uses_tls; /* TLS enabled on socket? */
>> @@ -783,6 +784,7 @@ doRemoteOpen (virConnectPtr conn,
>> case trans_ext: {
>> pid_t pid;
>> int sv[2];
>> +int errfd[2];
>>
>> /* Fork off the external process. Use socketpair to create a
>> private
>> * (unnamed) Unix domain socket to the child process so we don't
>> have
>> @@ -794,14 +796,22 @@ doRemoteOpen (virConnectPtr conn,
>> goto failed;
>> }
>>
>> +if (pipe(errfd) == -1) {
>> +virReportSystemError(errno, "%s",
>> + _("unable to create socket pair"));
>> +goto failed;
>> +}
>> +
>> if (virExec((const char**)cmd_argv, NULL, NULL,
>> -&pid, sv[1], &(sv[1]), NULL,
>> +&pid, sv[1], &(sv[1]), &(errfd[1]),
>> VIR_EXEC_CLEAR_CAPS) < 0)
>> goto failed;
>>
>> /* Parent continues here. */
>> close (sv[1]);
>> +close (errfd[1]);
>> priv->sock = sv[0];
>> +priv->errfd = errfd[0];
>> priv->pid = pid;
>>
>> /* Do not set 'is_secure' flag since we can't guarentee
>> @@ -827,6 +837,12 @@ doRemoteOpen (virConnectPtr conn,
>> goto failed;
>> }
>>
>> +if ((priv->errfd != -1) && virSetNonBlock(priv->errfd) < 0) {
>> +virReportSystemError(errno, "%s",
>> + _("unable to make socket non-blocking"));
>> +goto failed;
>> +}
>> +
>> if (pipe(wakeupFD) < 0) {
>> virReportSystemError(errno, "%s",
>> _("unable to make pipe"));
>> @@ -939,6 +955,9 @@ doRemoteOpen (virConnectPtr conn,
>>
>> failed:
>> /* Close the socket if we failed. */
>> +if (priv->errfd >= 0)
>> +close(priv->errfd);
>> +
>> if (priv->sock >= 0) {
>> if (priv->uses_tls && priv->session) {
>> gnutls_bye (priv->session, GNUTLS_SHUT_RDWR);
>> @@ -986,6 +1005,7 @@ remoteAllocPrivateData(virConnectPtr conn)
>> priv->localUses = 1;
>> priv->watch = -1;
>> priv->sock = -1;
>> +priv->errfd = -1;
>>
>> return priv;
>> }
>> @@ -1408,6 +1428,7 @@ doRemoteClose (virConnectPtr conn, struct private_data
>> *priv)
>> sasl_dispose (&priv->saslconn);
>> #endif
>> close (priv->sock);
>> +close (priv->errfd);
>>
>> #ifndef WIN32
>> if (priv->pid > 0) {
>> @@ -7785,12 +7806,23 @@ remoteIOReadBuffer(virConnectPtr conn,
>> if (errno == EWOULDBLOCK)
>> return 0;
>>
>> +char errout[1024] = "\0";
>> +if (priv->errfd != -1) {
>> +saferead(priv->errfd, errout, sizeof(errout));
>> +}
>> +
>> virReportSystemError(errno,
>> - "%s", _("cannot recv data"));
>> + _("cannot recv data: %s"), errout);
>> +
>> } else {
>> +char errout[1024] = "\0";
>> +if (priv->errfd != -1) {
>> +saferead(priv->errfd, errout, sizeof(errout));
>> +}
>> +
>> errorf (in_open ? NULL : conn,
>> VIR_ERR_SYSTEM_ERROR,
>> -"%s", _("server closed connection"));
>> +_("server closed connection: %s"), errout);
>> }
>> return -1;
>> }
>
> It would be nice if we could have a function to read and allocate from
> an fd instead of a static stack allocation, but it's not urgent,
>
> ACK,
>
Thanks, pushed now.
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Check for IA64 kvm
On 02/18/2010 03:57 PM, Daniel Veillard wrote:
> On Thu, Feb 18, 2010 at 10:56:48AM -0500, Cole Robinson wrote:
>> From: Dustin Xiong
>>
>> ACPI feature bit dropped: I asked internally if the -no-acpi option
>> had any meaning for IA64, and was told 'probably not'.
>>
>> Signed-off-by: Cole Robinson
>> ---
>> src/qemu/qemu_conf.c |1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 79bd1e8..f02db9c 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -394,6 +394,7 @@ static const struct qemu_arch_info const arch_info_hvm[]
>> = {
>> { "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 },
>> { "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 },
>> { "ppc",32, NULL, "qemu-system-ppc",NULL, NULL, 0 },
>> +{ "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 },
>> };
>
> ACK,
Thanks, pushed now.
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ PATCH ] fix multiple veth problem for OpenVZ
On Wed, Jan 06, 2010 at 10:36:06PM +0900, Yuji NISHIDA wrote:
> Dear all
>
> This is to fix multiple veth problem.
> NETIF setting was overwritten after first CT because any CT could not be
> found by character name.
>
I found that old patch that we apparently forgot.
> ---
> src/openvz/openvz_conf.c | 38 ++
> src/openvz/openvz_conf.h |1 +
> src/openvz/openvz_driver.c |2 +-
> 3 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
> index 43bbaf2..9fb9f7e 100644
> --- a/src/openvz/openvz_conf.c
> +++ b/src/openvz/openvz_conf.c
> @@ -948,3 +948,41 @@ static int openvzAssignUUIDs(void)
> VIR_FREE(conf_dir);
> return 0;
> }
> +
> +
> +/*
> + * Return CTID from name
> + *
> + */
> +
> +int openvzGetVEID(char *name) {
name is actually a "const char *"
> +char cmd[64];
> +int veid;
> +FILE *fp;
> +
> +strcpy( cmd, VZLIST );
> +strcat( cmd, " " );
> +strcat( cmd, name );
> +strcat( cmd, " -ovpsid -H" );
and to avoid an horrible out of buffer write if name or VZLIST path is
too long I fixed this to use virAsprintf() to allocate and format the
command.
> +if ((fp = popen(cmd, "r")) == NULL) {
> +openvzError(NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed"));
> +return -1;
> +}
> +
> +if (fscanf(fp, "%d\n", &veid ) != 1) {
> +if (feof(fp))
> +return -1;
> +
> +openvzError(NULL, VIR_ERR_INTERNAL_ERROR,
> +"%s", _("Failed to parse vzlist output"));
> +goto cleanup;
> +}
> +
> +return veid;
> +
> + cleanup:
> +fclose(fp);
> +return -1;
> +}
> diff --git a/src/openvz/openvz_conf.h b/src/openvz/openvz_conf.h
> index 00e18b4..518c267 100644
> --- a/src/openvz/openvz_conf.h
> +++ b/src/openvz/openvz_conf.h
> @@ -66,5 +66,6 @@ void openvzFreeDriver(struct openvz_driver *driver);
> int strtoI(const char *str);
> int openvzSetDefinedUUID(int vpsid, unsigned char *uuid);
> unsigned int openvzGetNodeCPUs(void);
> +int openvzGetVEID(char *name);
made the parameter const
> #endif /* OPENVZ_CONF_H */
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 196fd8c..879b5d0 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -667,7 +667,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char
> *vpsid,
> if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> char *dev_name_ve;
> -int veid = strtoI(vpsid);
> +int veid = openvzGetVEID(vpsid);
>
> //--netif_add ifname[,mac,host_ifname,host_mac]
> ADD_ARG_LIT("--netif_add") ;
That said the patch looks reasonable once those are fixed, so I pushed
this, please double check,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make an error message in PCI util code clearer.
On Thu, Feb 18, 2010 at 10:38:35AM -0500, Chris Lalancette wrote: > Signed-off-by: Chris Lalancette Okay, I pushed those 3 patches, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] FW: [PATCH 1/4] Addition of XenAPI support to libvirt
On Fri, Feb 19, 2010 at 11:33:36AM +, Sharadha Prabhakar (3P) wrote: > > thanks for the patch, just started to look at it. > > I assume that by libxenserver you mean the code to be added under src > in a following patch, right ? > > -> libxenserver is a library which provides APIs for performing various > Operations on the hypervisor. You can download it from > http://community.citrix.com/download/attachments/38633496/libxenserver-5.5.0-1-src.tar.bz2?version=1 > libxenserver folder should be in the same directory level of libvirt. Sorry that's not possible. Either the code is in libvirt, and in that case it doesn't have to be downloaded, but part of the patch submission (and managed on this list) , or it's a new external dependancy and it's availability must be checked by configure.ac. If we add a new dependancy, it must also be checked for Licence compatibility. So far we have always refused to use a third party library to access the hypervisor itself, libvirt(d) talking directly to the hypervisor components or launching it directly. For example we don't rely on the VMWare libraries to talk to ESX. > [...] > > +endif > > +#libvirt_driver_xenapi_la_LIBADD = $(XENAPI_LIBS) > > +libvirt_driver_xenapi_la_CFLAGS = $(XEN_CFLAGS) \ > > + $(shell xml2-config --cflags) \ > > + $(shell curl-config --cflags) > > +libvirt_driver_xenapi_la_LDFLAGS = -...@top_srcdir@/../libxenserver/ > > -lxenserver -g $(shell xml2-config --libs) \ > > + $(shell curl-config --libs) > > Hum, libxml2 is already fully detected in configure.ac, use the > LIBXML_* variables instead of trying to exec xml2-config. > > -> I've added the LIBXML_ flags in Makefile.am and posted the patch. okay, we will review this, but note that we start the feature freeze for 0.7.7 this week-end, so next week is more likely to be focused on bug fixes for already commited features. > Similary libcurl i looked at in configure.ac for ESX, this need > to be modified to be checked more generally and the LIBCURL_* > variables need to be used here too. > > -> I didn't quite understand the libcurl part you mentioned. I too noticed > that ESX has > a piece of code in configure.ac to set the libcurl flags, do you want me to > do add a similar > bit of code in configure.ac for XenAPI that defines LIBCURL flags and then > add in my Makefile.am? the code to look for libcurl in configure.ac is currently only activated if esx support is on. It must be run if either esx or xen-api are selected, the ESX portion need to be modified accordingly, and your parts and Makefiles patches too. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [FIX] macvtap mac_filter support
On Fri, Feb 19, 2010 at 07:54:04AM -0500, Stefan Berger wrote:
> Previous posting had an unused variable in the compile case of
> --without-macvtap.
>
> This patch adds the mac_filter support to the macvtap device.
>
> Signed-off-by: Stefan Berger
>
>
> Index: libvirt-macvtap/src/qemu/qemu_conf.c
> ===
> --- libvirt-macvtap.orig/src/qemu/qemu_conf.c
> +++ libvirt-macvtap/src/qemu/qemu_conf.c
> @@ -335,7 +335,7 @@ int qemudLoadDriverConfig(struct qemud_d
> if (!(driver->ebtables = ebtablesContextNew("qemu"))) {
> driver->macFilter = 0;
> virReportSystemError(errno,
> - _("failed to enable mac filter in in '%s'"),
> + _("failed to enable mac filter in '%s'"),
> __FILE__);
> }
>
> @@ -1432,6 +1432,7 @@ int qemudExtractVersion(struct qemud_dri
> */
> int
> qemudPhysIfaceConnect(virConnectPtr conn,
> + struct qemud_driver *driver,
>virDomainNetDefPtr net,
>char *linkdev,
>int brmode,
> @@ -1441,6 +1442,7 @@ qemudPhysIfaceConnect(virConnectPtr conn
> #if WITH_MACVTAP
> char *res_ifname = NULL;
> int vnet_hdr = 0;
> +int err;
>
> if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR &&
> net->model && STREQ(net->model, "virtio"))
> @@ -1452,12 +1454,21 @@ qemudPhysIfaceConnect(virConnectPtr conn
> VIR_FREE(net->ifname);
> net->ifname = res_ifname;
> }
> +
> +if (rc >=0 && driver->macFilter) {
> +if ((err = networkAllowMacOnPort(driver, net->ifname, net->mac))) {
> +virReportSystemError(err,
> + _("failed to add ebtables rule to allow MAC address on
> '%s'"),
> + net->ifname);
> +}
> +}
> #else
> (void)conn;
> (void)net;
> (void)linkdev;
> (void)brmode;
> (void)qemuCmdFlags;
> +(void)driver;
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("No support for macvtap device"));
> rc = -1;
> @@ -3757,7 +3768,7 @@ int qemudBuildCommandLine(virConnectPtr
> if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >=
> sizeof(tapfd_name))
> goto no_memory;
> } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> -int tapfd = qemudPhysIfaceConnect(conn, net,
> +int tapfd = qemudPhysIfaceConnect(conn, driver, net,
>net->data.direct.linkdev,
>net->data.direct.mode,
>qemuCmdFlags);
> Index: libvirt-macvtap/src/qemu/qemu_conf.h
> ===
> --- libvirt-macvtap.orig/src/qemu/qemu_conf.h
> +++ libvirt-macvtap/src/qemu/qemu_conf.h
> @@ -251,6 +251,7 @@ int qemudNetworkIfaceConnect
> ATTRIBUTE_NONNULL(1);
>
> int qemudPhysIfaceConnect(virConnectPtr conn,
> + struct qemud_driver *driver,
>virDomainNetDefPtr net,
>char *linkdev,
>int brmode,
> Index: libvirt-macvtap/src/qemu/qemu_driver.c
> ===
> --- libvirt-macvtap.orig/src/qemu/qemu_driver.c
> +++ libvirt-macvtap/src/qemu/qemu_driver.c
> @@ -5723,7 +5723,7 @@ static int qemudDomainAttachNetDevice(vi
> return -1;
> }
>
> -if ((tapfd = qemudPhysIfaceConnect(conn, net,
> +if ((tapfd = qemudPhysIfaceConnect(conn, driver, net,
> net->data.direct.linkdev,
> net->data.direct.mode,
> qemuCmdFlags)) < 0)
Okay, ACK,
pushed !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] Addition of XenAPI support to libvirt
> This is a patch to add XenAPI driver support for libvirt version 0.7.6. > XenAPI can be used against XenCloud platform and > managed through virsh and virt-manger. This patch supports domain related > APIs in libvirt. It is possible to get domain information, > list active and inactive domains, get Domain XML configuration and > Start/stop/pause/shutdown/destroy VMs. > There will be more patches after this review to support more libvirt APIs and > add remote storage support to XenAPI. > In order to run this patch you would require libxenserver library. > Libxenserver library can be downloaded from > http://community.citrix.com/download/attachments/38633496/libxenserver-5.5.0-1-src.tar.bz2?version=1 > Copy the libxenserver folder in the same directory level as libvirt. I don't really like this idea of downloading libxenserver to a special location relative to libvirt and then building/linking against it especially as libvirt will use libxenserver shared library. How would you install libvirt build in such environment? It is much better to require libxenserver package to be installed in the same way as other dependencies are required and then detect it in configure.ac. Then, you can use --with=libxenserver=path if it's installed in some special path. Also make sure libvirt compiles after every single patch from your series. Especially, you shouldn't change makefiles before the new source files are introduced. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] qemu: Make Set*Mem commands hotplug only
According to Daniel P. Berrange on 2/15/2010 5:00 AM: > FYI, I also have a test case that's intended to validate methods which > should only be called on running domains. If anyone wants to submit > patches for API calls which are missing in my current list > > > http://libvirt.org/git/?p=libvirt-tck.git;a=blob;f=scripts/domain/090-invalid-ops-when-inactive.t;hb=HEAD I didn't see libvirt-tck.git documented anywhere on the libvirt.org site. Seems like it would be useful to mention somewhere, perhaps on the Downloads page. -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
Eric Blake wrote: > According to Jim Meyering on 2/19/2010 6:50 AM: >>> Why the change in .gnulib? Shouldn't that be an independent patch? >> >> Well spotted. >> I saw it too, and (as you probably noticed) removed it >> from the updated patch. > > I still see it in your most recent post: > https://www.redhat.com/archives/libvir-list/2010-February/msg00698.html Humph ;-) Well I did remove it, but then pulled it back in with an --amend. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
Eric Blake wrote: > According to Jim Meyering on 2/19/2010 3:45 AM: >>> Hum, that one I'm not sure. In the case of virBufferGrow failure, >>> we just did va_end(locarg); in the loop before, so going to cleanup >>> there does it twice, and I'm not sure it's legal. Probably simpler to >>> add just va_end(argptr); before return in that case and drop the >>> cleanup: target. >> >> Good catch. Corrected, as you suggest: >> >> +++ b/.gnulib >> @@ -1 +1 @@ >> -Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6 >> +Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2 > > Shouldn't that be an independent patch? Yes. BTW, if I were to update to the latest from gnulib, it would break "syntax-check", due to a weakness in maint.mk's new hash.h check. It gets a false-positive on any inclusion of libvirt's own "hash.h": $ git ls-files|grep hash.h src/util/hash.h I haven't yet decided what to do about that. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
Eric Blake wrote: > According to Jim Meyering on 2/19/2010 3:45 AM: >>> Hum, that one I'm not sure. In the case of virBufferGrow failure, >>> we just did va_end(locarg); in the loop before, so going to cleanup >>> there does it twice, and I'm not sure it's legal. Probably simpler to >>> add just va_end(argptr); before return in that case and drop the >>> cleanup: target. >> >> Good catch. Corrected, as you suggest: >> >> +++ b/.gnulib >> @@ -1 +1 @@ >> -Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6 >> +Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2 > > Why the change in .gnulib? Shouldn't that be an independent patch? Well spotted. I saw it too, and (as you probably noticed) removed it from the updated patch. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
According to Jim Meyering on 2/19/2010 6:50 AM: >> Why the change in .gnulib? Shouldn't that be an independent patch? > > Well spotted. > I saw it too, and (as you probably noticed) removed it > from the updated patch. I still see it in your most recent post: https://www.redhat.com/archives/libvir-list/2010-February/msg00698.html -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
According to Jim Meyering on 2/19/2010 3:45 AM: >> Hum, that one I'm not sure. In the case of virBufferGrow failure, >> we just did va_end(locarg); in the loop before, so going to cleanup >> there does it twice, and I'm not sure it's legal. Probably simpler to >> add just va_end(argptr); before return in that case and drop the >> cleanup: target. > > Good catch. Corrected, as you suggest: > > +++ b/.gnulib > @@ -1 +1 @@ > -Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6 > +Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2 Why the change in .gnulib? Shouldn't that be an independent patch? -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
Jim Meyering wrote:
> Daniel Veillard wrote:
>>> Subject: [PATCH] virBufferVSprintf: do not skip va_end
>>>
>>> * src/util/buf.c (virBufferVSprintf): Do not omit or skip va_end calls.
> ...
>> Hum, that one I'm not sure. In the case of virBufferGrow failure,
>> we just did va_end(locarg); in the loop before, so going to cleanup
>> there does it twice, and I'm not sure it's legal. Probably simpler to
>> add just va_end(argptr); before return in that case and drop the
>> cleanup: target.
>
> Good catch. Corrected, as you suggest:
>
>>From 313af81e8ff93ceb06b6086ea917db6a7eb160cc Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Thu, 18 Feb 2010 21:25:01 +0100
> Subject: [PATCH] virBufferVSprintf: do not omit va_end call
>
> * src/util/buf.c (virBufferVSprintf): Do not omit va_end call.
> Improved-by: Daniel Veillard.
> ---
> .gnulib|2 +-
> src/util/buf.c |4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/.gnulib b/.gnulib
> index 11fbc57..a1d565a 16
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6
> +Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2
> diff --git a/src/util/buf.c b/src/util/buf.c
> index cc0a087..ef72294 100644
> --- a/src/util/buf.c
> +++ b/src/util/buf.c
> @@ -245,8 +245,10 @@ virBufferVSprintf(const virBufferPtr buf, const char
> *format, ...)
> va_end(locarg);
>
> grow_size = (count > 1000) ? count : 1000;
> -if (virBufferGrow(buf, grow_size) < 0)
> +if (virBufferGrow(buf, grow_size) < 0) {
> +va_end(argptr);
> return;
> +}
>
> size = buf->size - buf->use - 1;
> va_copy(locarg, argptr);
I looked at this again and realized that the above is insufficient.
We do have to call va_end(argptr) at the end, as well, so I'm merging
this additional change locally. Otherwise, we'd leak in the common case.
diff --git a/.gnulib b/.gnulib
index 11fbc57..a1d565a 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6
+Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2
diff --git a/src/util/buf.c b/src/util/buf.c
index ef72294..fc1217b 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -252,8 +252,9 @@ virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
size = buf->size - buf->use - 1;
va_copy(locarg, argptr);
}
+va_end(argptr);
va_end(locarg);
buf->use += count;
buf->content[buf->use] = '\0';
}
Amended patch:
>From 2a5ca2656325546231a546cf580f08bb0462d37a Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Thu, 18 Feb 2010 21:25:01 +0100
Subject: [PATCH] virBufferVSprintf: do not omit va_end(argptr) call
* src/util/buf.c (virBufferVSprintf): Do not omit va_end(argptr).
Improved-by: Daniel Veillard.
---
.gnulib|2 +-
src/util/buf.c |5 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/.gnulib b/.gnulib
index 11fbc57..a1d565a 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6
+Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2
diff --git a/src/util/buf.c b/src/util/buf.c
index cc0a087..fc1217b 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -245,12 +245,15 @@ virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
va_end(locarg);
grow_size = (count > 1000) ? count : 1000;
-if (virBufferGrow(buf, grow_size) < 0)
+if (virBufferGrow(buf, grow_size) < 0) {
+va_end(argptr);
return;
+}
size = buf->size - buf->use - 1;
va_copy(locarg, argptr);
}
+va_end(argptr);
va_end(locarg);
buf->use += count;
buf->content[buf->use] = '\0';
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] [FIX] macvtap mac_filter support
Previous posting had an unused variable in the compile case of
--without-macvtap.
This patch adds the mac_filter support to the macvtap device.
Signed-off-by: Stefan Berger
Index: libvirt-macvtap/src/qemu/qemu_conf.c
===
--- libvirt-macvtap.orig/src/qemu/qemu_conf.c
+++ libvirt-macvtap/src/qemu/qemu_conf.c
@@ -335,7 +335,7 @@ int qemudLoadDriverConfig(struct qemud_d
if (!(driver->ebtables = ebtablesContextNew("qemu"))) {
driver->macFilter = 0;
virReportSystemError(errno,
- _("failed to enable mac filter in in '%s'"),
+ _("failed to enable mac filter in '%s'"),
__FILE__);
}
@@ -1432,6 +1432,7 @@ int qemudExtractVersion(struct qemud_dri
*/
int
qemudPhysIfaceConnect(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainNetDefPtr net,
char *linkdev,
int brmode,
@@ -1441,6 +1442,7 @@ qemudPhysIfaceConnect(virConnectPtr conn
#if WITH_MACVTAP
char *res_ifname = NULL;
int vnet_hdr = 0;
+int err;
if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR &&
net->model && STREQ(net->model, "virtio"))
@@ -1452,12 +1454,21 @@ qemudPhysIfaceConnect(virConnectPtr conn
VIR_FREE(net->ifname);
net->ifname = res_ifname;
}
+
+if (rc >=0 && driver->macFilter) {
+if ((err = networkAllowMacOnPort(driver, net->ifname, net->mac))) {
+virReportSystemError(err,
+ _("failed to add ebtables rule to allow MAC address on '%s'"),
+ net->ifname);
+}
+}
#else
(void)conn;
(void)net;
(void)linkdev;
(void)brmode;
(void)qemuCmdFlags;
+(void)driver;
qemuReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("No support for macvtap device"));
rc = -1;
@@ -3757,7 +3768,7 @@ int qemudBuildCommandLine(virConnectPtr
if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name))
goto no_memory;
} else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-int tapfd = qemudPhysIfaceConnect(conn, net,
+int tapfd = qemudPhysIfaceConnect(conn, driver, net,
net->data.direct.linkdev,
net->data.direct.mode,
qemuCmdFlags);
Index: libvirt-macvtap/src/qemu/qemu_conf.h
===
--- libvirt-macvtap.orig/src/qemu/qemu_conf.h
+++ libvirt-macvtap/src/qemu/qemu_conf.h
@@ -251,6 +251,7 @@ int qemudNetworkIfaceConnect
ATTRIBUTE_NONNULL(1);
int qemudPhysIfaceConnect(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainNetDefPtr net,
char *linkdev,
int brmode,
Index: libvirt-macvtap/src/qemu/qemu_driver.c
===
--- libvirt-macvtap.orig/src/qemu/qemu_driver.c
+++ libvirt-macvtap/src/qemu/qemu_driver.c
@@ -5723,7 +5723,7 @@ static int qemudDomainAttachNetDevice(vi
return -1;
}
-if ((tapfd = qemudPhysIfaceConnect(conn, net,
+if ((tapfd = qemudPhysIfaceConnect(conn, driver, net,
net->data.direct.linkdev,
net->data.direct.mode,
qemuCmdFlags)) < 0)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] macvtap mac_filter support
This patch adds the mac_filter support to the macvtap device.
Signed-off-by: Stefan Berger
Index: libvirt-macvtap/src/qemu/qemu_conf.c
===
--- libvirt-macvtap.orig/src/qemu/qemu_conf.c
+++ libvirt-macvtap/src/qemu/qemu_conf.c
@@ -335,7 +335,7 @@ int qemudLoadDriverConfig(struct qemud_d
if (!(driver->ebtables = ebtablesContextNew("qemu"))) {
driver->macFilter = 0;
virReportSystemError(errno,
- _("failed to enable mac filter in in '%s'"),
+ _("failed to enable mac filter in '%s'"),
__FILE__);
}
@@ -1432,6 +1432,7 @@ int qemudExtractVersion(struct qemud_dri
*/
int
qemudPhysIfaceConnect(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainNetDefPtr net,
char *linkdev,
int brmode,
@@ -1441,6 +1442,7 @@ qemudPhysIfaceConnect(virConnectPtr conn
#if WITH_MACVTAP
char *res_ifname = NULL;
int vnet_hdr = 0;
+int err;
if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR &&
net->model && STREQ(net->model, "virtio"))
@@ -1452,6 +1454,14 @@ qemudPhysIfaceConnect(virConnectPtr conn
VIR_FREE(net->ifname);
net->ifname = res_ifname;
}
+
+if (rc >=0 && driver->macFilter) {
+if ((err = networkAllowMacOnPort(driver, net->ifname, net->mac))) {
+virReportSystemError(err,
+ _("failed to add ebtables rule to allow MAC address on '%s'"),
+ net->ifname);
+}
+}
#else
(void)conn;
(void)net;
@@ -3757,7 +3767,7 @@ int qemudBuildCommandLine(virConnectPtr
if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name))
goto no_memory;
} else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-int tapfd = qemudPhysIfaceConnect(conn, net,
+int tapfd = qemudPhysIfaceConnect(conn, driver, net,
net->data.direct.linkdev,
net->data.direct.mode,
qemuCmdFlags);
Index: libvirt-macvtap/src/qemu/qemu_conf.h
===
--- libvirt-macvtap.orig/src/qemu/qemu_conf.h
+++ libvirt-macvtap/src/qemu/qemu_conf.h
@@ -251,6 +251,7 @@ int qemudNetworkIfaceConnect
ATTRIBUTE_NONNULL(1);
int qemudPhysIfaceConnect(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainNetDefPtr net,
char *linkdev,
int brmode,
Index: libvirt-macvtap/src/qemu/qemu_driver.c
===
--- libvirt-macvtap.orig/src/qemu/qemu_driver.c
+++ libvirt-macvtap/src/qemu/qemu_driver.c
@@ -5723,7 +5723,7 @@ static int qemudDomainAttachNetDevice(vi
return -1;
}
-if ((tapfd = qemudPhysIfaceConnect(conn, net,
+if ((tapfd = qemudPhysIfaceConnect(conn, driver, net,
net->data.direct.linkdev,
net->data.direct.mode,
qemuCmdFlags)) < 0)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Addition of XenAPI support to libvirt
diff -Nur ./libvirt_org/src/xenapi/xenapi_utils.c
./libvirt/src/xenapi/xenapi_utils.c
--- ./libvirt_org/src/xenapi/xenapi_utils.c 1970-01-01 01:00:00.0
+0100
+++ ./libvirt/src/xenapi/xenapi_utils.c 2010-02-18 16:26:52.0 +
@@ -0,0 +1,507 @@
+/*
+ * xenapi_utils.c: Xen API driver -- utils parts.
+ * Copyright (C) 2009 Citrix Ltd.
+ * Sharadha Prabhakar
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "libvirt_internal.h"
+#include "libvirt/libvirt.h"
+#include "virterror_internal.h"
+#include "datatypes.h"
+#include "xenapi_driver.h"
+#include "util.h"
+#include "uuid.h"
+#include "memory.h"
+#include "driver.h"
+#include "buf.h"
+#include "xenapi_utils.h"
+
+/* returns 'file' or 'block' for the storage type */
+int
+getStorageVolumeType(char *type)
+{
+if((STREQ(type,"lvmoiscsi")) ||
+ (STREQ(type,"lvmohba")) ||
+ (STREQ(type,"lvm")) ||
+ (STREQ(type,"file")) ||
+ (STREQ(type,"iso")) ||
+ (STREQ(type,"ext")) ||
+ (STREQ(type,"nfs")))
+return (int)VIR_STORAGE_VOL_FILE;
+else if((STREQ(type,"iscsi")) ||
+ (STREQ(type,"equal")) ||
+ (STREQ(type,"hba")) ||
+ (STREQ(type,"cslg")) ||
+ (STREQ(type,"udev")) ||
+ (STREQ(type,"netapp")))
+return (int)VIR_STORAGE_VOL_BLOCK;
+return -1;
+}
+
+/* returns error description if any received from the server */
+char *
+returnErrorFromSession(xen_session *session)
+{
+int i;
+char *buf = NULL;
+for (i=0; ierror_description_count-1; i++) {
+if (buf==NULL) {
+buf = (char *)realloc(buf,strlen(session->error_description[i])+1);
+strcpy(buf,session->error_description[i]);
+} else {
+buf = (char
*)realloc(buf,strlen(buf)+strlen(session->error_description[i])+2);
+strcat(buf,":");
+strcat(buf,session->error_description[i]);
+}
+}
+return buf;
+}
+
+/* XenAPI error handler - internally calls libvirt error handler after
clearing error
+flag in session */
+void xenapiSessionErrorHandler(virConnectPtr conn, virErrorNumber errNum,
+ const char *buf, const char *filename, const
char *func, size_t lineno)
+{
+if (buf==NULL) {
+char *ret=NULL;
+ret = returnErrorFromSession(((struct _xenapiPrivate
*)(conn->privateData))->session);
+virReportErrorHelper (conn, VIR_FROM_XENAPI, errNum, filename, func,
lineno, _("%s\n"), ret);
+xen_session_clear_error(((struct _xenapiPrivate
*)(conn->privateData))->session);
+VIR_FREE(ret);
+} else {
+virReportErrorHelper (conn, VIR_FROM_XENAPI, errNum, filename, func,
lineno, _("%s\n"), buf);
+}
+}
+
+/* converts bitmap to string of the form '1,2...' */
+char *
+mapDomainPinVcpu(unsigned int vcpu, unsigned char *cpumap, int maplen)
+{
+char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64];
+char *ret=NULL;
+int i, j;
+mapstr[0] = 0;
+for (i = 0; i < maplen; i++) {
+for (j = 0; j < 8; j++) {
+if (cpumap[i] & (1 << j)) {
+snprintf(buf, sizeof(buf), "%d,", (8 * i) + j);
+strcat(mapstr, buf);
+ }
+}
+}
+mapstr[strlen(mapstr) - 1] = 0;
+snprintf(buf, sizeof(buf), "%d", vcpu);
+ret = strdup(mapstr);
+return ret;
+}
+
+/* obtains the CPU bitmap from the string passed */
+void
+getCpuBitMapfromString(char *mask, unsigned char *cpumap, int maplen)
+{
+int pos;
+int max_bits = maplen * 8;
+char *num = NULL;
+bzero(cpumap, maplen);
+num = strtok (mask, ",");
+while (num != NULL) {
+sscanf (num, "%d", &pos);
+if (pos<0 || pos>max_bits-1)
+printf ("number in str %d exceeds cpumap's max bits %d\n", pos,
max_bits);
+else
+(cpumap)[pos/8] |= (1<<(pos%8));
+num = strtok (NULL, ",");
+}
+}
+
+
+/* mapping XenServer power state to Libvirt power state */
+virDomainState
+mapPowerState(enum xen_vm_power_state state)
+{
+virDomainState virState;
+switch (state) {
+case (XEN_VM_POWER_STATE_HALTED):
+case (XEN_VM_POWER_STATE_SUSPENDED):
+virState = VIR_DOMAIN_SHUTOFF;
+break;
+case (XEN_VM_POWER_STATE_PAUSED):
+virState = VIR_DOMAIN_PAUSED;
+break;
+case (XEN_VM_POWER_STATE_RUNNING):
+virState = VIR_DOMAIN_RUNNING;
+break;
+case (XEN_VM_POWER_STATE_UNKNOWN):
+case (XEN_VM_POWER_STATE_UNDEFINED):
+virState = VIR_DOMAIN_NOSTATE;
+break;
+default:
+virState = VIR_DOMAIN_NOSTATE;
+break;
+}
+return virState;
+}
+
+/* Gets the value of the child for the current node passed */
+char *
+getXmlChildValue(xmlDocPtr doc, x
[libvirt] [PATCH 2/4] Addition of XenAPI support to libvirt
diff -ur ./libvirt_org/src/libvirt.c ./libvirt/src/libvirt.c
--- ./libvirt_org/src/libvirt.c 2010-02-17 17:38:08.0 +
+++ ./libvirt/src/libvirt.c 2010-02-18 12:21:43.0 +
@@ -64,6 +64,9 @@
#ifdef WITH_ESX
#include "esx/esx_driver.h"
#endif
+#ifdef WITH_XENAPI
+#include "xenapi/xenapi_driver.h"
+#endif
#endif
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -357,6 +360,7 @@
virDriverLoadModule("openvz");
virDriverLoadModule("vbox");
virDriverLoadModule("esx");
+virDriverLoadModule("xenapi");
virDriverLoadModule("remote");
#else
#ifdef WITH_TEST
@@ -377,6 +381,9 @@
#ifdef WITH_ESX
if (esxRegister() == -1) return -1;
#endif
+#ifdef WITH_XENAPI
+if (xenapiRegister () == -1) return -1;
+#endif
#ifdef WITH_REMOTE
if (remoteRegister () == -1) return -1;
#endif
@@ -1035,6 +1042,10 @@
if (STRCASEEQ(type, "Remote"))
*typeVer = remoteVersion();
#endif
+#if WITH_XENAPI
+if (STRCASEEQ(type, "XenAPI"))
+*typeVer = LIBVIR_VERSION_NUMBER;
+#endif
if (*typeVer == 0) {
virLibConnError(NULL, VIR_ERR_NO_SUPPORT, type);
goto error;
diff -ur ./libvirt_org/src/driver.h ./libvirt/src/driver.h
--- ./libvirt_org/src/driver.h 2010-02-17 17:38:08.0 +
+++ ./libvirt/src/driver.h 2010-02-18 10:45:54.0 +
@@ -27,6 +27,7 @@
VIR_DRV_ONE = 9,
VIR_DRV_ESX = 10,
VIR_DRV_PHYP = 11,
+VIR_DRV_XENAPI = 12
} virDrvNo;
diff -ur ./libvirt_org/include/libvirt/virterror.h
./libvirt/include/libvirt/virterror.h
--- ./libvirt_org/include/libvirt/virterror.h 2010-02-17 17:37:51.0
+
+++ ./libvirt/include/libvirt/virterror.h 2010-02-18 12:17:54.0
+
@@ -69,6 +69,7 @@
VIR_FROM_PHYP, /* Error from IBM power hypervisor */
VIR_FROM_SECRET,/* Error from secret storage */
VIR_FROM_CPU, /* Error from CPU driver */
+VIR_FROM_XENAPI /* Error from XenAPI */
} virErrorDomain;
diff -ur ./libvirt_org/src/util/virterror.c ./libvirt/src/util/virterror.c
--- ./libvirt_org/src/util/virterror.c 2010-02-17 17:38:14.0 +
+++ ./libvirt/src/util/virterror.c 2010-02-18 12:13:08.0 +
@@ -85,6 +85,9 @@
case VIR_FROM_XEN:
dom = "Xen ";
break;
+case VIR_FROM_XENAPI:
+dom = "XenAPI ";
+break;
case VIR_FROM_XML:
dom = "XML ";
break;
Only in ./libvirt/src: xenapi
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Addition of XenAPI support to libvirt
Resending patches in plain text. ~/Libvirt/Src/Makefile.am contains some changes suggested by Daniel Veillard. This is a patch to add XenAPI driver support for libvirt version 0.7.6. XenAPI can be used against XenCloud platform and managed through virsh and virt-manger. This patch supports domain related APIs in libvirt. It is possible to get domain information, list active and inactive domains, get Domain XML configuration and Start/stop/pause/shutdown/destroy VMs. There will be more patches after this review to support more libvirt APIs and add remote storage support to XenAPI. In order to run this patch you would require libxenserver library. Libxenserver library can be downloaded from http://community.citrix.com/download/attachments/38633496/libxenserver-5.5.0-1-src.tar.bz2?version=1 Copy the libxenserver folder in the same directory level as libvirt. The XenCloud platform can be downloaded from http://xen.org/products/cloudxen.html diff -ur ./libvirt_org/configure.ac ./libvirt/configure.ac --- ./libvirt_org/configure.ac 2010-02-17 17:39:21.0 + +++ ./libvirt/configure.ac 2010-02-18 11:51:29.0 + @@ -219,6 +219,8 @@ AC_HELP_STRING([--with-libssh2=@<:@PFX@:>@], [libssh2 location @<:@default=/usr/local/lib@:>@]),[],[with_libssh2=yes]) AC_ARG_WITH([phyp], AC_HELP_STRING([--with-phyp], [add PHYP support @<:@default=check@:>@]),[],[with_phyp=check]) +AC_ARG_WITH([xenapi], + AC_HELP_STRING([--with-xenapi], [add XenAPI support @<:@default=yes@:>@]),[],[with_xenapi=check]) AC_ARG_WITH([vbox], AC_HELP_STRING([--with-vbox], [add VirtualBox support @<:@default=yes@:>@]),[],[with_vbox=yes]) AC_ARG_WITH([lxc], @@ -307,6 +309,11 @@ fi AM_CONDITIONAL([WITH_VBOX], [test "$with_vbox" = "yes"]) +if test "$with_xenapi" = "yes"; then +AC_DEFINE_UNQUOTED([WITH_XENAPI], 1, [whether XenAPI driver is enabled]) +fi +AM_CONDITIONAL([WITH_XENAPI], [test "$with_xenapi" = "yes"]) + if test "$with_libvirtd" = "no" ; then with_qemu=no fi @@ -1894,6 +1901,7 @@ AC_MSG_NOTICE([ UML: $with_uml]) AC_MSG_NOTICE([ OpenVZ: $with_openvz]) AC_MSG_NOTICE([VBox: $with_vbox]) +AC_MSG_NOTICE([ XenAPI: $with_xenapi]) AC_MSG_NOTICE([ LXC: $with_lxc]) AC_MSG_NOTICE([PHYP: $with_phyp]) AC_MSG_NOTICE([ ONE: $with_one]) Binary files ./libvirt_org/daemon/.libs/libvirtd and ./libvirt/daemon/.libs/libvirtd differ Only in ./libvirt_org/daemon/.libs: lt-libvirtd --- ./libvirt_org/src/Makefile.am 2010-02-17 17:38:13.0 + +++ ./libvirt/src/Makefile.am 2010-02-19 10:55:51.0 + @@ -3,12 +3,19 @@ # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets # that actually use them + +XENAPI_CFLAGS = -...@top_srcdir@/../libxenserver/include + INCLUDES = \ -I$(top_srcdir)/gnulib/lib \ -I../gnulib/lib \ -I../include\ +-I/usr/include \ -...@top_srcdir@/src/util \ + -...@top_srcdir@/src \ + -...@top_srcdir@/src/xenapi \ -...@top_srcdir@/include \ + $(XENAPI_CFLAGS)\ $(DRIVER_MODULE_CFLAGS) \ $(LIBXML_CFLAGS)\ -DLIBDIR=\""$(libdir)"\"\ @@ -42,6 +49,9 @@ augeastestdir = $(datadir)/augeas/lenses/tests augeastest_DATA = +XENAPI_LIBS = @top_srcdir@/../libxenserver/libxenserver.so +XENAPI_LDFLAGS = -...@top_srcdir@/../libxenserver/ -lxenserver + # These files are not related to driver APIs. Simply generic # helper APIs for various purposes UTIL_SOURCES = \ @@ -205,6 +215,10 @@ qemu/qemu_security_dac.h\ qemu/qemu_security_dac.c +XENAPI_DRIVER_SOURCES = \ +xenapi/xenapi_driver.c xenapi/xenapi_driver.h \ +xenapi/xenapi_utils.c xenapi/xenapi_utils.h + UML_DRIVER_SOURCES = \ uml/uml_conf.c uml/uml_conf.h \ uml/uml_driver.c uml/uml_driver.h @@ -466,6 +480,28 @@ libvirt_driver_vbox_la_SOURCES = $(VBOX_DRIVER_SOURCES) endif +if WITH_XENAPI +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_xenapi.la +else +noinst_LTLIBRARIES += libvirt_driver_xenapi.la + +libvirt_la_LIBADD += libvirt_driver_xenapi.la \ + $(XENAPI_LIBS) +endif +#libvirt_driver_xenapi_la_LIBADD = $(XENAPI_LIBS) +libvirt_driver_xenapi_la_CFLAGS = $(XEN_CFLAGS)
Re: [libvirt] [PATCH] Make virDomainObjFormat static.
On Thu, Feb 18, 2010 at 10:38:36AM -0500, Chris Lalancette wrote:
> Signed-off-by: Chris Lalancette
> ---
> src/conf/domain_conf.c |6 +++---
> src/conf/domain_conf.h |3 ---
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4b5510b..c13973b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5533,9 +5533,9 @@ char *virDomainDefFormat(virDomainDefPtr def,
> return NULL;
> }
>
> -char *virDomainObjFormat(virCapsPtr caps,
> - virDomainObjPtr obj,
> - int flags)
> +static char *virDomainObjFormat(virCapsPtr caps,
> +virDomainObjPtr obj,
> +int flags)
> {
> char *config_xml = NULL;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index d3a0775..3c672c6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -800,9 +800,6 @@ int virDomainDefAddDiskControllers(virDomainDefPtr def);
> #endif
> char *virDomainDefFormat(virDomainDefPtr def,
> int flags);
> -char *virDomainObjFormat(virCapsPtr caps,
> - virDomainObjPtr obj,
> - int flags);
>
> int virDomainCpuSetParse(const char **str,
> char sep,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Better error reporting for failed migration.
On Thu, Feb 18, 2010 at 10:38:37AM -0500, Chris Lalancette wrote:
> If the hostname as returned by "gethostname" resolves
> to "localhost" (as it does with the broken Fedora-12
> installer), then live migration will fail because the
> source will try to migrate to itself. Detect this
> situation up-front and abort the live migration before
> we do any real work.
>
> Signed-off-by: Chris Lalancette
> ---
> src/libvirt_private.syms |1 +
> src/qemu/qemu_driver.c |2 +-
> src/util/util.c | 37 +++--
> src/util/util.h |1 +
> 4 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e3806cd..69ad686 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -568,6 +568,7 @@ virExecDaemonize;
> virSetCloseExec;
> virSetNonBlock;
> virFormatMacAddr;
> +virGetHostnameLocalhost;
> virGetHostname;
> virParseMacAddr;
> virFileDeletePid;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0d8ec04..2123880 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7748,7 +7748,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
> if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0;
>
> /* Get hostname */
> -if ((hostname = virGetHostname(dconn)) == NULL)
> +if ((hostname = virGetHostnameLocalhost(0)) == NULL)
> goto cleanup;
>
> /* XXX this really should have been a properly well-formed
> diff --git a/src/util/util.c b/src/util/util.c
> index cdab300..72cc222 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -2193,11 +2193,11 @@ char *virIndexToDiskName(int idx, const char *prefix)
> #define AI_CANONIDN 0
> #endif
>
> -char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED)
> +char *virGetHostnameLocalhost(int allow_localhost)
> {
> int r;
> char hostname[HOST_NAME_MAX+1], *result;
> -struct addrinfo hints, *info;
> +struct addrinfo hints, *info, *res;
>
> r = gethostname (hostname, sizeof(hostname));
> if (r == -1) {
> @@ -2217,6 +2217,34 @@ char *virGetHostname(virConnectPtr conn
> ATTRIBUTE_UNUSED)
> hostname, gai_strerror(r));
> return NULL;
> }
> +
> +/* if we aren't allowing localhost, then we iterate through the
> + * list and make sure none of the IPv4 addresses are 127.0.0.1 and
> + * that none of the IPv6 addresses are ::1
> + */
> +if (!allow_localhost) {
> +res = info;
> +while (res) {
> +if (res->ai_family == AF_INET) {
> +if (htonl(((struct sockaddr_in
> *)res->ai_addr)->sin_addr.s_addr) == INADDR_LOOPBACK) {
> +virUtilError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("canonical hostname pointed to localhost,
> but this is not allowed"));
> +freeaddrinfo(info);
> +return NULL;
> +}
> +}
> +else if (res->ai_family == AF_INET6) {
> +if (IN6_IS_ADDR_LOOPBACK(&((struct sockaddr_in6
> *)res->ai_addr)->sin6_addr)) {
> +virUtilError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("canonical hostname pointed to localhost,
> but this is not allowed"));
> +freeaddrinfo(info);
> +return NULL;
> +}
> +}
> +res = res->ai_next;
> +}
> +}
> +
> if (info->ai_canonname == NULL) {
> virUtilError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("could not determine canonical host name"));
> @@ -2233,6 +2261,11 @@ char *virGetHostname(virConnectPtr conn
> ATTRIBUTE_UNUSED)
> return result;
> }
>
> +char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED)
> +{
> +return virGetHostnameLocalhost(1);
> +}
> +
> /* send signal to a single process */
> int virKillProcess(pid_t pid, int sig)
> {
> diff --git a/src/util/util.h b/src/util/util.h
> index 4207508..d024fe1 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -232,6 +232,7 @@ static inline int getuid (void) { return 0; }
> static inline int getgid (void) { return 0; }
> #endif
>
> +char *virGetHostnameLocalhost(int allow_localhost);
> char *virGetHostname(virConnectPtr conn);
>
> int virKillProcess(pid_t pid, int sig);
Yes that looks fine, ACK
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make an error message in PCI util code clearer.
On Thu, Feb 18, 2010 at 10:38:35AM -0500, Chris Lalancette wrote:
> Signed-off-by: Chris Lalancette
> ---
> src/util/pci.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/pci.c b/src/util/pci.c
> index aa8344e..83ed461 100644
> --- a/src/util/pci.c
> +++ b/src/util/pci.c
> @@ -540,7 +540,7 @@ pciTrySecondaryBusReset(pciDevice *dev,
> */
> if (pciRead(dev, 0, config_space, PCI_CONF_LEN) < 0) {
> pciReportError(VIR_ERR_NO_SUPPORT,
> - _("Failed to save PCI config space for %s"),
> + _("Failed to read PCI config space for %s"),
> dev->name);
> goto out;
> }
> @@ -586,7 +586,7 @@ pciTryPowerManagementReset(pciDevice *dev)
> /* Save and restore the device's config space. */
> if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) {
> pciReportError(VIR_ERR_NO_SUPPORT,
> - _("Failed to save PCI config space for %s"),
> + _("Failed to read PCI config space for %s"),
> dev->name);
> return -1;
> }
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] xenDaemonDomainSetAutostart: avoid appearance of impropriety
On Thu, Feb 18, 2010 at 07:20:17AM +0100, Jim Meyering wrote:
> Coverity noticed that of the 13 uses of sexpr_lookup,
> only this one was unchecked, and here, the result is dereferenced.
> It happens to be a false positive, due to the preceding sexpr_node
> check, but worth fixing: sexpr_node is just a very thin wrapper
> around sexpr_lookup so there's little justification for looking
> up the same string twice.
>
> >From a9ab34214cf9d247d39731563dcc70b8f1dc73b5 Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Wed, 17 Feb 2010 22:14:25 +0100
> Subject: [PATCH] xenDaemonDomainSetAutostart: avoid appearance of impropriety
>
> * src/xen/xend_internal.c (xenDaemonDomainSetAutostart): Rewrite to
> avoid dereferencing the result of sexpr_lookup. While in this
> particular case, it was guaranteed never to be NULL, due to the
> preceding "if sexpr_node(...)" guard, it's cleaner to skip the
> sexpr_node call altogether, and also saves a lookup.
> ---
> src/xen/xend_internal.c | 10 +-
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 88923c8..1f8b106 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -4383,7 +4383,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain,
> int autostart)
> {
> struct sexpr *root, *autonode;
> -const char *autostr;
> char buf[4096];
> int ret = -1;
> xenUnifiedPrivatePtr priv;
> @@ -4408,16 +4407,17 @@ xenDaemonDomainSetAutostart(virDomainPtr domain,
> return (-1);
> }
>
> -autostr = sexpr_node(root, "domain/on_xend_start");
> -if (autostr) {
> -if (!STREQ(autostr, "ignore") && !STREQ(autostr, "start")) {
> +autonode = sexpr_lookup(root, "domain/on_xend_start");
> +if (autonode) {
> +const char *val = (autonode->u.s.car->kind == SEXPR_VALUE
> + ? autonode->u.s.car->u.value : NULL);
> +if (!STREQ(val, "ignore") && !STREQ(val, "start")) {
> virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR,
> "%s", _("unexpected value from on_xend_start"));
> goto error;
> }
>
> // Change the autostart value in place, then define the new sexpr
> -autonode = sexpr_lookup(root, "domain/on_xend_start");
> VIR_FREE(autonode->u.s.car->u.value);
> autonode->u.s.car->u.value = (autostart ? strdup("start")
> : strdup("ignore"));
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
Daniel Veillard wrote:
>> Subject: [PATCH] virBufferVSprintf: do not skip va_end
>>
>> * src/util/buf.c (virBufferVSprintf): Do not omit or skip va_end calls.
...
> Hum, that one I'm not sure. In the case of virBufferGrow failure,
> we just did va_end(locarg); in the loop before, so going to cleanup
> there does it twice, and I'm not sure it's legal. Probably simpler to
> add just va_end(argptr); before return in that case and drop the
> cleanup: target.
Good catch. Corrected, as you suggest:
>From 313af81e8ff93ceb06b6086ea917db6a7eb160cc Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Thu, 18 Feb 2010 21:25:01 +0100
Subject: [PATCH] virBufferVSprintf: do not omit va_end call
* src/util/buf.c (virBufferVSprintf): Do not omit va_end call.
Improved-by: Daniel Veillard.
---
.gnulib|2 +-
src/util/buf.c |4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/.gnulib b/.gnulib
index 11fbc57..a1d565a 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 11fbc57405a118e6ec9a3ebc19bbf5ececdae4d6
+Subproject commit a1d565aa5b47ec56243e74d4ac8b4988a751fef2
diff --git a/src/util/buf.c b/src/util/buf.c
index cc0a087..ef72294 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -245,8 +245,10 @@ virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
va_end(locarg);
grow_size = (count > 1000) ? count : 1000;
-if (virBufferGrow(buf, grow_size) < 0)
+if (virBufferGrow(buf, grow_size) < 0) {
+va_end(argptr);
return;
+}
size = buf->size - buf->use - 1;
va_copy(locarg, argptr);
--
1.7.0.233.g05e1a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: be careful to return "FALSE" upon OOM
On Thu, Feb 18, 2010 at 11:07:46AM +0100, Jim Meyering wrote:
> Clang reported this as a "dead store" to "ret".
> Here's one way to fix it.
>
> Or just "return FALSE;" and remove the preceding assignment.
>
> >From debb6f5198027a056aa24dca95ea4930184ad3fe Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Thu, 18 Feb 2010 11:05:38 +0100
> Subject: [PATCH] virsh: be careful to return "FALSE" upon OOM
>
> * tools/virsh.c (cmdCPUBaseline): Add an explicit "return" statement
> after the "no_memory:" label.
> ---
> tools/virsh.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index e1d1300..dd916f3 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7141,6 +7141,7 @@ cleanup:
> no_memory:
> vshError(ctl, "%s", _("Out of memory"));
> ret = FALSE;
> +return ret;
> }
>
> /* Common code for the edit / net-edit / pool-edit functions which follow. */
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] macvtap IFF_VNET_HDR configuration
On Thu, Feb 18, 2010 at 05:43:40PM -0500, Stefan Berger wrote:
> This patch sets or unsets the IFF_VNET_HDR flag depending on what device
> is used in the VM. The manipulation of the flag is done in the open
> function and is only fatal if the IFF_VNET_HDR flag could not be cleared
> although it has to be (or if an ioctl generally fails). In that case the
> macvtap tap is closed again and the macvtap interface torn.
>
> This patch also passes 'make syntax-check' :-).
Yup, thanks :-)
> Signed-off-by: Stefan Berger
>
>
> Index: libvirt-macvtap/src/qemu/qemu_conf.c
> ===
> --- libvirt-macvtap.orig/src/qemu/qemu_conf.c
> +++ libvirt-macvtap/src/qemu/qemu_conf.c
> @@ -1434,14 +1434,20 @@ int
> qemudPhysIfaceConnect(virConnectPtr conn,
>virDomainNetDefPtr net,
>char *linkdev,
> - int brmode)
> + int brmode,
> + unsigned long long qemuCmdFlags)
> {
> int rc;
> #if WITH_MACVTAP
> char *res_ifname = NULL;
> +int vnet_hdr = 0;
> +
> +if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR &&
> +net->model && STREQ(net->model, "virtio"))
> +vnet_hdr = 1;
>
> rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode,
> -&res_ifname);
> +&res_ifname, vnet_hdr);
> if (rc >= 0) {
> VIR_FREE(net->ifname);
> net->ifname = res_ifname;
> @@ -1451,6 +1457,7 @@ qemudPhysIfaceConnect(virConnectPtr conn
> (void)net;
> (void)linkdev;
> (void)brmode;
> +(void)qemuCmdFlags;
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("No support for macvtap device"));
> rc = -1;
> @@ -3752,7 +3759,8 @@ int qemudBuildCommandLine(virConnectPtr
> } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> int tapfd = qemudPhysIfaceConnect(conn, net,
>net->data.direct.linkdev,
> - net->data.direct.mode);
> + net->data.direct.mode,
> + qemuCmdFlags);
> if (tapfd < 0)
> goto error;
>
> Index: libvirt-macvtap/src/util/macvtap.c
> ===
> --- libvirt-macvtap.orig/src/util/macvtap.c
> +++ libvirt-macvtap/src/util/macvtap.c
> @@ -615,6 +615,64 @@ macvtapModeFromInt(enum virDomainNetdevM
>
>
> /**
> + * configMacvtapTap:
> + * @tapfd: file descriptor of the macvtap tap
> + * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it
> + *
> + * Returns 0 on success, -1 in case of fatal error, error code otherwise.
> + *
> + * Turn the IFF_VNET_HDR flag, if requested and available, make sure
> + * it's off in the other cases.
> + * A fatal error is defined as the VNET_HDR flag being set but it cannot
> + * be turned off for some reason. This is reported with -1. Other fatal
> + * error is not being able to read the interface flags. In that case the
> + * macvtap device should not be used.
> + */
> +static int
> +configMacvtapTap(int tapfd, int vnet_hdr)
> +{
> +unsigned int features;
> +struct ifreq ifreq;
> +short new_flags = 0;
> +int rc_on_fail = 0;
> +const char *errmsg = NULL;
> +
> +memset(&ifreq, 0, sizeof(ifreq));
> +
> +if (ioctl(tapfd, TUNGETIFF, &ifreq) < 0) {
> +virReportSystemError(errno, "%s",
> + _("cannot get interface flags on macvtap tap"));
> +return -1;
> +}
> +
> +new_flags = ifreq.ifr_flags;
> +
> +if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) {
> +new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR;
> +rc_on_fail = -1;
> +errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap");
> +} else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) {
> +if (ioctl(tapfd, TUNGETFEATURES, &features) != 0)
> +return errno;
> +if ((features & IFF_VNET_HDR)) {
> +new_flags = ifreq.ifr_flags | IFF_VNET_HDR;
> +errmsg = _("cannot set IFF_VNET_HDR flag on macvtap tap");
> +}
> +}
> +
> +if (new_flags != ifreq.ifr_flags) {
> +ifreq.ifr_flags = new_flags;
> +if (ioctl(tapfd, TUNSETIFF, &ifreq) < 0) {
> +virReportSystemError(errno, "%s", errmsg);
> +return rc_on_fail;
> +}
> +}
> +
> +return 0;
> +}
> +
> +
> +/**
> * openMacvtapTap:
> * Create an instance of a macvtap device and open its tap character
> * device.
> @@ -640,7 +698,8 @@ openMacvtapTap(virConnectPtr conn,
> const unsigned char *macaddress,
> const char *linkdev,
> int mode,
> - char **res_ifname)
> + char **res_if
Re: [libvirt] [PATCH] virBufferVSprintf: do not skip va_end
On Thu, Feb 18, 2010 at 09:27:25PM +0100, Jim Meyering wrote: > This fixes the last of the varargs problems reported by coverity: > > va_end(argptr) was never called, and va_end(locarg) would have > been skipped upon OOM. > > >From 7a75b9da0d08a54e9f256dd26cca061b59c32c6d Mon Sep 17 00:00:00 2001 > From: Jim Meyering > Date: Thu, 18 Feb 2010 21:25:01 +0100 > Subject: [PATCH] virBufferVSprintf: do not skip va_end > > * src/util/buf.c (virBufferVSprintf): Do not omit or skip va_end calls. > --- > src/util/buf.c |7 +-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/util/buf.c b/src/util/buf.c > index cc0a087..caf8ee0 100644 > --- a/src/util/buf.c > +++ b/src/util/buf.c > @@ -246,14 +246,17 @@ virBufferVSprintf(const virBufferPtr buf, const char > *format, ...) > > grow_size = (count > 1000) ? count : 1000; > if (virBufferGrow(buf, grow_size) < 0) > -return; > +goto cleanup; > > size = buf->size - buf->use - 1; > va_copy(locarg, argptr); > } > -va_end(locarg); > buf->use += count; > buf->content[buf->use] = '\0'; > + > + cleanup: > +va_end(argptr); > +va_end(locarg); > } > > /** Hum, that one I'm not sure. In the case of virBufferGrow failure, we just did va_end(locarg); in the loop before, so going to cleanup there does it twice, and I'm not sure it's legal. Probably simpler to add just va_end(argptr); before return in that case and drop the cleanup: target. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virBufferStrcat: do not skip va_end
On Thu, Feb 18, 2010 at 08:51:59PM +0100, Jim Meyering wrote:
> Another missed va_end:
>
> >From b2e727f9dd25f427d634ef4d79733b37af2b29dd Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Thu, 18 Feb 2010 20:46:24 +0100
> Subject: [PATCH] virBufferStrcat: do not skip va_end
>
> * src/util/buf.c (virBufferStrcat): Do not skip va_end due to
> an early return.
> ---
> src/util/buf.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/buf.c b/src/util/buf.c
> index e683928..cc0a087 100644
> --- a/src/util/buf.c
> +++ b/src/util/buf.c
> @@ -1,21 +1,21 @@
> /*
> * buf.c: buffers for libvirt
> *
> - * Copyright (C) 2005-2008 Red Hat, Inc.
> + * Copyright (C) 2005-2008, 2010 Red Hat, Inc.
> *
> * See COPYING.LIB for the License of this software
> *
> * Daniel Veillard
> */
>
> #include
>
> #include
> #include
> #include
> #include
> #include "c-ctype.h"
>
> #define __VIR_BUFFER_C__
>
> #include "buf.h"
> @@ -410,25 +410,25 @@ virBufferURIEncodeString (virBufferPtr buf, const char
> *str)
> void
> virBufferStrcat(virBufferPtr buf, ...)
> {
> va_list ap;
> char *str;
>
> if (buf->error)
> return;
>
> va_start(ap, buf);
>
> while ((str = va_arg(ap, char *)) != NULL) {
> unsigned int len = strlen(str);
> unsigned int needSize = buf->use + len + 2;
>
> if (needSize > buf->size) {
> if (virBufferGrow(buf, needSize - buf->use) < 0)
> -return;
> +break;
> }
> memcpy(&buf->content[buf->use], str, len);
> buf->use += len;
> buf->content[buf->use] = 0;
> }
> va_end(ap);
> }
Ah, right, ACK !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qparams.c: do not skip va_end, twice
On Thu, Feb 18, 2010 at 08:29:38PM +0100, Jim Meyering wrote:
> More coverity-prompted fixes:
>
> >From 56d339d99b09c5943fa36600ca39939080cc64f4 Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Thu, 18 Feb 2010 20:27:22 +0100
> Subject: [PATCH] qparams.c: do not skip va_end, twice
>
> * src/util/qparams.c (new_qparam_set, append_qparams): Do not skip
> va_end due to an early return.
> ---
> src/util/qparams.c | 14 +-
> 1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/src/util/qparams.c b/src/util/qparams.c
> index 9535ca4..f6d0713 100644
> --- a/src/util/qparams.c
> +++ b/src/util/qparams.c
> @@ -1,6 +1,6 @@
> -/* Copyright (C) 2007, 2009 Red Hat, Inc.
> +/* Copyright (C) 2007, 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
> * License as published by the Free Software Foundation; either
> * version 2.1 of the License, or (at your option) any later version.
> @@ -60,11 +60,12 @@ new_qparam_set (int init_alloc, ...)
> while ((pname = va_arg (args, char *)) != NULL) {
> pvalue = va_arg (args, char *);
>
> if (append_qparam (ps, pname, pvalue) == -1) {
> free_qparam_set (ps);
> -return NULL;
> +ps = NULL;
> +break;
> }
> }
> va_end (args);
>
> return ps;
> @@ -73,21 +74,24 @@ new_qparam_set (int init_alloc, ...)
> int
> append_qparams (struct qparam_set *ps, ...)
> {
> va_list args;
> const char *pname, *pvalue;
> +int ret = 0;
>
> va_start (args, ps);
> while ((pname = va_arg (args, char *)) != NULL) {
> pvalue = va_arg (args, char *);
>
> -if (append_qparam (ps, pname, pvalue) == -1)
> -return -1;
> +if (append_qparam (ps, pname, pvalue) == -1) {
> +ret = -1;
> +break;
> +}
> }
> va_end (args);
>
> -return 0;
> +return ret;
> }
>
> /* Ensure there is space to store at least one more parameter
> * at the end of the set.
> */
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
