Markus Groß wrote:
> ---
> configure.ac | 2 +
> daemon/Makefile.am | 3 +
> src/Makefile.am | 8 ++-
> src/libxl/libxl_driver.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 102 insertions(+), 5 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 12bf0f6..3519011 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -607,6 +607,8 @@ AM_CONDITIONAL([WITH_XEN], [test "$with_xen" = "yes"])
> AC_SUBST([XEN_CFLAGS])
> AC_SUBST([XEN_LIBS])
>
> +AM_CONDITIONAL([WITH_XENXS], [test "$with_libxl" = "yes" || test "$with_xen" = "yes"])
> +
> dnl
> dnl check for kernel headers required by xen_inotify
> dnl
> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
> index 9e3a557..4344127 100644
> --- a/daemon/Makefile.am
> +++ b/daemon/Makefile.am
> @@ -108,6 +108,9 @@ endif
>
> if WITH_LIBXL
> libvirtd_LDADD += ../src/libvirt_driver_libxl.la
> + libvirtd_LDADD += ../src/libvirt_xenxs.la
> + libvirtd_LDADD += ../src/libvirt_util.la
> + libvirtd_LDADD += ../src/libvirt_conf.la
> endif
>
I'd like to get another opinion on this hunk. Although I cant find any
issues with it, I'd just like to ensure there is not a more appropriate
way to add these libs.
>
> if WITH_UML
> diff --git a/src/Makefile.am b/src/Makefile.am
> index c3729a6..ae0c6ed 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -474,7 +474,7 @@ libvirt_vmx_la_CFLAGS = \
> libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
> endif
>
> -if WITH_XEN
> +if WITH_XENXS
> noinst_LTLIBRARIES += libvirt_xenxs.la
> libvirt_la_BUILT_LIBADD += libvirt_xenxs.la
> libvirt_xenxs_la_CFLAGS = \
> @@ -704,8 +704,10 @@ noinst_LTLIBRARIES += libvirt_driver_libxl.la
> # Stateful, so linked to daemon instead
> #libvirt_la_BUILT_LIBADD += libvirt_driver_libxl.la
> endif
> -libvirt_driver_libxl_la_CFLAGS = $(LIBXL_CFLAGS) \
> - -I@top_srcdir@/src/conf $(AM_CFLAGS)
> +libvirt_driver_libxl_la_CFLAGS = $(LIBXL_CFLAGS) \
> + -I@top_srcdir@/src/conf \
> + -I@top_srcdir@/src/xenxs \
> + $(AM_CFLAGS)
> libvirt_driver_libxl_la_LDFLAGS = $(AM_LDFLAGS)
> libvirt_driver_libxl_la_LIBADD = $(LIBXL_LIBS)
> if WITH_DRIVER_MODULES
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 32bf12e..b05cd77 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -33,6 +33,7 @@
> #include "internal.h"
> #include "logging.h"
> #include "virterror_internal.h"
> +#include "conf.h"
> #include "datatypes.h"
> #include "files.h"
> #include "memory.h"
> @@ -41,6 +42,7 @@
> #include "command.h"
> #include "libxl_driver.h"
> #include "libxl_conf.h"
> +#include "xen_xm.h"
>
>
> #define VIR_FROM_THIS VIR_FROM_LIBXL
> @@ -51,6 +53,8 @@
> #define LIBXL_DOM_REQ_CRASH 3
> #define LIBXL_DOM_REQ_HALT 4
>
> +#define LIBXL_CONFIG_FORMAT_XM "xen-xm"
> +
> static libxlDriverPrivatePtr libxl_driver = NULL;
>
>
> @@ -1620,6 +1624,92 @@ libxlDomainDumpXML(virDomainPtr dom, int flags)
> return ret;
> }
>
> +static char *
> +libxlDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat,
> + const char * nativeConfig,
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
> + libxlDriverPrivatePtr driver = conn->privateData;
> + const libxl_version_info *ver_info;
> + virDomainDefPtr def = NULL;
> + virConfPtr conf = NULL;
> + char *xml = NULL;
> +
> + if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
> + libxlError(VIR_ERR_INVALID_ARG,
> + _("unsupported config type %s"), nativeFormat);
> + goto cleanup;
> + }
> +
> + if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) {
> + VIR_ERROR0(_("cannot get version information from libxenlight"));
> + goto cleanup;
> + }
> +
> + if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
> + goto cleanup;
> +
> + if (!(def = xenParseXM(conf, ver_info->xen_version_major, driver->caps))) {
> + libxlError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing xm config failed"));
> + goto cleanup;
> + }
> +
> + xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE);
> +
> +cleanup:
> + virDomainDefFree(def);
> + if (conf)
> + virConfFree(conf);
>
At first glance this seems like a useless if-before-free, but I see that
virConfFree emits an error if conf is NULL.
ACK, assuming someone else agrees with the daemon/Makefile.am hunk.
Regards,
Jim
> + return xml;
> +}
> +
> +#define MAX_CONFIG_SIZE (1024 * 65)
> +static char *
> +libxlDomainXMLToNative(virConnectPtr conn, const char * nativeFormat,
> + const char * domainXml,
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
> + libxlDriverPrivatePtr driver = conn->privateData;
> + const libxl_version_info *ver_info;
> + virDomainDefPtr def = NULL;
> + virConfPtr conf = NULL;
> + int len = MAX_CONFIG_SIZE;
> + char *ret = NULL;
> +
> + if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
> + libxlError(VIR_ERR_INVALID_ARG,
> + _("unsupported config type %s"), nativeFormat);
> + goto cleanup;
> + }
> +
> + if ((ver_info = libxl_get_version_info(&driver->ctx)) == NULL) {
> + VIR_ERROR0(_("cannot get version information from libxenlight"));
> + goto cleanup;
> + }
> +
> + if (!(def = virDomainDefParseString(driver->caps, domainXml, 0)))
> + goto cleanup;
> +
> + if (!(conf = xenFormatXM(conn, def, ver_info->xen_version_major)))
> + goto cleanup;
> +
> + if (VIR_ALLOC_N(ret, len) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (virConfWriteMem(ret, &len, conf) < 0) {
> + VIR_FREE(ret);
> + goto cleanup;
> + }
> +
> +cleanup:
> + virDomainDefFree(def);
> + if (conf)
> + virConfFree(conf);
> + return ret;
> +}
> +
> static int
> libxlListDefinedDomains(virConnectPtr conn,
> char **const names, int nnames)
> @@ -1978,8 +2068,8 @@ static virDriver libxlDriver = {
> NULL, /* domainGetSecurityLabel */
> NULL, /* nodeGetSecurityModel */
> libxlDomainDumpXML, /* domainDumpXML */
> - NULL, /* domainXmlFromNative */
> - NULL, /* domainXmlToNative */
> + libxlDomainXMLFromNative, /* domainXmlFromNative */
> + libxlDomainXMLToNative, /* domainXmlToNative */
> libxlListDefinedDomains, /* listDefinedDomains */
> libxlNumDefinedDomains, /* numOfDefinedDomains */
> libxlDomainCreate, /* domainCreate */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list