[libvirt] [PATCH v1 6/6] ivshmem: add start param to server attribute
Martin Kletzander
mkletzan at redhat.com
Tue Aug 26 09:33:09 UTC 2014
[Cc'ing David due to some questions/ideas about the ivshmem server]
On Fri, Aug 22, 2014 at 12:47:05PM +0200, Maxime Leroy wrote:
>With the new start param, we are able to start the
>ivshmem-server.
>
>With this xml:
>
> <shmem name='ivshmem0' model='ivshmem'>
> <server path='/tmp/ivshmem0.sock' start='yes'/>
> <size unit='M'>32</size>
> <msi vectors='32' ioeventfd='on'/>
> </shmem>
>
>Libvirt will execute an ivshmem-server:
>
> /usr/bin/ivshmem-server -m ivshmem0 -S /tmp/ivshmem0.sock \
> -p /var/run/libvirt/ivshmem-server/ivshd-ivshmem0.pid -n 32
>
>Signed-off-by: Maxime Leroy <maxime.leroy at 6wind.com>
>---
> configure.ac | 4 +
> docs/formatdomain.html.in | 5 +-
> docs/schemas/domaincommon.rng | 3 +
> po/POTFILES.in | 1 +
> src/Makefile.am | 1 +
> src/conf/domain_conf.c | 19 +++-
> src/conf/domain_conf.h | 1 +
> src/libvirt_private.syms | 5 +
> src/qemu/qemu_command.c | 7 ++
> src/qemu/qemu_process.c | 10 ++
> src/util/virivshmemserver.c | 141 ++++++++++++++++++++++++
> src/util/virivshmemserver.h | 28 +++++
> tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 2 +-
> 13 files changed, 223 insertions(+), 4 deletions(-)
> create mode 100644 src/util/virivshmemserver.c
> create mode 100644 src/util/virivshmemserver.h
>
[...]
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 9fcceae..fe2fc1a 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -34,6 +34,7 @@
> #include "virarch.h"
> #include "virerror.h"
> #include "virfile.h"
>+#include "virivshmemserver.h"
> #include "virnetdev.h"
> #include "virstring.h"
> #include "virtime.h"
>@@ -5120,6 +5121,12 @@ qemuBuildIvshmemCommandLine(virCommandPtr cmd,
> return -1;
> virCommandAddArg(cmd, devstr);
> VIR_FREE(devstr);
>+
>+ if (ivshmem->server.start == VIR_TRISTATE_BOOL_YES) {
>+ if (virStartIvshmemServer(dev->name, ivshmem->server.path,
>+ ivshmem->size, ivshmem->msi.vectors))
>+ return -1;
>+ }
> }
What if the server is already running? Either 2 domains have server
start='yes' or the user started it already. We should not fail in
these cases, I guess.
>
> return 0;
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index baa866a..aaf03a3 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -67,6 +67,7 @@
> #include "virstring.h"
> #include "virhostdev.h"
> #include "storage/storage_driver.h"
>+#include "virivshmemserver.h"
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
>@@ -4684,6 +4685,15 @@ void qemuProcessStop(virQEMUDriverPtr driver,
> }
> }
>
>+ /* stop runnning ivshmem server */
>+ for (i = 0; i < vm->def->nshmems; i++) {
>+ virDomainShmemDefPtr shmem = vm->def->shmems[i];
>+ if (shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM &&
>+ shmem->data.ivshmem.server.start == VIR_TRISTATE_BOOL_YES) {
>+ virStopIvshmemServer(shmem->name);
>+ }
>+ }
>+
It would be great if the domain with server start='yes' didn't have to
be started first and killed last. Or if we could just signal the
server (let's say SIGUSR1) that it should unlink the shmem and quit if
no domains are connected. That way we could just send a signal to the
server whenever any domain is disconnecting (maybe some check that no
domain is being started might be good too ;) ).
> vm->taint = 0;
> vm->pid = -1;
> virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
>diff --git a/src/util/virivshmemserver.c b/src/util/virivshmemserver.c
>new file mode 100644
>index 0000000..89130b1
>--- /dev/null
>+++ b/src/util/virivshmemserver.c
>@@ -0,0 +1,141 @@
>+/*
>+ * Copyright 2014 6WIND S.A.
>+ *
>+ * 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/>.
>+ *
>+ */
>+
>+#include <config.h>
>+#include <string.h>
>+#include <signal.h>
>+
>+#include "virivshmemserver.h"
>+#include "vircommand.h"
>+#include "viralloc.h"
>+#include "virerror.h"
>+#include "virlog.h"
>+#include "virstring.h"
>+#include "virfile.h"
>+#include "virpidfile.h"
>+#include "virprocess.h"
>+#include "configmake.h"
>+
>+#define VIR_FROM_THIS VIR_FROM_NONE
>+
>+VIR_LOG_INIT("util.ivshmemserver")
>+
>+#define IVSHMEM_STATE_DIR LOCALSTATEDIR "/run/libvirt/ivshmem-server"
>+
>+static char *virIvshmemServerPidFileBasename(const char *shm_name) {
>+ char *pidfilebase;
>+
>+ ignore_value(virAsprintf(&pidfilebase, "ivshd-%s", shm_name));
>+ return pidfilebase;
>+}
>+
>+/**
>+ * virStartIvshmemServer:
>+ * @shm_name: the name of the shared memory
>+ * @unix_socket_path: the path to the socket unix file
>+ * @size: the size of the shared memory (optionnal)
>+ * @vectors: the number of vectors (optionnal)
>+ *
>+ * Start an Ivshmem Server
>+ *
>+ * Returns 0 in case of success or -1 in case of failure.
>+ */
>+int
>+virStartIvshmemServer(const char *shm_name, const char *unix_socket_path,
>+ size_t size, unsigned vectors)
>+{
>+ char *ivshmserver_pidbase = NULL;
>+ char *pidfile = NULL;
>+ virCommandPtr cmd = NULL;
>+ int ret = -1;
>+
>+ if (!virFileIsExecutable(IVSHMEMSERVER)) {
>+ virReportSystemError(errno, _(" %s is not available"),
>+ IVSHMEMSERVER);
IVSHMEM_SERVER (with underscore) would be a bit more readable. I'm
not sure new file is needed for the stop/start, but shouldn't hurt.
>+ goto cleanup;
>+ }
>+
>+ if (virFileMakePath(IVSHMEM_STATE_DIR) < 0) {
>+ virReportSystemError(errno,
>+ _("cannot create directory %s"),
>+ IVSHMEM_STATE_DIR);
>+ goto cleanup;
>+ }
>+
>+ /* construct pidfile name */
>+ if (!(ivshmserver_pidbase = virIvshmemServerPidFileBasename(shm_name)))
>+ goto cleanup;
>+ if (!(pidfile = virPidFileBuildPath(IVSHMEM_STATE_DIR, ivshmserver_pidbase)))
>+ goto cleanup;
>+
>+ cmd = virCommandNewArgList(IVSHMEMSERVER, "-m", shm_name,
>+ "-S", unix_socket_path, "-p", pidfile, NULL);
>+
>+ if (size)
>+ virCommandAddArgFormat(cmd, "-l %zu", size);
>+
>+ if (vectors)
>+ virCommandAddArgFormat(cmd, "-n %u", vectors);
>+
>+ virCommandSetPidFile(cmd, pidfile);
>+ virCommandDaemonize(cmd);
>+
>+ if (virCommandRun(cmd, NULL) < 0) {
>+ virReportSystemError(VIR_ERR_INTERNAL_ERROR,
>+ _("Unable to start %s for %s shm"),
>+ IVSHMEMSERVER, shm_name);
>+ goto cleanup;
>+ }
>+ ret = 0;
>+ cleanup:
>+ virCommandFree(cmd);
>+ VIR_FREE(ivshmserver_pidbase);
>+ VIR_FREE(pidfile);
>+ return ret;
>+}
>+
>+/**
>+ * virStopIvshmemServer:
>+ * @shm_name: the name of the shared memory
>+ *
>+ * Stop an Ivshmem Server
>+ *
>+ * Returns 0 in case of success or -1 in case of failure.
>+ */
>+int
>+virStopIvshmemServer(const char *shm_name)
>+{
>+ char *ivshmserver_pidbase = NULL;
>+ pid_t ivshmserver_pid;
>+ int ret = -1;
>+
>+ /* get pid of the ivshmem server */
>+ if (!(ivshmserver_pidbase = virIvshmemServerPidFileBasename(shm_name)))
>+ goto cleanup;
>+ if (virPidFileRead(IVSHMEM_STATE_DIR, ivshmserver_pidbase,
>+ &ivshmserver_pid))
>+ goto cleanup;
>+
>+ virProcessKill(ivshmserver_pid, SIGTERM);
>+ virPidFileDelete(IVSHMEM_STATE_DIR, ivshmserver_pidbase);
The pidfile support could be added to the server too, maybe...
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140826/2cf5e876/attachment-0001.sig>
More information about the libvir-list
mailing list