[libvirt] [PATCH] qemu: process: Avoid uninitialized use two vars when reconnecting to vm
by Peter Krempa
3ecebf07110ca8d3413072557f29137943e848e3 breaks the build as it adds a
way to jump to cleanup before the 'cfg' object is retrieved and 'priv'
is initialized.
---
Notes:
Pushed under the build-breaker rule.
src/qemu/qemu_process.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 310a08d..a14b6f7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3571,16 +3571,16 @@ qemuProcessReconnect(void *opaque)
* deleted if qemuConnectMonitor() failed */
virObjectRef(obj);
+ cfg = virQEMUDriverGetConfig(driver);
+ priv = obj->privateData;
+
if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0)
goto killvm;
virNWFilterReadLockFilterUpdates();
- cfg = virQEMUDriverGetConfig(driver);
VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name);
- priv = obj->privateData;
-
/* XXX check PID liveliness & EXE path */
if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, -1) < 0)
goto error;
--
2.1.0
9 years, 11 months
[libvirt] [PATCHv2] qemu: process: Refactor reconnecting to qemu processes
by Peter Krempa
Move entering the job into the thread to simplify the program flow. Also
as the code holds a separate reference to the domain object some
conditions can be simplified.
---
Notes:
Version 2:
- fix leak of 'data' when not reconnecting
src/qemu/qemu_process.c | 161 ++++++++++++++++++++++--------------------------
1 file changed, 72 insertions(+), 89 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 08d6b7c..310a08d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3524,8 +3524,7 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver,
struct qemuProcessReconnectData {
virConnectPtr conn;
virQEMUDriverPtr driver;
- void *payload;
- struct qemuDomainJobObj oldjob;
+ virDomainObjPtr obj;
};
/*
* Open an existing VM's monitor, re-detect VCPU threads
@@ -3534,13 +3533,27 @@ struct qemuProcessReconnectData {
* We own the virConnectPtr we are passed here - whoever started
* this thread function has increased the reference counter to it
* so that we now have to close it.
+ *
+ * This function also inherits a locked domain object.
+ *
+ * This function needs to:
+ * 1. Enter job
+ * 1. just before monitor reconnect do lightweight MonitorEnter
+ * (increase VM refcount and unlock VM)
+ * 2. reconnect to monitor
+ * 3. do lightweight MonitorExit (lock VM)
+ * 4. continue reconnect process
+ * 5. EndJob
+ *
+ * We can't do normal MonitorEnter & MonitorExit because these two lock the
+ * monitor lock, which does not exists in this early phase.
*/
static void
qemuProcessReconnect(void *opaque)
{
struct qemuProcessReconnectData *data = opaque;
virQEMUDriverPtr driver = data->driver;
- virDomainObjPtr obj = data->payload;
+ virDomainObjPtr obj = data->obj;
qemuDomainObjPrivatePtr priv;
virConnectPtr conn = data->conn;
struct qemuDomainJobObj oldjob;
@@ -3550,26 +3563,24 @@ qemuProcessReconnect(void *opaque)
size_t i;
int ret;
- memcpy(&oldjob, &data->oldjob, sizeof(oldjob));
-
VIR_FREE(data);
- virNWFilterReadLockFilterUpdates();
+ qemuDomainObjRestoreJob(obj, &oldjob);
- virObjectLock(obj);
+ /* Hold an extra reference because we can't allow 'vm' to be
+ * deleted if qemuConnectMonitor() failed */
+ virObjectRef(obj);
+
+ if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0)
+ goto killvm;
+
+ virNWFilterReadLockFilterUpdates();
cfg = virQEMUDriverGetConfig(driver);
VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name);
priv = obj->privateData;
- /* Job was started by the caller for us */
- qemuDomainObjTransferJob(obj);
-
- /* Hold an extra reference because we can't allow 'vm' to be
- * deleted if qemuConnectMonitor() failed */
- virObjectRef(obj);
-
/* XXX check PID liveliness & EXE path */
if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, -1) < 0)
goto error;
@@ -3701,10 +3712,10 @@ qemuProcessReconnect(void *opaque)
driver->inhibitCallback(true, driver->inhibitOpaque);
endjob:
- if (!qemuDomainObjEndJob(driver, obj))
- obj = NULL;
+ /* we hold an extra reference, so this will never fail */
+ ignore_value(qemuDomainObjEndJob(driver, obj));
- if (obj && virObjectUnref(obj))
+ if (virObjectUnref(obj))
virObjectUnlock(obj);
virObjectUnref(conn);
@@ -3714,35 +3725,35 @@ qemuProcessReconnect(void *opaque)
return;
error:
- if (!qemuDomainObjEndJob(driver, obj))
- obj = NULL;
-
- if (obj) {
- if (!virDomainObjIsActive(obj)) {
- if (virObjectUnref(obj))
- virObjectUnlock(obj);
- } else if (virObjectUnref(obj)) {
- /* We can't get the monitor back, so must kill the VM
- * to remove danger of it ending up running twice if
- * user tries to start it again later
- */
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
- /* If we couldn't get the monitor and qemu supports
- * no-shutdown, we can safely say that the domain
- * crashed ... */
- state = VIR_DOMAIN_SHUTOFF_CRASHED;
- } else {
- /* ... but if it doesn't we can't say what the state
- * really is and FAILED means "failed to start" */
- state = VIR_DOMAIN_SHUTOFF_UNKNOWN;
- }
- qemuProcessStop(driver, obj, state, 0);
- if (!obj->persistent)
- qemuDomainRemoveInactive(driver, obj);
- else
- virObjectUnlock(obj);
+ /* we hold an extra reference, so this will never fail */
+ ignore_value(qemuDomainObjEndJob(driver, obj));
+
+ killvm:
+ if (virDomainObjIsActive(obj)) {
+ /* We can't get the monitor back, so must kill the VM
+ * to remove danger of it ending up running twice if
+ * user tries to start it again later
+ */
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
+ /* If we couldn't get the monitor and qemu supports
+ * no-shutdown, we can safely say that the domain
+ * crashed ... */
+ state = VIR_DOMAIN_SHUTOFF_CRASHED;
+ } else {
+ /* ... but if it doesn't we can't say what the state
+ * really is and FAILED means "failed to start" */
+ state = VIR_DOMAIN_SHUTOFF_UNKNOWN;
}
+ qemuProcessStop(driver, obj, state, 0);
}
+
+ if (virObjectUnref(obj)) {
+ if (!obj->persistent)
+ qemuDomainRemoveInactive(driver, obj);
+ else
+ virObjectUnlock(obj);
+ }
+
virObjectUnref(conn);
virObjectUnref(cfg);
virNWFilterUnlockFilterUpdates();
@@ -3756,6 +3767,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
struct qemuProcessReconnectData *src = opaque;
struct qemuProcessReconnectData *data;
+ /* If the VM was inactive, we don't need to reconnect */
if (!obj->pid)
return 0;
@@ -3763,64 +3775,35 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
return -1;
memcpy(data, src, sizeof(*data));
- data->payload = obj;
-
- /*
- * We create a separate thread to run qemuProcessReconnect in it.
- * However, qemuProcessReconnect needs to:
- * 1. just before monitor reconnect do lightweight MonitorEnter
- * (increase VM refcount, unlock VM & driver)
- * 2. reconnect to monitor
- * 3. do lightweight MonitorExit (lock VM)
- * 4. continue reconnect process
- * 5. EndJob
- *
- * NB, we can't do normal MonitorEnter & MonitorExit because
- * these two lock the monitor lock, which does not exists in
- * this early phase.
- */
+ data->obj = obj;
+ /* this lock will be eventually transferred to the thread that handles the
+ * reconnect */
virObjectLock(obj);
- qemuDomainObjRestoreJob(obj, &data->oldjob);
-
- if (qemuDomainObjBeginJob(src->driver, obj, QEMU_JOB_MODIFY) < 0)
- goto error;
-
- /* Since we close the connection later on, we have to make sure
- * that the threads we start see a valid connection throughout their
- * lifetime. We simply increase the reference counter here.
+ /* Since we close the connection later on, we have to make sure that the
+ * threads we start see a valid connection throughout their lifetime. We
+ * simply increase the reference counter here.
*/
virObjectRef(data->conn);
if (virThreadCreate(&thread, false, qemuProcessReconnect, data) < 0) {
-
- virObjectUnref(data->conn);
-
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not create thread. QEMU initialization "
"might be incomplete"));
- if (!qemuDomainObjEndJob(src->driver, obj)) {
- obj = NULL;
- } else if (virObjectUnref(obj)) {
- /* We can't spawn a thread and thus connect to monitor.
- * Kill qemu */
- qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, 0);
- if (!obj->persistent)
- qemuDomainRemoveInactive(src->driver, obj);
- else
- virObjectUnlock(obj);
- }
- goto error;
- }
+ /* We can't spawn a thread and thus connect to monitor. Kill qemu. */
+ qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, 0);
+ if (!obj->persistent)
+ qemuDomainRemoveInactive(src->driver, obj);
+ else
+ virObjectUnlock(obj);
- virObjectUnlock(obj);
+ virObjectUnref(data->conn);
+ VIR_FREE(data);
+ return -1;
+ }
return 0;
-
- error:
- VIR_FREE(data);
- return -1;
}
/**
--
2.1.0
9 years, 11 months
[libvirt] [PATCH] spec: Remove qemu-img dependency from storage driver
by Jiri Denemark
Both qemu-img and its stripped down version qcow-create are searched for
dynamically with virFindFileInPath, functions using them report errors
when the required binary is not present, and we don't compile in a fixed
path detected by configure anymore. So I don't see a reason for keeping
the dependencies for our storage driver. Moreover, it's weired when the
dependencies (of the storage driver) change depending on wheter a
particular hypervisor driver (xen/qemu) is enabled or not.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
libvirt.spec.in | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index bda28e7..dbeb62a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -530,15 +530,6 @@ BuildRequires: PolicyKit-devel >= 0.6
# For mount/umount in FS driver
BuildRequires: util-linux
%endif
-%if %{with_qemu}
-# From QEMU RPMs
-BuildRequires: /usr/bin/qemu-img
-%else
- %if %{with_xen}
-# From Xen RPMs
-BuildRequires: /usr/sbin/qcow-create
- %endif
-%endif
%if %{with_storage_lvm}
# For LVM drivers
BuildRequires: lvm2
@@ -871,15 +862,6 @@ Requires: device-mapper
# For Sheepdog support
Requires: sheepdog
%endif
- %if %{with_qemu}
-# From QEMU RPMs
-Requires: /usr/bin/qemu-img
- %else
- %if %{with_xen}
-# From Xen RPMs
-Requires: /usr/sbin/qcow-create
- %endif
- %endif
%description daemon-driver-storage
The storage driver plugin for the libvirtd daemon, providing
--
2.2.0
9 years, 11 months
[libvirt] [PATCH] drvbhyve: Automatically tear down guest domains on shutdown
by Conrad Meyer
Reboot requires more sophistication and is left as a future work item --
but at least part of the plumbing is in place.
---
Looking for feedback. I'm pretty unfamiliar with libvirt; maybe this isn't
quite the right way to do this. Sorry. If you want me to rework it in some way,
just let me know.
I tried to model this off of DrvQEMU, which I knew to support this behavior.
Reboot should probably be processed on a threadpool thread outside of the event
loop, so I omitted it for now.
P.S., the 'read-non-seekable' check test seems to just hang forever on FreeBSD.
I haven't dug into it, but POSIX FIFO behavior is pretty weird.
---
src/Makefile.am | 2 +
src/bhyve/bhyve_monitor.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++
src/bhyve/bhyve_monitor.h | 36 +++++++++
src/bhyve/bhyve_process.c | 14 +++-
4 files changed, 233 insertions(+), 3 deletions(-)
create mode 100644 src/bhyve/bhyve_monitor.c
create mode 100644 src/bhyve/bhyve_monitor.h
diff --git a/src/Makefile.am b/src/Makefile.am
index d8fe624..b6c1701 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -833,6 +833,8 @@ BHYVE_DRIVER_SOURCES = \
bhyve/bhyve_domain.h \
bhyve/bhyve_driver.h \
bhyve/bhyve_driver.c \
+ bhyve/bhyve_monitor.c \
+ bhyve/bhyve_monitor.h \
bhyve/bhyve_process.c \
bhyve/bhyve_process.h \
bhyve/bhyve_utils.h \
diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
new file mode 100644
index 0000000..cd3cf6e
--- /dev/null
+++ b/src/bhyve/bhyve_monitor.c
@@ -0,0 +1,184 @@
+/*
+ * bhyve_monitor.c: Tear-down or reboot bhyve domains on guest shutdown
+ *
+ * Copyright (C) 2014 Conrad Meyer
+ *
+ * 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.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Conrad Meyer <cse.cem(a)gmail.com>
+ */
+
+#include <config.h>
+
+#include <sys/types.h>
+#include <sys/event.h>
+#include <sys/time.h>
+#include <sys/wait.h>
+
+#include "bhyve_monitor.h"
+#include "bhyve_process.h"
+#include "viralloc.h"
+#include "virerror.h"
+#include "virfile.h"
+#include "virlog.h"
+
+#define VIR_FROM_THIS VIR_FROM_BHYVE
+
+VIR_LOG_INIT("bhyve.bhyve_monitor");
+
+struct _bhyveMonitor {
+ int kq;
+ int watch;
+ virDomainObjPtr vm;
+ bhyveConnPtr driver;
+};
+
+static void
+bhyveMonitorIO(int watch, int kq, int events ATTRIBUTE_UNUSED, void *opaque)
+{
+ const struct timespec zerowait = {};
+ bhyveMonitorPtr mon = opaque;
+ struct kevent kev;
+ int rc, status;
+
+ if (watch != mon->watch || kq != mon->kq) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("event from unexpected fd %d!=%d / watch %d!=%d"),
+ mon->kq, kq, mon->watch, watch);
+ return;
+ }
+
+ rc = kevent(kq, NULL, 0, &kev, 1, &zerowait);
+ if (rc < 0) {
+ virReportSystemError(errno, "%s", _("Unable to query kqueue"));
+ return;
+ }
+
+ if (rc == 0)
+ return;
+
+ if ((kev.flags & EV_ERROR) != 0) {
+ virReportSystemError(kev.data, "%s", _("Unable to query kqueue"));
+ return;
+ }
+
+ if (kev.filter == EVFILT_PROC && (kev.fflags & NOTE_EXIT) != 0) {
+ if ((pid_t)kev.ident != mon->vm->pid) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("event from unexpected proc %ju!=%ju"),
+ (uintmax_t)mon->vm->pid, (uintmax_t)kev.ident);
+ return;
+ }
+
+ status = kev.data;
+ if (WIFSIGNALED(status) && WCOREDUMP(status)) {
+ VIR_ERROR("Guest %s got signal %d and crashed", mon->vm->def->name,
+ WTERMSIG(status));
+ virBhyveProcessStop(mon->driver, mon->vm,
+ VIR_DOMAIN_SHUTOFF_CRASHED);
+ } else if (WIFEXITED(status)) {
+ if (WEXITSTATUS(status) == 0) {
+ /* 0 - reboot */
+ /* TODO: Implementing reboot is a little more complicated. */
+ VIR_INFO("Guest %s rebooted; destroying domain.",
+ mon->vm->def->name);
+ virBhyveProcessStop(mon->driver, mon->vm,
+ VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ } else if (WEXITSTATUS(status) < 3) {
+ /* 1 - shutdown, 2 - halt, 3 - triple fault. others - error */
+ VIR_INFO("Guest %s shut itself down; destroying domain.",
+ mon->vm->def->name);
+ virBhyveProcessStop(mon->driver, mon->vm,
+ VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ } else {
+ VIR_INFO("Guest %s had an error and exited with status %d; destroying domain.",
+ mon->vm->def->name, WEXITSTATUS(status));
+ virBhyveProcessStop(mon->driver, mon->vm,
+ VIR_DOMAIN_SHUTOFF_UNKNOWN);
+ }
+ }
+ }
+}
+
+static void
+bhyveMonitorRelease(void *opaque)
+{
+ bhyveMonitorPtr mon = opaque;
+
+ VIR_FORCE_CLOSE(mon->kq);
+ virObjectUnref(mon->vm);
+ VIR_FREE(mon);
+}
+
+bhyveMonitorPtr
+bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
+{
+ bhyveMonitorPtr mon;
+ struct kevent kev;
+ int rc;
+
+ if (VIR_ALLOC(mon) < 0)
+ return NULL;
+
+ mon->vm = virObjectRef(vm);
+ mon->driver = driver;
+
+ mon->kq = kqueue();
+ if (mon->kq < 0) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Unable to create kqueue"));
+ goto cleanup;
+ }
+
+ EV_SET(&kev, vm->pid, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, mon);
+ rc = kevent(mon->kq, &kev, 1, NULL, 0, NULL);
+ if (rc < 0) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Unable to register process kevent"));
+ goto cleanup;
+ }
+
+ mon->watch = virEventAddHandle(mon->kq,
+ VIR_EVENT_HANDLE_READABLE |
+ VIR_EVENT_HANDLE_ERROR |
+ VIR_EVENT_HANDLE_HANGUP,
+ bhyveMonitorIO,
+ mon,
+ bhyveMonitorRelease);
+ if (mon->watch < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unable to register monitor events"));
+ goto cleanup;
+ }
+
+ return mon;
+
+ cleanup:
+ bhyveMonitorRelease(mon);
+ return NULL;
+}
+
+void
+bhyveMonitorClose(bhyveMonitorPtr mon)
+{
+
+ if (mon == NULL)
+ return;
+
+ if (mon->watch > 0)
+ virEventRemoveHandle(mon->watch);
+ else
+ bhyveMonitorRelease(mon);
+}
diff --git a/src/bhyve/bhyve_monitor.h b/src/bhyve/bhyve_monitor.h
new file mode 100644
index 0000000..226d878
--- /dev/null
+++ b/src/bhyve/bhyve_monitor.h
@@ -0,0 +1,36 @@
+/*
+ * bhyve_monitor.h: Tear-down or reboot bhyve domains on guest shutdown
+ *
+ * Copyright (C) 2014 Conrad Meyer
+ *
+ * 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.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Conrad Meyer <cse.cem(a)gmail.com>
+ */
+
+#ifndef BHYVE_MONITOR_H
+# define BHYVE_MONITOR_H
+
+# include "internal.h"
+# include "domain_conf.h"
+# include "bhyve_utils.h"
+
+typedef struct _bhyveMonitor bhyveMonitor;
+typedef bhyveMonitor *bhyveMonitorPtr;
+
+bhyveMonitorPtr bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver);
+void bhyveMonitorClose(bhyveMonitorPtr mon);
+
+#endif /* BHYVE_MONITOR_H */
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index a30e36a..284641a 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -32,8 +32,9 @@
#include <net/if_tap.h>
#include "bhyve_device.h"
-#include "bhyve_process.h"
#include "bhyve_command.h"
+#include "bhyve_monitor.h"
+#include "bhyve_process.h"
#include "datatypes.h"
#include "virerror.h"
#include "virlog.h"
@@ -209,6 +210,7 @@ virBhyveProcessStart(virConnectPtr conn,
vm->def->id = vm->pid;
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
+ vm->privateData = bhyveMonitorOpen(vm, driver);
if (virDomainSaveStatus(driver->xmlopt,
BHYVE_STATE_DIR,
@@ -268,6 +270,9 @@ virBhyveProcessStop(bhyveConnPtr driver,
return -1;
}
+ if (vm->privateData != NULL)
+ bhyveMonitorClose((bhyveMonitorPtr)vm->privateData);
+
/* First, try to kill 'bhyve' process */
if (virProcessKillPainfully(vm->pid, true) != 0)
VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %d)",
@@ -371,9 +376,12 @@ virBhyveProcessReconnect(virDomainObjPtr vm,
goto cleanup;
proc_argv = kvm_getargv(data->kd, kp, 0);
- if (proc_argv && proc_argv[0])
- if (STREQ(expected_proctitle, proc_argv[0]))
+ if (proc_argv && proc_argv[0]) {
+ if (STREQ(expected_proctitle, proc_argv[0])) {
ret = 0;
+ vm->privateData = bhyveMonitorOpen(vm, data->driver);
+ }
+ }
cleanup:
if (ret < 0) {
--
1.9.3
9 years, 11 months
[libvirt] [PATCH 0/3] qemu: support update graphic device persistently
by Wang Rui
We can change vnc password by using virDomainUpdateDeviceFlags API with
live flag. But it can't be changed with config flag.
1/3: improve the error number when update graphics failed
2/3: refactor the function qemuDomainFindGraphics for the future patch
3/3: support updating vnc/spice auth arguments persistently
Wang Rui (3):
qemu: report properer error number when change graphics failed
qemu: revise qemuDomainFindGraphics to be reused in the future patch
qemu: make persistent update of graphics device supported
src/conf/domain_conf.c | 3 ++-
src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.c | 23 +++++++++++------------
src/qemu/qemu_hotplug.h | 3 +++
4 files changed, 53 insertions(+), 14 deletions(-)
--
1.7.12.4
9 years, 11 months
[libvirt] [PATCHv2 0/9] Let libvirt manage a bridge's FDB
by Laine Stump
(Everyone - see the request for opinions/ideas towards the bottom)
The idea behind these patches is the following:
1) most virtual machines only have a single MAC address behind each
interface, and that MAC address is known by libvirt.
2) If we (i.e. libvirt) manually add an entry to the bridge's
forwarding database (fdb) for the MAC address associated with a port
on the bridge, we can turn off learning and unicast_flooding for that
port.
3) kernels starting with 3.15 (and actually working correctly starting
in kernel 3.17) will notice that all of a bridge's ports have flood
and learning turned off, and in that case will turn off promiscuous
mode on all ports. If all but one of the ports have flood/learning
turned off, then promiscuous will be turned off on that port (and left
on for all the other ports)
4) When (4) can be done, there is a measurable performance
advantage. It can also *kind of* help security, as it will prevent a
guest from doing anything useful if it changes its MAC address (but
won't prevent the guest from *sending* packets with a spoofed MAC
address).
NB: These only work with a fixed MAC address, and no vlan tags set in
the guest. Support for both of those will be coming.
HERE IS THE REQUEST FOR OPINIONS/IDEAS:
This V2 of the patchset addresses several issues brought up by jferlan
on the original series, and changes the name of the attribute from:
promiscLinks='yes|no'
to
fdb='learningWithFlood|managed'
I'm somewhat more happy with this new naming than the previous but
still looking for better ideas. It is closer to describing what the
new code really does, but "learningWithFlood" seems a bit long and
awkward, while I have been told that "fdb" is too short and
unrecognizeable (I will point out that 1) "fdb" is the same name used
by iproute2's "bridge" command, and 2) another bridge option, "stp" is
also a three letter acronym that will only be recognized by those
familiar with configuring an L2 bridge device or watching NASCAR on
Saturday afternoons (or whenever it's on - not a fan myself :-))
Here is a full list of every idea that either I or someone else has
come up with since I started thinking about this:
promiscLinks='yes|no'
After initially going with this for the v1 of the patchset, I later
decided against it, because it doesn't describe what libvirt is
doing, but only a *possible* side effect on *some* of the ports
connected to the bridge (in practice, it only happens to the physdev
port).
fdb='auto|managed'
I like "fdb" as the name of the attribute, because I think it really
gets at what libvirt is doing - it is taking over management of the
bridge's fdb (Forwarding Database), which ends up providing better
performance in several ways.
The problem with this proposal is that the two values are kind of
ambiguous - it's not clear which one is using the bridge module's
built-in management (I had figured this would be "auto"), and which
is telling libvirt to manage it ("managed"). (on the other hand,
the first option is "ignore the issue, let the underlying system
handle it", vs. "libvirt should manage it", so maybe it *is* a
reasonable choice).
Also, see the comment above about the perceived terseness and obscurity
of "fdb".
fdb='learningWithFlood|managed'
This alternate name was suggested by Michael Tsirkin as a way to
unambiguously indicate what was being done in the mode where libvirt
isn't involved in the fdb management. There was some criticism on
IRC that the name is *too* verbose, especially when contrasted with
"fdb".
fdb='learning|managed'
A suggested shortening of "learningWithFlood".
forwardingDatabase='blah'
A way to get around criticism of "fdb". I think this is too verbose,
but maybe I'm biased :-)
[specify each minor item that separately]
In order to manage the fdb by itself, libvirt disables "learning"
and "unicast_flood" for each tap device, enables "vlan_filtering"
for the bridge itself, then adds an fdb entry for each tap to the
bridge. There was one suggestion that, rather than trying to come
up with a single option that says "do all of these things", we
should instead make each of them separately configured. The problem
with this is that it makes it too easy to screw up the
configuration such that it causes sub-par performance, or simply
doesn't work at all. Part of libvirt's job is making it difficult
to screw up (or at least easier to succeed); for example, libvirt's
virtual networks do a lot of things automatically - create a
bridge, add iptables rules for filtering and NAT, run an instance
of dnsmasq - over time we've offered the option of tweaking more
and more of the details of this setup, but the initial aim was to
provide something that worked with as few required (or even
permitted) tweaks as possible.
I guess what I'm getting at is that I think it would be a mistake
to require turning on several different knobs (which individually
make little/no sense) in order to get the bridge into this higher
performing mode.
So - does anyone have an opinion of any of the options offered above,
or any ideas for alternates?
In the meantime, note that while the default is currently
"learningWithFlood" (meaning that that name is never actually directly
used/required anywhere, but is just in the RNG and the enum
definition), the intent of the people who developed this functionality
in the kernel is that eventually it will work so well that libvirt
management of the fdb can silently become the default with no visible
change in behavior.
NOTE: If you want to actually try out these patches, you'll need to
apply the following patch which I haven't yet pushed:
https://www.redhat.com/archives/libvir-list/2014-November/msg00948.html
Also, while the description of V1 stated that patches 08 and 09 were
not intended to be pushed yet, due to a problem they caused when
restarting libvirtd after an update, that problem has been solved, so
I now intend to push patches 08 and 09 along with the rest.
Laine Stump (9):
util: new functions for setting bridge and bridge port attributes
util: functions to manage bridge fdb (forwarding database)
conf: new network bridge device attribute fdb
network: save bridge name in ActualNetDef when actualType==network too
network: store network fdb setting in NetDef actual object
network: setup bridge devices for fdb='managed'
qemu: setup tap devices for fdb='managed'
qemu: always use virDomainNetGetActualBridgeName to get interface's
bridge
lxc: always use virDomainNetGetActualBridgeName to get interface's
bridge
docs/formatnetwork.html.in | 42 ++-
docs/schemas/network.rng | 9 +
src/conf/domain_conf.c | 129 ++++---
src/conf/domain_conf.h | 2 +
src/conf/network_conf.c | 50 ++-
src/conf/network_conf.h | 11 +
src/libvirt_private.syms | 11 +
src/lxc/lxc_driver.c | 32 +-
src/lxc/lxc_process.c | 32 +-
src/network/bridge_driver.c | 74 ++++
src/qemu/qemu_command.c | 53 ++-
src/qemu/qemu_hotplug.c | 60 +---
src/util/virnetdevbridge.c | 382 ++++++++++++++++++++-
src/util/virnetdevbridge.h | 44 ++-
tests/networkxml2xmlin/host-bridge-no-flood.xml | 6 +
.../nat-network-explicit-flood.xml | 21 ++
tests/networkxml2xmlout/host-bridge-no-flood.xml | 6 +
.../nat-network-explicit-flood.xml | 23 ++
tests/networkxml2xmltest.c | 2 +
19 files changed, 778 insertions(+), 211 deletions(-)
create mode 100644 tests/networkxml2xmlin/host-bridge-no-flood.xml
create mode 100644 tests/networkxml2xmlin/nat-network-explicit-flood.xml
create mode 100644 tests/networkxml2xmlout/host-bridge-no-flood.xml
create mode 100644 tests/networkxml2xmlout/nat-network-explicit-flood.xml
--
1.9.3
9 years, 11 months
[libvirt] [PATCH] Fix handling of whitespae in preprocessor macros for API generator
by Daniel P. Berrange
The apibuild.py script did not handle whitespace in preprocessor
macros, so it failed to detect constants declared with '# define'
instead of '#define'. Since we now correctly indent our public
header files, we have silently lost all constants from
libvirt-api.xml. This also caused us to not detect formatting
errors in constant docs
---
docs/apibuild.py | 8 ++++++++
include/libvirt/libvirt-host.h | 12 ++++++++----
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/docs/apibuild.py b/docs/apibuild.py
index 7549a63..9fa9361 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -459,6 +459,14 @@ class CLexer:
if line[0] == '#':
self.tokens = map((lambda x: ('preproc', x)),
string.split(line))
+
+ # We might have whitespace between the '#' and preproc
+ # macro name, so instead of having a single token element
+ # of '#define' we might end up with '#' and 'define'. This
+ # merges them back together
+ if self.tokens[0][1] == "#":
+ self.tokens[0] = ('preproc', self.tokens[0][1] + self.tokens[1][1])
+ self.tokens = self.tokens[:1] + self.tokens[2:]
break
l = len(line)
if line[0] == '"' or line[0] == "'":
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
index 5bd9563..53b529f 100644
--- a/include/libvirt/libvirt-host.h
+++ b/include/libvirt/libvirt-host.h
@@ -536,28 +536,32 @@ struct _virNodeMemoryStats {
*/
# define VIR_NODE_MEMORY_SHARED_PAGES_SHARING "shm_pages_sharing"
-/* VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED:
+/*
+ * VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED:
*
* Macro for typed parameter that represents how many pages unique
* but repeatedly checked for merging.
*/
# define VIR_NODE_MEMORY_SHARED_PAGES_UNSHARED "shm_pages_unshared"
-/* VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE:
+/*
+ * VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE:
*
* Macro for typed parameter that represents how many pages changing
* too fast to be placed in a tree.
*/
# define VIR_NODE_MEMORY_SHARED_PAGES_VOLATILE "shm_pages_volatile"
-/* VIR_NODE_MEMORY_SHARED_FULL_SCAN:
+/*
+ * VIR_NODE_MEMORY_SHARED_FULL_SCANS:
*
* Macro for typed parameter that represents how many times all
* mergeable areas have been scanned.
*/
# define VIR_NODE_MEMORY_SHARED_FULL_SCANS "shm_full_scans"
-/* VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES:
+/*
+ * VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES:
*
* Macro for typed parameter that represents whether pages from
* different NUMA nodes can be merged. The parameter has type int,
--
2.1.0
9 years, 11 months
[libvirt] [PATCH] docs: Create html documentation even if XHTML1 DTD is not available to validate
by Ian Campbell
On a Debian system lacking the w3c-dtd-xhtml package the build fails
with:
$ make -C docs/ formatcaps.html
make: Entering directory '/local/scratch/ianc/devel/libvirt.git/docs'
Generating formatcaps.html.tmp
I/O error : Attempt to load network entity http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd
formatcaps.html.in:2: warning: failed to load external entity "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"
C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"
^
I/O error : Attempt to load network entity http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd
../docs/sitemap.html.in:2: warning: failed to load external entity "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"
C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"
^
missing XHTML1 DTD
rm formatcaps.html.tmp
make: Leaving directory '/local/scratch/ianc/devel/libvirt.git/docs'
$ ls docs/formatcaps*
docs/formatcaps.html.in
https://www.redhat.com/archives/libvir-list/2009-November/msg00413.html
suggests that the XHTML1 DTD should not be a hard requirement and the
docs should be generated but not validated if it is not available.
Therefore when the DTD is not available arrange for the .html.tmp file
to be propagated to the .html output.
Signed-off-by: Ian Campbell <ian.campbell(a)citrix.com>
Cc: Daniel Veillard <veillard(a)redhat.com>
---
docs/Makefile.am | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 5485ee9..c5ba688 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -232,7 +232,7 @@ internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in
SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \
$(XMLLINT) --catalogs --nonet --format --valid $< > $(srcdir)/$@ \
|| { rm $(srcdir)/$@ && exit 1; }; \
- else echo "missing XHTML1 DTD" ; fi ; fi
+ else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
%.php.tmp: %.php.in site.xsl page.xsl sitemap.html.in
@if [ -x $(XSLTPROC) ] ; then \
@@ -258,7 +258,7 @@ html/index.html: libvirt-api.xml newapi.xsl page.xsl sitemap.html.in
> /dev/null ; then \
SGML_CATALOG_FILES='$(XML_CATALOG_FILE)' \
$(XMLLINT) --catalogs --nonet --valid --noout $(srcdir)/html/*.html ; \
- else echo "missing XHTML1 DTD" ; fi ; fi
+ else echo "missing XHTML1 DTD"; cat $< > $(srcdir)/$@ ; fi ; fi
$(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml $(devhelpxsl)
$(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \
--
2.1.1
9 years, 11 months
[libvirt] [libvirt-perl][PATCH] Fix constants for VIR_DOMAIN_GET_ALL_STATS_*
by Zhe Peng
Constants of GET_ALL_STATS_* incorrect,this patch fix them in Domain.pm
---
lib/Sys/Virt/Domain.pm | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm
index 5680afa..e5ce2dd 100644
--- a/lib/Sys/Virt/Domain.pm
+++ b/lib/Sys/Virt/Domain.pm
@@ -3287,39 +3287,39 @@ bulk domain stats from C<Sys::Virt::get_all_domain_stats>.
=over 4
-=item Sys::Virt::GET_ALL_STATS_ACTIVE
+=item Sys::Virt::Domain::GET_ALL_STATS_ACTIVE
Include stats for active domains
-=item Sys::Virt::GET_ALL_STATS_INACTIVE
+=item Sys::Virt::Domain::GET_ALL_STATS_INACTIVE
Include stats for inactive domains
-=item Sys::Virt::GET_ALL_STATS_OTHER
+=item Sys::Virt::Domain::GET_ALL_STATS_OTHER
Include stats for other domains
-=item Sys::Virt::GET_ALL_STATS_PAUSED
+=item Sys::Virt::Domain::GET_ALL_STATS_PAUSED
Include stats for paused domains
-=item Sys::Virt::GET_ALL_STATS_PERSISTENT
+=item Sys::Virt::Domain::GET_ALL_STATS_PERSISTENT
Include stats for persistent domains
-=item Sys::Virt::GET_ALL_STATS_RUNNING
+=item Sys::Virt::Domain::GET_ALL_STATS_RUNNING
Include stats for running domains
-=item Sys::Virt::GET_ALL_STATS_SHUTOFF
+=item Sys::Virt::Domain::GET_ALL_STATS_SHUTOFF
Include stats for shutoff domains
-=item Sys::Virt::GET_ALL_STATS_TRANSIENT
+=item Sys::Virt::Domain::GET_ALL_STATS_TRANSIENT
Include stats for transient domains
-=item Sys::Virt::GET_ALL_STATS_ENFORCE_STATS
+=item Sys::Virt::Domain::GET_ALL_STATS_ENFORCE_STATS
Require that all requested stats fields are returned
--
1.9.0
9 years, 11 months
[libvirt] [PATCH] network: don't allow multiple dhcp sections
by Kyle DeFrancia
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=907779
A <dhcp> element can exist in only one IPv4 address and one IPv6
address per network. This patch enforces that in virNetworkUpdate.
---
src/conf/network_conf.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 067334e..a914962 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3472,6 +3472,31 @@ virNetworkIpDefByIndex(virNetworkDefPtr def, int
parentIndex) }
static int
+virNetworkDefUpdateCheckMultiDHCP(virNetworkDefPtr def,
+ virNetworkIpDefPtr ipdef)
+{
+ int family = VIR_SOCKET_ADDR_FAMILY(&ipdef->address);
+ size_t i;
+ virNetworkIpDefPtr ip;
+
+ for (i = 0;
+ (ip = virNetworkDefGetIpByIndex(def, family, i));
+ i++) {
+ if (ip != ipdef) {
+ if (ip->nranges || ip->nhosts) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("dhcp is supported only for a "
+ "single %s address on each network"),
+ (family == AF_INET) ? "IPv4" : "IPv6");
+ return -1;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int
virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def,
unsigned int command,
int parentIndex,
@@ -3536,6 +3561,9 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr
def, } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
(command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+ if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
+ goto cleanup;
+
/* log error if an entry with same name/address/ip already
exists */ for (i = 0; i < ipdef->nhosts; i++) {
if ((host.mac &&
@@ -3643,6 +3671,9 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr
def, if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
(command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+ if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
+ goto cleanup;
+
if (i < ipdef->nranges) {
char *startip = virSocketAddrFormat(&range.start);
char *endip = virSocketAddrFormat(&range.end);
--
2.1.0
9 years, 11 months
