[libvirt] [RFC PATCH 1/5] Qemu Monitor API entry point.
Daniel P. Berrange
berrange at redhat.com
Fri Apr 16 11:16:52 UTC 2010
On Tue, Apr 13, 2010 at 02:36:46PM -0400, Chris Lalancette wrote:
> Signed-off-by: Chris Lalancette <clalance at redhat.com>
> ---
> include/libvirt/Makefile.am | 1 +
> include/libvirt/libvirt_qemu.h | 30 ++++++++++++
> src/Makefile.am | 11 ++++-
> src/driver.h | 5 ++
> src/esx/esx_driver.c | 1 +
> src/internal.h | 1 +
> src/libvirt_qemu.c | 96 ++++++++++++++++++++++++++++++++++++++++
> src/lxc/lxc_driver.c | 1 +
> src/opennebula/one_driver.c | 1 +
> src/openvz/openvz_driver.c | 1 +
> src/phyp/phyp_driver.c | 1 +
> src/qemu/qemu_driver.c | 1 +
> src/remote/remote_driver.c | 1 +
> src/test/test_driver.c | 1 +
> src/uml/uml_driver.c | 1 +
> src/vbox/vbox_tmpl.c | 1 +
> src/xen/xen_driver.c | 1 +
> src/xenapi/xenapi_driver.c | 1 +
> 18 files changed, 155 insertions(+), 1 deletions(-)
> create mode 100644 include/libvirt/libvirt_qemu.h
> create mode 100644 src/libvirt_qemu.c
>
> diff --git a/include/libvirt/Makefile.am b/include/libvirt/Makefile.am
> index 8589dc5..ac5f32c 100644
> --- a/include/libvirt/Makefile.am
> +++ b/include/libvirt/Makefile.am
> @@ -3,6 +3,7 @@
> virincdir = $(includedir)/libvirt
>
> virinc_HEADERS = libvirt.h \
> + libvirt_qemu.h \
> virterror.h
Minor nitpick - I'd prefer an '-' instead of '_'
> diff --git a/include/libvirt/libvirt_qemu.h b/include/libvirt/libvirt_qemu.h
> new file mode 100644
> index 0000000..7d78a7f
> --- /dev/null
> +++ b/include/libvirt/libvirt_qemu.h
> @@ -0,0 +1,30 @@
> +/* -*- c -*-
> + * libvirt_qemu.h:
> + * Summary: qemu specific interfaces
> + * Description: Provides the interfaces of the libvirt library to handle
> + * qemu specific methods
> + *
> + * Copy: Copyright (C) 2010 Red Hat, Inc.
> + *
> + * See COPYING.LIB for the License of this software
> + *
> + * Author: Chris Lalancette <clalance at redhat.com>
> + */
> +
> +#ifndef __VIR_QEMU_H__
> +#define __VIR_QEMU_H__
> +
> +#include "libvirt.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +int virQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result,
> + unsigned int flags);
For naming, we should continue to keep the object name as the prefix, since
this makes things a little easier for python bindings. eg
virDomainQemuMonitorRunCommand
> diff --git a/src/Makefile.am b/src/Makefile.am
> index d54e6d0..c01c94e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -32,7 +32,7 @@ if WITH_NETWORK
> UUID=$(shell uuidgen 2>/dev/null)
> endif
>
> -lib_LTLIBRARIES = libvirt.la
> +lib_LTLIBRARIES = libvirt.la libvirt_qemu.la
And a '-' here too :-)
>
> moddir = $(libdir)/libvirt/drivers
> mod_LTLIBRARIES =
> @@ -945,6 +945,15 @@ libvirt_test_la_LIBADD = $(libvirt_la_LIBADD)
> libvirt_test_la_LDFLAGS = $(test_LDFLAGS)
> libvirt_test_la_CFLAGS = $(COVERAGE_CFLAGS)
>
> +libvirt_qemu_la_SOURCES = libvirt_qemu.c util/threads.c util/threads.h \
> + util/threads-pthread.h \
> + util/threads-win32.h \
> + util/virterror.c \
> + util/virterror_internal.h
Do we need to directly recompile those pieces here ? I think
if you just add 'libvirt.la' to the libvirt_qemu_la_LIBADD
it should be able to link to the existing built functions
> +
> +libvirt_qemu_la_LDFLAGS = $(libvirt_la_LDFALGS)
> +libvirt_qemu_la_CFLAGS = $(COVERAGE_CFLAGS)
> +
>
> libexec_PROGRAMS =
>
> diff --git a/src/driver.h b/src/driver.h
> index f8db9c1..651653d 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -448,6 +448,10 @@ typedef int
> (*virDrvDomainSnapshotDelete)(virDomainSnapshotPtr snapshot,
> unsigned int flags);
>
> +typedef int
> + (*virDrvQemuMonitorCommand)(virDomainPtr domain, const char *cmd,
> + char **result, unsigned int flags);
> +
>
> /**
> * _virDriver:
> @@ -558,6 +562,7 @@ struct _virDriver {
> virDrvDomainSnapshotCurrent domainSnapshotCurrent;
> virDrvDomainRevertToSnapshot domainRevertToSnapshot;
> virDrvDomainSnapshotDelete domainSnapshotDelete;
> + virDrvQemuMonitorCommand qemuMonitorCommand;
> };
As mentioned in my reply to your question in patch 0, I think we
could possibly ignore the drivers here, and just call directly to
the QEMU function ?
> diff --git a/src/internal.h b/src/internal.h
> index 2e73210..57dc660 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -22,6 +22,7 @@
> # include "gettext.h"
>
> # include "libvirt/libvirt.h"
> +# include "libvirt/libvirt_qemu.h"
> # include "libvirt/virterror.h"
It'd be better to avoid that here if possible - just have it included
in libvirt_qemu.c so we avoid polluting all the drivers with the QEMU
specific header
> # include "libvirt_internal.h"
> diff --git a/src/libvirt_qemu.c b/src/libvirt_qemu.c
> new file mode 100644
> index 0000000..9c72fe2
> --- /dev/null
> +++ b/src/libvirt_qemu.c
> @@ -0,0 +1,96 @@
> +#include <config.h>
> +
> +#include "virterror_internal.h"
> +#include "logging.h"
> +#include "datatypes.h"
> +#include "driver.h"
> +
> +/**
> + * virLibConnError:
> + * @conn: the connection if available
> + * @error: the error number
> + * @info: extra information string
> + *
> + * Handle an error at the connection level
> + */
> +static void
> +virLibConnError(virConnectPtr conn, virErrorNumber error, const char *info)
> +{
> + const char *errmsg;
> +
> + if (error == VIR_ERR_OK)
> + return;
> +
> + errmsg = virErrorMsg(error, info);
> + virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, error, VIR_ERR_ERROR,
> + errmsg, info, NULL, 0, 0, errmsg, info);
> +}
> +
> +/**
> + * virLibDomainError:
> + * @domain: the domain if available
> + * @error: the error number
> + * @info: extra information string
> + *
> + * Handle an error at the connection level
> + */
> +static void
> +virLibDomainError(virDomainPtr domain, virErrorNumber error,
> + const char *info)
> +{
> + virConnectPtr conn = NULL;
> + const char *errmsg;
> +
> + if (error == VIR_ERR_OK)
> + return;
> +
> + errmsg = virErrorMsg(error, info);
> + if (error != VIR_ERR_INVALID_DOMAIN) {
> + conn = domain->conn;
> + }
> + virRaiseError(conn, domain, NULL, VIR_FROM_DOM, error, VIR_ERR_ERROR,
> + errmsg, info, NULL, 0, 0, errmsg, info);
> +}
> +
> +int
> +virQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + DEBUG("domain=%p, cmd=%s, result=%p, flags=%u", domain, cmd, result, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virDispatchError(NULL);
> + return -1;
> + }
> +
> + conn = domain->conn;
> +
> + if (result == NULL) {
> + virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> + if (conn->flags & VIR_CONNECT_RO) {
> + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + goto error;
> + }
> +
> + if (conn->driver->qemuMonitorCommand) {
> + int ret;
> + ret = conn->driver->qemuMonitorCommand(domain, cmd, result, flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + virDispatchError(conn);
> + return -1;
> +}
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list