4:46 p.m.
This series contains several improvements for the libxl driver.
Patches 1 and 2 are primarily code movement, putting code in
files that are more consistent with other drivers.
Patches 3 through 12 improve locking of the libxlDriverPrivate
struct, similar to the work done in the QEMU and LXC drivers
Obviously post-1.1.2 material, but would be nice to get it in
soon thereafter for testing throughout the 1.1.3 cycle...
Jim Fehlig (12):
libxl: Move detection of autoballoon to libxl_conf
libxl: Introduce libxl_domain.[ch]
libxl: Earlier detection of not running on Xen
libxl: Add libxl_version_info to libxlDriverPrivate
libxl: libxl: Use per-domain ctx in libxlMakeDomCreateInfo
libxl: User per-domain ctx in libxlDomainGetInfo
libxl: Introduce libxlDriverConfig object
libxl: Use atomic ops for driver->nactive
libxl: Add comments to libxlDriverPrivate fields
libxl: Move driver lock/unlock to libxl_conf
libxl: Remove unnecessary driver locking
libxl: Add libxlDomObjFromDomain
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libxl/libxl_conf.c | 149 +++++-
src/libxl/libxl_conf.h | 87 +--
src/libxl/libxl_domain.c | 469 ++++++++++++++++
src/libxl/libxl_domain.h | 61 +++
src/libxl/libxl_driver.c | 1329 ++++++++++------------------------------------
7 files changed, 1010 insertions(+), 1087 deletions(-)
create mode 100644 src/libxl/libxl_domain.c
create mode 100644 src/libxl/libxl_domain.h
--
1.8.1.4
4:46 p.m.
New subject: [libvirt] [PATCH 01/12] libxl: Move detection of autoballoon to libxl_conf
Detecting whether or not to autoballoon is configuration related,
so move the code to libxl_conf.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 22 ++++++++++++++++++++++
src/libxl/libxl_conf.h | 3 +++
src/libxl/libxl_driver.c | 25 +------------------------
3 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8129c7f..f8937a4 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -978,6 +978,28 @@ error:
return -1;
}
+bool
+libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
+{
+ const libxl_version_info *info;
+ regex_t regex;
+ int ret;
+
+ info = libxl_get_version_info(driver->ctx);
+ if (!info)
+ return true; /* default to on */
+
+ ret = regcomp(®ex,
+ "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+ REG_NOSUB | REG_EXTENDED);
+ if (ret)
+ return true;
+
+ ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ regfree(®ex);
+ return ret == REG_NOMATCH;
+}
+
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx)
{
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 9d72b72..0498012 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -120,6 +120,9 @@ struct _libxlSavefileHeader {
uint32_t unused[10];
};
+bool
+libxlGetAutoballoonConf(libxlDriverPrivatePtr driver);
+
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9fb4fa5..91beb30 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1207,29 +1207,6 @@ libxlStateCleanup(void)
return 0;
}
-static bool
-libxlGetAutoballoon(libxlDriverPrivatePtr driver)
-{
- const libxl_version_info *info;
- regex_t regex;
- int ret;
-
- info = libxl_get_version_info(driver->ctx);
- if (!info)
- return true; /* default to on */
-
- ret = regcomp(®ex,
- "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
- REG_NOSUB | REG_EXTENDED);
- if (ret)
- return true;
-
- ret = regexec(®ex, info->commandline, 0, NULL, 0);
- regfree(®ex);
- return ret == REG_NOMATCH;
-}
-
-
static int
libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
@@ -1376,7 +1353,7 @@ libxlStateInitialize(bool privileged,
}
/* setup autoballoon */
- libxl_driver->autoballoon = libxlGetAutoballoon(libxl_driver);
+ libxl_driver->autoballoon = libxlGetAutoballoonConf(libxl_driver);
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 01/12] libxl: Move detection of autoballoon to libxl_conf
On 30.08.2013 23:46, Jim Fehlig wrote:
Detecting whether or not to autoballoon is configuration related,
so move the code to libxl_conf.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 22 ++++++++++++++++++++++
src/libxl/libxl_conf.h | 3 +++
src/libxl/libxl_driver.c | 25 +------------------------
3 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8129c7f..f8937a4 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -978,6 +978,28 @@ error:
return -1;
}
+bool
+libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
+{
+ const libxl_version_info *info;
+ regex_t regex;
+ int ret;
+
+ info = libxl_get_version_info(driver->ctx);
+ if (!info)
+ return true; /* default to on */
+
+ ret = regcomp(®ex,
+ "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+ REG_NOSUB | REG_EXTENDED);
+ if (ret)
+ return true;
Pre-existing, but if we fail to compile the regex, shouldn't we error out?
+
+ ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ regfree(®ex);
+ return ret == REG_NOMATCH;
+}
+
ACK
Michal
5:47 p.m.
New subject: [libvirt] [PATCH 01/12] libxl: Move detection of autoballoon to libxl_conf
Michal Privoznik wrote:
On 30.08.2013 23:46, Jim Fehlig wrote:
> Detecting whether or not to autoballoon is configuration related,
> so move the code to libxl_conf.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_conf.c | 22 ++++++++++++++++++++++
> src/libxl/libxl_conf.h | 3 +++
> src/libxl/libxl_driver.c | 25 +------------------------
> 3 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 8129c7f..f8937a4 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -978,6 +978,28 @@ error:
> return -1;
> }
>
> +bool
> +libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
> +{
> + const libxl_version_info *info;
> + regex_t regex;
> + int ret;
> +
> + info = libxl_get_version_info(driver->ctx);
> + if (!info)
> + return true; /* default to on */
> +
> + ret = regcomp(®ex,
> + "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
> + REG_NOSUB | REG_EXTENDED);
> + if (ret)
> + return true;
>
Pre-existing, but if we fail to compile the regex, shouldn't we error out?
I think so, especially since patch 7 makes similar changes, e.g. bailing
out on libxl_get_version_info() failures.
I'll send a separate patch to fix this pre-existing issue.
Regards,
Jim
6:20 a.m.
New subject: [libvirt] [PATCH 01/12] libxl: Move detection of autoballoon to libxl_conf
On Fri, Aug 30, 2013 at 03:46:47PM -0600, Jim Fehlig wrote:
Detecting whether or not to autoballoon is configuration related,
so move the code to libxl_conf.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 22 ++++++++++++++++++++++
src/libxl/libxl_conf.h | 3 +++
src/libxl/libxl_driver.c | 25 +------------------------
3 files changed, 26 insertions(+), 24 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 02/12] libxl: Introduce libxl_domain.[ch]
Create libxl_domain.[ch] and move all functions operating on
libxlDomainObjPrivate to these files. This will be useful for
future patches that e.g. add job support for libxlDomainObjPrivate.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libxl/libxl_conf.c | 2 +-
src/libxl/libxl_conf.h | 18 --
src/libxl/libxl_domain.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
src/libxl/libxl_domain.h | 61 ++++++
src/libxl/libxl_driver.c | 436 +------------------------------------------
7 files changed, 535 insertions(+), 453 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 9a83069..281274e 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -67,6 +67,7 @@ src/lxc/lxc_conf.c
src/lxc/lxc_controller.c
src/lxc/lxc_driver.c
src/lxc/lxc_process.c
+src/libxl/libxl_domain.c
src/libxl/libxl_driver.c
src/libxl/libxl_conf.c
src/network/bridge_driver.c
diff --git a/src/Makefile.am b/src/Makefile.am
index d8b943d..82aefe3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -657,6 +657,7 @@ XENAPI_DRIVER_SOURCES = \
LIBXL_DRIVER_SOURCES = \
libxl/libxl_conf.c libxl/libxl_conf.h \
+ libxl/libxl_domain.c libxl/libxl_domain.h \
libxl/libxl_driver.c libxl/libxl_driver.h
UML_DRIVER_SOURCES = \
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f8937a4..f9ffe5d 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -39,7 +39,7 @@
#include "viralloc.h"
#include "viruuid.h"
#include "capabilities.h"
-#include "libxl_driver.h"
+#include "libxl_domain.h"
#include "libxl_conf.h"
#include "libxl_utils.h"
#include "virstoragefile.h"
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 0498012..68e770c 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -89,24 +89,6 @@ struct _libxlDriverPrivate {
typedef struct _libxlEventHookInfo libxlEventHookInfo;
typedef libxlEventHookInfo *libxlEventHookInfoPtr;
-typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
-typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
-struct _libxlDomainObjPrivate {
- virObjectLockable parent;
-
- /* per domain log stream for libxl messages */
- FILE *logger_file;
- xentoollog_logger *logger;
- /* per domain libxl ctx */
- libxl_ctx *ctx;
- /* console */
- virChrdevsPtr devs;
- libxl_evgen_domain_death *deathW;
-
- /* list of libxl timeout registrations */
- libxlEventHookInfoPtr timerRegistrations;
-};
-
# define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r"
# define LIBXL_SAVE_VERSION 1
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
new file mode 100644
index 0000000..1d03797
--- /dev/null
+++ b/src/libxl/libxl_domain.c
@@ -0,0 +1,469 @@
+/*---------------------------------------------------------------------------*/
+/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ *
+ * 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/>;.
+ *
+ * Authors:
+ * Jim Fehlig <jfehlig(a)suse.com>
+ */
+/*---------------------------------------------------------------------------*/
+
+#include <config.h>
+
+#include "libxl_domain.h"
+
+#include "viralloc.h"
+#include "virfile.h"
+#include "virerror.h"
+#include "virlog.h"
+#include "virstring.h"
+
+#define VIR_FROM_THIS VIR_FROM_LIBXL
+
+
+/* Append an event registration to the list of registrations */
+#define LIBXL_EV_REG_APPEND(head, add) \
+ do { \
+ libxlEventHookInfoPtr temp; \
+ if (head) { \
+ temp = head; \
+ while (temp->next) \
+ temp = temp->next; \
+ temp->next = add; \
+ } else { \
+ head = add; \
+ } \
+ } while (0)
+
+/* Remove an event registration from the list of registrations */
+#define LIBXL_EV_REG_REMOVE(head, del) \
+ do { \
+ libxlEventHookInfoPtr temp; \
+ if (head == del) { \
+ head = head->next; \
+ } else { \
+ temp = head; \
+ while (temp->next && temp->next != del) \
+ temp = temp->next; \
+ if (temp->next) { \
+ temp->next = del->next; \
+ } \
+ } \
+ } while (0)
+
+/* Object used to store info related to libxl event registrations */
+struct _libxlEventHookInfo {
+ libxlEventHookInfoPtr next;
+ libxlDomainObjPrivatePtr priv;
+ void *xl_priv;
+ int id;
+};
+
+static virClassPtr libxlDomainObjPrivateClass;
+
+static void
+libxlDomainObjPrivateDispose(void *obj);
+
+static int
+libxlDomainObjPrivateOnceInit(void)
+{
+ if (!(libxlDomainObjPrivateClass = virClassNew(virClassForObjectLockable(),
+ "libxlDomainObjPrivate",
+ sizeof(libxlDomainObjPrivate),
+ libxlDomainObjPrivateDispose)))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(libxlDomainObjPrivate)
+
+static void
+libxlDomainObjEventHookInfoFree(void *obj)
+{
+ libxlEventHookInfoPtr info = obj;
+
+ /* Drop reference on libxlDomainObjPrivate */
+ virObjectUnref(info->priv);
+ VIR_FREE(info);
+}
+
+static void
+libxlDomainObjFDEventCallback(int watch ATTRIBUTE_UNUSED,
+ int fd,
+ int vir_events,
+ void *fd_info)
+{
+ libxlEventHookInfoPtr info = fd_info;
+ int events = 0;
+
+ virObjectLock(info->priv);
+ if (vir_events & VIR_EVENT_HANDLE_READABLE)
+ events |= POLLIN;
+ if (vir_events & VIR_EVENT_HANDLE_WRITABLE)
+ events |= POLLOUT;
+ if (vir_events & VIR_EVENT_HANDLE_ERROR)
+ events |= POLLERR;
+ if (vir_events & VIR_EVENT_HANDLE_HANGUP)
+ events |= POLLHUP;
+
+ virObjectUnlock(info->priv);
+ libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events);
+}
+
+static int
+libxlDomainObjFDRegisterEventHook(void *priv,
+ int fd,
+ void **hndp,
+ short events,
+ void *xl_priv)
+{
+ int vir_events = VIR_EVENT_HANDLE_ERROR;
+ libxlEventHookInfoPtr info;
+
+ if (VIR_ALLOC(info) < 0)
+ return -1;
+
+ info->priv = priv;
+ /*
+ * Take a reference on the domain object. Reference is dropped in
+ * libxlDomainObjEventHookInfoFree, ensuring the domain object outlives
+ * the fd event objects.
+ */
+ virObjectRef(info->priv);
+ info->xl_priv = xl_priv;
+
+ if (events & POLLIN)
+ vir_events |= VIR_EVENT_HANDLE_READABLE;
+ if (events & POLLOUT)
+ vir_events |= VIR_EVENT_HANDLE_WRITABLE;
+
+ info->id = virEventAddHandle(fd, vir_events, libxlDomainObjFDEventCallback,
+ info, libxlDomainObjEventHookInfoFree);
+ if (info->id < 0) {
+ virObjectUnref(info->priv);
+ VIR_FREE(info);
+ return -1;
+ }
+
+ *hndp = info;
+
+ return 0;
+}
+
+static int
+libxlDomainObjFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ void **hndp,
+ short events)
+{
+ libxlEventHookInfoPtr info = *hndp;
+ int vir_events = VIR_EVENT_HANDLE_ERROR;
+
+ virObjectLock(info->priv);
+ if (events & POLLIN)
+ vir_events |= VIR_EVENT_HANDLE_READABLE;
+ if (events & POLLOUT)
+ vir_events |= VIR_EVENT_HANDLE_WRITABLE;
+
+ virEventUpdateHandle(info->id, vir_events);
+ virObjectUnlock(info->priv);
+
+ return 0;
+}
+
+static void
+libxlDomainObjFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ void *hnd)
+{
+ libxlEventHookInfoPtr info = hnd;
+ libxlDomainObjPrivatePtr p = info->priv;
+
+ virObjectLock(p);
+ virEventRemoveHandle(info->id);
+ virObjectUnlock(p);
+}
+
+static void
+libxlDomainObjTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
+{
+ libxlEventHookInfoPtr info = timer_info;
+ libxlDomainObjPrivatePtr p = info->priv;
+
+ virObjectLock(p);
+ /*
+ * libxl expects the event to be deregistered when calling
+ * libxl_osevent_occurred_timeout, but we dont want the event info
+ * destroyed. Disable the timeout and only remove it after returning
+ * from libxl.
+ */
+ virEventUpdateTimeout(info->id, -1);
+ virObjectUnlock(p);
+ libxl_osevent_occurred_timeout(p->ctx, info->xl_priv);
+ virObjectLock(p);
+ /*
+ * Timeout could have been freed while the lock was dropped.
+ * Only remove it from the list if it still exists.
+ */
+ if (virEventRemoveTimeout(info->id) == 0)
+ LIBXL_EV_REG_REMOVE(p->timerRegistrations, info);
+ virObjectUnlock(p);
+}
+
+static int
+libxlDomainObjTimeoutRegisterEventHook(void *priv,
+ void **hndp,
+ struct timeval abs_t,
+ void *xl_priv)
+{
+ libxlEventHookInfoPtr info;
+ struct timeval now;
+ struct timeval res;
+ static struct timeval zero;
+ int timeout;
+
+ if (VIR_ALLOC(info) < 0)
+ return -1;
+
+ info->priv = priv;
+ /*
+ * Also take a reference on the domain object. Reference is dropped in
+ * libxlDomainObjEventHookInfoFree, ensuring the domain object outlives the
+ * timeout event objects.
+ */
+ virObjectRef(info->priv);
+ info->xl_priv = xl_priv;
+
+ gettimeofday(&now, NULL);
+ timersub(&abs_t, &now, &res);
+ /* Ensure timeout is not overflowed */
+ if (timercmp(&res, &zero, <)) {
+ timeout = 0;
+ } else if (res.tv_sec > INT_MAX / 1000) {
+ timeout = INT_MAX;
+ } else {
+ timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
+ }
+ info->id = virEventAddTimeout(timeout, libxlDomainObjTimerCallback,
+ info, libxlDomainObjEventHookInfoFree);
+ if (info->id < 0) {
+ virObjectUnref(info->priv);
+ VIR_FREE(info);
+ return -1;
+ }
+
+ virObjectLock(info->priv);
+ LIBXL_EV_REG_APPEND(info->priv->timerRegistrations, info);
+ virObjectUnlock(info->priv);
+ *hndp = info;
+
+ return 0;
+}
+
+/*
+ * Note: There are two changes wrt timeouts starting with xen-unstable
+ * changeset 26469:
+ *
+ * 1. Timeout modify callbacks will only be invoked with an abs_t of {0,0},
+ * i.e. make the timeout fire immediately. Prior to this commit, timeout
+ * modify callbacks were never invoked.
+ *
+ * 2. Timeout deregister hooks will no longer be called.
+ */
+static int
+libxlDomainObjTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
+ void **hndp,
+ struct timeval abs_t ATTRIBUTE_UNUSED)
+{
+ libxlEventHookInfoPtr info = *hndp;
+
+ virObjectLock(info->priv);
+ /* Make the timeout fire */
+ virEventUpdateTimeout(info->id, 0);
+ virObjectUnlock(info->priv);
+
+ return 0;
+}
+
+static void
+libxlDomainObjTimeoutDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
+ void *hnd)
+{
+ libxlEventHookInfoPtr info = hnd;
+ libxlDomainObjPrivatePtr p = info->priv;
+
+ virObjectLock(p);
+ /*
+ * Only remove the timeout from the list if removal from the
+ * event loop is successful.
+ */
+ if (virEventRemoveTimeout(info->id) == 0)
+ LIBXL_EV_REG_REMOVE(p->timerRegistrations, info);
+ virObjectUnlock(p);
+}
+
+
+static const libxl_osevent_hooks libxl_event_callbacks = {
+ .fd_register = libxlDomainObjFDRegisterEventHook,
+ .fd_modify = libxlDomainObjFDModifyEventHook,
+ .fd_deregister = libxlDomainObjFDDeregisterEventHook,
+ .timeout_register = libxlDomainObjTimeoutRegisterEventHook,
+ .timeout_modify = libxlDomainObjTimeoutModifyEventHook,
+ .timeout_deregister = libxlDomainObjTimeoutDeregisterEventHook,
+};
+
+static void *
+libxlDomainObjPrivateAlloc(void)
+{
+ libxlDomainObjPrivatePtr priv;
+
+ if (libxlDomainObjPrivateInitialize() < 0)
+ return NULL;
+
+ if (!(priv = virObjectLockableNew(libxlDomainObjPrivateClass)))
+ return NULL;
+
+ if (!(priv->devs = virChrdevAlloc())) {
+ virObjectUnref(priv);
+ return NULL;
+ }
+
+ return priv;
+}
+
+static void
+libxlDomainObjPrivateDispose(void *obj)
+{
+ libxlDomainObjPrivatePtr priv = obj;
+
+ if (priv->deathW)
+ libxl_evdisable_domain_death(priv->ctx, priv->deathW);
+
+ virChrdevFree(priv->devs);
+
+ xtl_logger_destroy(priv->logger);
+ if (priv->logger_file)
+ VIR_FORCE_FCLOSE(priv->logger_file);
+
+ libxl_ctx_free(priv->ctx);
+}
+
+static void
+libxlDomainObjPrivateFree(void *data)
+{
+ libxlDomainObjPrivatePtr priv = data;
+
+ if (priv->deathW) {
+ libxl_evdisable_domain_death(priv->ctx, priv->deathW);
+ priv->deathW = NULL;
+ }
+
+ virObjectUnref(priv);
+}
+
+virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = {
+ .alloc = libxlDomainObjPrivateAlloc,
+ .free = libxlDomainObjPrivateFree,
+};
+
+
+static int
+libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
+ virDomainDefPtr def,
+ virCapsPtr caps ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
+ dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+ dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE
&&
+ STRNEQ(def->os.type, "hvm"))
+ dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
+
+ return 0;
+}
+
+virDomainDefParserConfig libxlDomainDefParserConfig = {
+ .macPrefix = { 0x00, 0x16, 0x3e },
+ .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
+};
+
+int
+libxlDomainObjPrivateInitCtx(virDomainObjPtr vm)
+{
+ libxlDomainObjPrivatePtr priv = vm->privateData;
+ char *log_file;
+ int ret = -1;
+
+ if (priv->ctx)
+ return 0;
+
+ if (virAsprintf(&log_file, "%s/%s.log", LIBXL_LOG_DIR,
vm->def->name) < 0)
+ return -1;
+
+ if ((priv->logger_file = fopen(log_file, "a")) == NULL) {
+ virReportSystemError(errno,
+ _("failed to open logfile %s"),
+ log_file);
+ goto cleanup;
+ }
+
+ priv->logger =
+ (xentoollog_logger *)xtl_createlogger_stdiostream(priv->logger_file,
+ XTL_DEBUG, 0);
+ if (!priv->logger) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot create libxenlight logger for domain %s"),
+ vm->def->name);
+ goto cleanup;
+ }
+
+ if (libxl_ctx_alloc(&priv->ctx, LIBXL_VERSION, 0, priv->logger)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed libxl context initialization"));
+ goto cleanup;
+ }
+
+ libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv);
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(log_file);
+ return ret;
+}
+
+void
+libxlDomainObjRegisteredTimeoutsCleanup(libxlDomainObjPrivatePtr priv)
+{
+ libxlEventHookInfoPtr info;
+
+ virObjectLock(priv);
+ info = priv->timerRegistrations;
+ while (info) {
+ /*
+ * libxl expects the event to be deregistered when calling
+ * libxl_osevent_occurred_timeout, but we dont want the event info
+ * destroyed. Disable the timeout and only remove it after returning
+ * from libxl.
+ */
+ virEventUpdateTimeout(info->id, -1);
+ libxl_osevent_occurred_timeout(priv->ctx, info->xl_priv);
+ virEventRemoveTimeout(info->id);
+ info = info->next;
+ }
+ priv->timerRegistrations = NULL;
+ virObjectUnlock(priv);
+}
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
new file mode 100644
index 0000000..2797d38
--- /dev/null
+++ b/src/libxl/libxl_domain.h
@@ -0,0 +1,61 @@
+/*---------------------------------------------------------------------------*/
+/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ *
+ * 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/>;.
+ *
+ * Authors:
+ * Jim Fehlig <jfehlig(a)suse.com>
+ */
+/*---------------------------------------------------------------------------*/
+
+#ifndef LIBXL_DOMAIN_H
+# define LIBXL_DOMAIN_H
+
+# include <libxl.h>
+
+# include "domain_conf.h"
+# include "libxl_conf.h"
+# include "virchrdev.h"
+
+typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
+typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
+struct _libxlDomainObjPrivate {
+ virObjectLockable parent;
+
+ /* per domain log stream for libxl messages */
+ FILE *logger_file;
+ xentoollog_logger *logger;
+ /* per domain libxl ctx */
+ libxl_ctx *ctx;
+ /* console */
+ virChrdevsPtr devs;
+ libxl_evgen_domain_death *deathW;
+
+ /* list of libxl timeout registrations */
+ libxlEventHookInfoPtr timerRegistrations;
+};
+
+
+extern virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks;
+extern virDomainDefParserConfig libxlDomainDefParserConfig;
+
+
+int
+libxlDomainObjPrivateInitCtx(virDomainObjPtr vm);
+
+void
+libxlDomainObjRegisteredTimeoutsCleanup(libxlDomainObjPrivatePtr priv);
+
+#endif /* LIBXL_DOMAIN_H */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 91beb30..ff4f6be 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -41,6 +41,7 @@
#include "viralloc.h"
#include "viruuid.h"
#include "vircommand.h"
+#include "libxl_domain.h"
#include "libxl_driver.h"
#include "libxl_conf.h"
#include "xen_xm.h"
@@ -63,45 +64,6 @@
/* Number of Xen scheduler parameters */
#define XEN_SCHED_CREDIT_NPARAM 2
-/* Append an event registration to the list of registrations */
-#define LIBXL_EV_REG_APPEND(head, add) \
- do { \
- libxlEventHookInfoPtr temp; \
- if (head) { \
- temp = head; \
- while (temp->next) \
- temp = temp->next; \
- temp->next = add; \
- } else { \
- head = add; \
- } \
- } while (0)
-
-/* Remove an event registration from the list of registrations */
-#define LIBXL_EV_REG_REMOVE(head, del) \
- do { \
- libxlEventHookInfoPtr temp; \
- if (head == del) { \
- head = head->next; \
- } else { \
- temp = head; \
- while (temp->next && temp->next != del) \
- temp = temp->next; \
- if (temp->next) { \
- temp->next = del->next; \
- } \
- } \
- } while (0)
-
-/* Object used to store info related to libxl event registrations */
-struct _libxlEventHookInfo {
- libxlEventHookInfoPtr next;
- libxlDomainObjPrivatePtr priv;
- void *xl_priv;
- int id;
-};
-
-static virClassPtr libxlDomainObjPrivateClass;
static libxlDriverPrivatePtr libxl_driver = NULL;
@@ -115,24 +77,6 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
static void
-libxlDomainObjPrivateDispose(void *obj);
-
-/* Function definitions */
-static int
-libxlDomainObjPrivateOnceInit(void)
-{
- if (!(libxlDomainObjPrivateClass = virClassNew(virClassForObjectLockable(),
- "libxlDomainObjPrivate",
- sizeof(libxlDomainObjPrivate),
- libxlDomainObjPrivateDispose)))
- return -1;
-
- return 0;
-}
-
-VIR_ONCE_GLOBAL_INIT(libxlDomainObjPrivate)
-
-static void
libxlDriverLock(libxlDriverPrivatePtr driver)
{
virMutexLock(&driver->lock);
@@ -144,382 +88,6 @@ libxlDriverUnlock(libxlDriverPrivatePtr driver)
virMutexUnlock(&driver->lock);
}
-static void
-libxlEventHookInfoFree(void *obj)
-{
- libxlEventHookInfoPtr info = obj;
-
- /* Drop reference on libxlDomainObjPrivate */
- virObjectUnref(info->priv);
- VIR_FREE(info);
-}
-
-static void
-libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
- int fd,
- int vir_events,
- void *fd_info)
-{
- libxlEventHookInfoPtr info = fd_info;
- int events = 0;
-
- virObjectLock(info->priv);
- if (vir_events & VIR_EVENT_HANDLE_READABLE)
- events |= POLLIN;
- if (vir_events & VIR_EVENT_HANDLE_WRITABLE)
- events |= POLLOUT;
- if (vir_events & VIR_EVENT_HANDLE_ERROR)
- events |= POLLERR;
- if (vir_events & VIR_EVENT_HANDLE_HANGUP)
- events |= POLLHUP;
-
- virObjectUnlock(info->priv);
- libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events);
-}
-
-static int
-libxlFDRegisterEventHook(void *priv, int fd, void **hndp,
- short events, void *xl_priv)
-{
- int vir_events = VIR_EVENT_HANDLE_ERROR;
- libxlEventHookInfoPtr info;
-
- if (VIR_ALLOC(info) < 0)
- return -1;
-
- info->priv = priv;
- /*
- * Take a reference on the domain object. Reference is dropped in
- * libxlEventHookInfoFree, ensuring the domain object outlives the fd
- * event objects.
- */
- virObjectRef(info->priv);
- info->xl_priv = xl_priv;
-
- if (events & POLLIN)
- vir_events |= VIR_EVENT_HANDLE_READABLE;
- if (events & POLLOUT)
- vir_events |= VIR_EVENT_HANDLE_WRITABLE;
-
- info->id = virEventAddHandle(fd, vir_events, libxlFDEventCallback,
- info, libxlEventHookInfoFree);
- if (info->id < 0) {
- virObjectUnref(info->priv);
- VIR_FREE(info);
- return -1;
- }
-
- *hndp = info;
-
- return 0;
-}
-
-static int
-libxlFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED,
- void **hndp,
- short events)
-{
- libxlEventHookInfoPtr info = *hndp;
- int vir_events = VIR_EVENT_HANDLE_ERROR;
-
- virObjectLock(info->priv);
- if (events & POLLIN)
- vir_events |= VIR_EVENT_HANDLE_READABLE;
- if (events & POLLOUT)
- vir_events |= VIR_EVENT_HANDLE_WRITABLE;
-
- virEventUpdateHandle(info->id, vir_events);
- virObjectUnlock(info->priv);
-
- return 0;
-}
-
-static void
-libxlFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED,
- void *hnd)
-{
- libxlEventHookInfoPtr info = hnd;
- libxlDomainObjPrivatePtr p = info->priv;
-
- virObjectLock(p);
- virEventRemoveHandle(info->id);
- virObjectUnlock(p);
-}
-
-static void
-libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
-{
- libxlEventHookInfoPtr info = timer_info;
- libxlDomainObjPrivatePtr p = info->priv;
-
- virObjectLock(p);
- /*
- * libxl expects the event to be deregistered when calling
- * libxl_osevent_occurred_timeout, but we dont want the event info
- * destroyed. Disable the timeout and only remove it after returning
- * from libxl.
- */
- virEventUpdateTimeout(info->id, -1);
- virObjectUnlock(p);
- libxl_osevent_occurred_timeout(p->ctx, info->xl_priv);
- virObjectLock(p);
- /*
- * Timeout could have been freed while the lock was dropped.
- * Only remove it from the list if it still exists.
- */
- if (virEventRemoveTimeout(info->id) == 0)
- LIBXL_EV_REG_REMOVE(p->timerRegistrations, info);
- virObjectUnlock(p);
-}
-
-static int
-libxlTimeoutRegisterEventHook(void *priv,
- void **hndp,
- struct timeval abs_t,
- void *xl_priv)
-{
- libxlEventHookInfoPtr info;
- struct timeval now;
- struct timeval res;
- static struct timeval zero;
- int timeout;
-
- if (VIR_ALLOC(info) < 0)
- return -1;
-
- info->priv = priv;
- /*
- * Also take a reference on the domain object. Reference is dropped in
- * libxlEventHookInfoFree, ensuring the domain object outlives the timeout
- * event objects.
- */
- virObjectRef(info->priv);
- info->xl_priv = xl_priv;
-
- gettimeofday(&now, NULL);
- timersub(&abs_t, &now, &res);
- /* Ensure timeout is not overflowed */
- if (timercmp(&res, &zero, <))
- timeout = 0;
- else if (res.tv_sec > INT_MAX / 1000)
- timeout = INT_MAX;
- else
- timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
-
- info->id = virEventAddTimeout(timeout, libxlTimerCallback,
- info, libxlEventHookInfoFree);
- if (info->id < 0) {
- virObjectUnref(info->priv);
- VIR_FREE(info);
- return -1;
- }
-
- virObjectLock(info->priv);
- LIBXL_EV_REG_APPEND(info->priv->timerRegistrations, info);
- virObjectUnlock(info->priv);
- *hndp = info;
-
- return 0;
-}
-
-/*
- * Note: There are two changes wrt timeouts starting with xen-unstable
- * changeset 26469:
- *
- * 1. Timeout modify callbacks will only be invoked with an abs_t of {0,0},
- * i.e. make the timeout fire immediately. Prior to this commit, timeout
- * modify callbacks were never invoked.
- *
- * 2. Timeout deregister hooks will no longer be called.
- */
-static int
-libxlTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
- void **hndp,
- struct timeval abs_t ATTRIBUTE_UNUSED)
-{
- libxlEventHookInfoPtr info = *hndp;
-
- virObjectLock(info->priv);
- /* Make the timeout fire */
- virEventUpdateTimeout(info->id, 0);
- virObjectUnlock(info->priv);
-
- return 0;
-}
-
-static void
-libxlTimeoutDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
- void *hnd)
-{
- libxlEventHookInfoPtr info = hnd;
- libxlDomainObjPrivatePtr p = info->priv;
-
- virObjectLock(p);
- /*
- * Only remove the timeout from the list if removal from the
- * event loop is successful.
- */
- if (virEventRemoveTimeout(info->id) == 0)
- LIBXL_EV_REG_REMOVE(p->timerRegistrations, info);
- virObjectUnlock(p);
-}
-
-static void
-libxlRegisteredTimeoutsCleanup(libxlDomainObjPrivatePtr priv)
-{
- libxlEventHookInfoPtr info;
-
- virObjectLock(priv);
- info = priv->timerRegistrations;
- while (info) {
- /*
- * libxl expects the event to be deregistered when calling
- * libxl_osevent_occurred_timeout, but we dont want the event info
- * destroyed. Disable the timeout and only remove it after returning
- * from libxl.
- */
- virEventUpdateTimeout(info->id, -1);
- libxl_osevent_occurred_timeout(priv->ctx, info->xl_priv);
- virEventRemoveTimeout(info->id);
- info = info->next;
- }
- priv->timerRegistrations = NULL;
- virObjectUnlock(priv);
-}
-
-static const libxl_osevent_hooks libxl_event_callbacks = {
- .fd_register = libxlFDRegisterEventHook,
- .fd_modify = libxlFDModifyEventHook,
- .fd_deregister = libxlFDDeregisterEventHook,
- .timeout_register = libxlTimeoutRegisterEventHook,
- .timeout_modify = libxlTimeoutModifyEventHook,
- .timeout_deregister = libxlTimeoutDeregisterEventHook,
-};
-
-static int
-libxlDomainObjPrivateInitCtx(virDomainObjPtr vm)
-{
- libxlDomainObjPrivatePtr priv = vm->privateData;
- char *log_file;
- int ret = -1;
-
- if (priv->ctx)
- return 0;
-
- if (virAsprintf(&log_file, "%s/%s.log", LIBXL_LOG_DIR,
vm->def->name) < 0)
- return -1;
-
- if ((priv->logger_file = fopen(log_file, "a")) == NULL) {
- virReportSystemError(errno,
- _("failed to open logfile %s"),
- log_file);
- goto cleanup;
- }
-
- priv->logger =
- (xentoollog_logger *)xtl_createlogger_stdiostream(priv->logger_file,
- XTL_DEBUG, 0);
- if (!priv->logger) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot create libxenlight logger for domain %s"),
- vm->def->name);
- goto cleanup;
- }
-
- if (libxl_ctx_alloc(&priv->ctx, LIBXL_VERSION, 0, priv->logger)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Failed libxl context initialization"));
- goto cleanup;
- }
-
- libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv);
-
- ret = 0;
-
-cleanup:
- VIR_FREE(log_file);
- return ret;
-}
-
-static void *
-libxlDomainObjPrivateAlloc(void)
-{
- libxlDomainObjPrivatePtr priv;
-
- if (libxlDomainObjPrivateInitialize() < 0)
- return NULL;
-
- if (!(priv = virObjectLockableNew(libxlDomainObjPrivateClass)))
- return NULL;
-
- if (!(priv->devs = virChrdevAlloc())) {
- virObjectUnref(priv);
- return NULL;
- }
-
- return priv;
-}
-
-static void
-libxlDomainObjPrivateDispose(void *obj)
-{
- libxlDomainObjPrivatePtr priv = obj;
-
- if (priv->deathW)
- libxl_evdisable_domain_death(priv->ctx, priv->deathW);
-
- virChrdevFree(priv->devs);
-
- xtl_logger_destroy(priv->logger);
- if (priv->logger_file)
- VIR_FORCE_FCLOSE(priv->logger_file);
-
- libxl_ctx_free(priv->ctx);
-}
-
-static void
-libxlDomainObjPrivateFree(void *data)
-{
- libxlDomainObjPrivatePtr priv = data;
-
- if (priv->deathW) {
- libxl_evdisable_domain_death(priv->ctx, priv->deathW);
- priv->deathW = NULL;
- }
-
- virObjectUnref(priv);
-}
-
-virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = {
- .alloc = libxlDomainObjPrivateAlloc,
- .free = libxlDomainObjPrivateFree,
-};
-
-
-static int
-libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
- virDomainDefPtr def,
- virCapsPtr caps ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
-{
- if (dev->type == VIR_DOMAIN_DEVICE_CHR &&
- dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
- dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE
&&
- STRNEQ(def->os.type, "hvm"))
- dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
-
- return 0;
-}
-
-
-virDomainDefParserConfig libxlDomainDefParserConfig = {
- .macPrefix = { 0x00, 0x16, 0x3e },
- .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
-};
-
-
/* driver must be locked before calling */
static void
libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
@@ -728,7 +296,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
vm->newDef = NULL;
}
- libxlRegisteredTimeoutsCleanup(priv);
+ libxlDomainObjRegisteredTimeoutsCleanup(priv);
}
/*
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 02/12] libxl: Introduce libxl_domain.[ch]
On 30.08.2013 23:46, Jim Fehlig wrote:
Create libxl_domain.[ch] and move all functions operating on
libxlDomainObjPrivate to these files. This will be useful for
future patches that e.g. add job support for libxlDomainObjPrivate.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libxl/libxl_conf.c | 2 +-
src/libxl/libxl_conf.h | 18 --
src/libxl/libxl_domain.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
src/libxl/libxl_domain.h | 61 ++++++
src/libxl/libxl_driver.c | 436 +------------------------------------------
7 files changed, 535 insertions(+), 453 deletions(-)
Did a compile test and it passes. The diff stat looks reasonably, so I
am comfortable giving my ACK, or do you really want me to check if you
haven't changed any function? :)
Michal
6:22 a.m.
New subject: [libvirt] [PATCH 02/12] libxl: Introduce libxl_domain.[ch]
On Fri, Aug 30, 2013 at 03:46:48PM -0600, Jim Fehlig wrote:
Create libxl_domain.[ch] and move all functions operating on
libxlDomainObjPrivate to these files. This will be useful for
future patches that e.g. add job support for libxlDomainObjPrivate.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libxl/libxl_conf.c | 2 +-
src/libxl/libxl_conf.h | 18 --
src/libxl/libxl_domain.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
src/libxl/libxl_domain.h | 61 ++++++
src/libxl/libxl_driver.c | 436 +------------------------------------------
7 files changed, 535 insertions(+), 453 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 9a83069..281274e 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -67,6 +67,7 @@ src/lxc/lxc_conf.c
src/lxc/lxc_controller.c
src/lxc/lxc_driver.c
src/lxc/lxc_process.c
+src/libxl/libxl_domain.c
src/libxl/libxl_driver.c
src/libxl/libxl_conf.c
src/network/bridge_driver.c
diff --git a/src/Makefile.am b/src/Makefile.am
index d8b943d..82aefe3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -657,6 +657,7 @@ XENAPI_DRIVER_SOURCES = \
LIBXL_DRIVER_SOURCES = \
libxl/libxl_conf.c libxl/libxl_conf.h \
+ libxl/libxl_domain.c libxl/libxl_domain.h \
libxl/libxl_driver.c libxl/libxl_driver.h
UML_DRIVER_SOURCES = \
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f8937a4..f9ffe5d 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -39,7 +39,7 @@
#include "viralloc.h"
#include "viruuid.h"
#include "capabilities.h"
-#include "libxl_driver.h"
+#include "libxl_domain.h"
#include "libxl_conf.h"
#include "libxl_utils.h"
#include "virstoragefile.h"
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 0498012..68e770c 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -89,24 +89,6 @@ struct _libxlDriverPrivate {
typedef struct _libxlEventHookInfo libxlEventHookInfo;
typedef libxlEventHookInfo *libxlEventHookInfoPtr;
-typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
-typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
-struct _libxlDomainObjPrivate {
- virObjectLockable parent;
-
- /* per domain log stream for libxl messages */
- FILE *logger_file;
- xentoollog_logger *logger;
- /* per domain libxl ctx */
- libxl_ctx *ctx;
- /* console */
- virChrdevsPtr devs;
- libxl_evgen_domain_death *deathW;
-
- /* list of libxl timeout registrations */
- libxlEventHookInfoPtr timerRegistrations;
-};
-
# define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r"
# define LIBXL_SAVE_VERSION 1
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
new file mode 100644
index 0000000..1d03797
--- /dev/null
+++ b/src/libxl/libxl_domain.c
@@ -0,0 +1,469 @@
+/*---------------------------------------------------------------------------*/
+/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
It is a pretty minor nitpick, but the normal style
/*
* filename.h: blah description blah
*
* Copyright (C) 2013 ....
without any '/*-------------------....'
+ *
+ * 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/>;.
+ *
+ * Authors:
+ * Jim Fehlig <jfehlig(a)suse.com>
+ */
+/*---------------------------------------------------------------------------*/
Can remove the '------------------' here too
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> new file mode 100644
> index 0000000..2797d38
> --- /dev/null
> +++ b/src/libxl/libxl_domain.h
> @@ -0,0 +1,61 @@
> +/*---------------------------------------------------------------------------*/
> +/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ *
+ * 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/>;.
+ *
+ * Authors:
+ * Jim Fehlig <jfehlig(a)suse.com>
+ */
+/*---------------------------------------------------------------------------*/
Same nitpick here too.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:24 a.m.
New subject: [libvirt] [PATCH 02/12] libxl: Introduce libxl_domain.[ch]
On Mon, Sep 02, 2013 at 12:22:40PM +0100, Daniel P. Berrange wrote:
On Fri, Aug 30, 2013 at 03:46:48PM -0600, Jim Fehlig wrote:
> Create libxl_domain.[ch] and move all functions operating on
> libxlDomainObjPrivate to these files. This will be useful for
> future patches that e.g. add job support for libxlDomainObjPrivate.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> po/POTFILES.in | 1 +
> src/Makefile.am | 1 +
> src/libxl/libxl_conf.c | 2 +-
> src/libxl/libxl_conf.h | 18 --
> src/libxl/libxl_domain.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
> src/libxl/libxl_domain.h | 61 ++++++
> src/libxl/libxl_driver.c | 436 +------------------------------------------
> 7 files changed, 535 insertions(+), 453 deletions(-)
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 9a83069..281274e 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -67,6 +67,7 @@ src/lxc/lxc_conf.c
> src/lxc/lxc_controller.c
> src/lxc/lxc_driver.c
> src/lxc/lxc_process.c
> +src/libxl/libxl_domain.c
> src/libxl/libxl_driver.c
> src/libxl/libxl_conf.c
> src/network/bridge_driver.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index d8b943d..82aefe3 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -657,6 +657,7 @@ XENAPI_DRIVER_SOURCES = \
>
> LIBXL_DRIVER_SOURCES = \
> libxl/libxl_conf.c libxl/libxl_conf.h \
> + libxl/libxl_domain.c libxl/libxl_domain.h \
> libxl/libxl_driver.c libxl/libxl_driver.h
>
> UML_DRIVER_SOURCES = \
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index f8937a4..f9ffe5d 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -39,7 +39,7 @@
> #include "viralloc.h"
> #include "viruuid.h"
> #include "capabilities.h"
> -#include "libxl_driver.h"
> +#include "libxl_domain.h"
> #include "libxl_conf.h"
> #include "libxl_utils.h"
> #include "virstoragefile.h"
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 0498012..68e770c 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -89,24 +89,6 @@ struct _libxlDriverPrivate {
> typedef struct _libxlEventHookInfo libxlEventHookInfo;
> typedef libxlEventHookInfo *libxlEventHookInfoPtr;
>
> -typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
> -typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
> -struct _libxlDomainObjPrivate {
> - virObjectLockable parent;
> -
> - /* per domain log stream for libxl messages */
> - FILE *logger_file;
> - xentoollog_logger *logger;
> - /* per domain libxl ctx */
> - libxl_ctx *ctx;
> - /* console */
> - virChrdevsPtr devs;
> - libxl_evgen_domain_death *deathW;
> -
> - /* list of libxl timeout registrations */
> - libxlEventHookInfoPtr timerRegistrations;
> -};
> -
> # define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r"
> # define LIBXL_SAVE_VERSION 1
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> new file mode 100644
> index 0000000..1d03797
> --- /dev/null
> +++ b/src/libxl/libxl_domain.c
> @@ -0,0 +1,469 @@
> +/*---------------------------------------------------------------------------*/
> +/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
It is a pretty minor nitpick, but the normal style
/*
* filename.h: blah description blah
*
* Copyright (C) 2013 ....
without any '/*-------------------....'
> + *
> + * 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/>;.
> + *
> + * Authors:
> + * Jim Fehlig <jfehlig(a)suse.com>
> + */
> +/*---------------------------------------------------------------------------*/
Can remove the '------------------' here too
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> new file mode 100644
> index 0000000..2797d38
> --- /dev/null
> +++ b/src/libxl/libxl_domain.h
> @@ -0,0 +1,61 @@
> +/*---------------------------------------------------------------------------*/
> +/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
> + *
> + * 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/>;.
> + *
> + * Authors:
> + * Jim Fehlig <jfehlig(a)suse.com>
> + */
> +/*---------------------------------------------------------------------------*/
Same nitpick here too.
Forgot to say ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:59 p.m.
New subject: [libvirt] [PATCH 02/12] libxl: Introduce libxl_domain.[ch]
Daniel P. Berrange wrote:
On Fri, Aug 30, 2013 at 03:46:48PM -0600, Jim Fehlig wrote:
> Create libxl_domain.[ch] and move all functions operating on
> libxlDomainObjPrivate to these files. This will be useful for
> future patches that e.g. add job support for libxlDomainObjPrivate.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> po/POTFILES.in | 1 +
> src/Makefile.am | 1 +
> src/libxl/libxl_conf.c | 2 +-
> src/libxl/libxl_conf.h | 18 --
> src/libxl/libxl_domain.c | 469 +++++++++++++++++++++++++++++++++++++++++++++++
> src/libxl/libxl_domain.h | 61 ++++++
> src/libxl/libxl_driver.c | 436 +------------------------------------------
> 7 files changed, 535 insertions(+), 453 deletions(-)
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 9a83069..281274e 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -67,6 +67,7 @@ src/lxc/lxc_conf.c
> src/lxc/lxc_controller.c
> src/lxc/lxc_driver.c
> src/lxc/lxc_process.c
> +src/libxl/libxl_domain.c
> src/libxl/libxl_driver.c
> src/libxl/libxl_conf.c
> src/network/bridge_driver.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index d8b943d..82aefe3 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -657,6 +657,7 @@ XENAPI_DRIVER_SOURCES = \
>
> LIBXL_DRIVER_SOURCES = \
> libxl/libxl_conf.c libxl/libxl_conf.h \
> + libxl/libxl_domain.c libxl/libxl_domain.h \
> libxl/libxl_driver.c libxl/libxl_driver.h
>
> UML_DRIVER_SOURCES = \
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index f8937a4..f9ffe5d 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -39,7 +39,7 @@
> #include "viralloc.h"
> #include "viruuid.h"
> #include "capabilities.h"
> -#include "libxl_driver.h"
> +#include "libxl_domain.h"
> #include "libxl_conf.h"
> #include "libxl_utils.h"
> #include "virstoragefile.h"
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 0498012..68e770c 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -89,24 +89,6 @@ struct _libxlDriverPrivate {
> typedef struct _libxlEventHookInfo libxlEventHookInfo;
> typedef libxlEventHookInfo *libxlEventHookInfoPtr;
>
> -typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
> -typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
> -struct _libxlDomainObjPrivate {
> - virObjectLockable parent;
> -
> - /* per domain log stream for libxl messages */
> - FILE *logger_file;
> - xentoollog_logger *logger;
> - /* per domain libxl ctx */
> - libxl_ctx *ctx;
> - /* console */
> - virChrdevsPtr devs;
> - libxl_evgen_domain_death *deathW;
> -
> - /* list of libxl timeout registrations */
> - libxlEventHookInfoPtr timerRegistrations;
> -};
> -
> # define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r"
> # define LIBXL_SAVE_VERSION 1
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> new file mode 100644
> index 0000000..1d03797
> --- /dev/null
> +++ b/src/libxl/libxl_domain.c
> @@ -0,0 +1,469 @@
> +/*---------------------------------------------------------------------------*/
> +/* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
>
It is a pretty minor nitpick, but the normal style
/*
* filename.h: blah description blah
*
* Copyright (C) 2013 ....
without any '/*-------------------....'
Ok, I'll change this and send a followup patch for the other libxl files
that similarly deviate from the norm :).
Regards,
Jim
4:46 p.m.
New subject: [libvirt] [PATCH 03/12] libxl: Earlier detection of not running on Xen
Detect early on in libxl driver initialization if the driver
should be loaded at all, avoiding needless initialization steps
that only have to be undone later. While at it, move th
detection to a helper function to improve readability.
After detecting that the driver should be loaded, subsequent
failures such as initializing the log stream, allocating libxl
ctx, etc. should be treated as failure to initialize the driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 22 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ff4f6be..759fed5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -775,34 +775,54 @@ libxlStateCleanup(void)
return 0;
}
-static int
-libxlStateInitialize(bool privileged,
- virStateInhibitCallback callback ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
+static bool
+libxlDriverShouldLoad(bool privileged)
{
- const libxl_version_info *ver_info;
- char *log_file = NULL;
+ bool ret = false;
virCommandPtr cmd;
- int status, ret = 0;
- unsigned int free_mem;
- char ebuf[1024];
+ int status;
- /* Disable libxl driver if non-root */
+ /* Don't load if non-root */
if (!privileged) {
VIR_INFO("Not running privileged, disabling libxenlight driver");
- return 0;
+ return ret;
+ }
+
+ /* Don't load if not running on a Xen control domain (dom0) */
+ if (!virFileExists("/proc/xen/capabilities")) {
+ VIR_INFO("No Xen capabilities detected, probably not running "
+ "in a Xen Dom0. Disabling libxenlight driver");
+
+ return ret;
}
- /* Disable driver if legacy xen toolstack (xend) is in use */
+ /* Don't load if legacy xen toolstack (xend) is in use */
cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
if (virCommandRun(cmd, &status) == 0 && status == 0) {
VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
"libxenlight driver.");
- virCommandFree(cmd);
- return 0;
+ } else {
+ ret = true;
}
virCommandFree(cmd);
+ return ret;
+}
+
+static int
+libxlStateInitialize(bool privileged,
+ virStateInhibitCallback callback ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ const libxl_version_info *ver_info;
+ char *log_file = NULL;
+ int ret = 0;
+ unsigned int free_mem;
+ char ebuf[1024];
+
+ if (!libxlDriverShouldLoad(privileged))
+ return 0;
+
if (VIR_ALLOC(libxl_driver) < 0)
return -1;
@@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged,
libxl_driver->logger =
(xentoollog_logger
*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
if (!libxl_driver->logger) {
- VIR_INFO("cannot create logger for libxenlight, disabling driver");
- goto fail;
+ VIR_ERROR(_("Failed to create logger for libxenlight"));
+ goto error;
}
if (libxl_ctx_alloc(&libxl_driver->ctx,
LIBXL_VERSION, 0,
libxl_driver->logger)) {
- VIR_INFO("cannot initialize libxenlight context, probably not running
"
- "in a Xen Dom0, disabling driver");
- goto fail;
+ VIR_ERROR(_("Failed to initialize libxenlight context"));
+ goto error;
}
if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
- VIR_INFO("cannot version information from libxenlight, disabling
driver");
- goto fail;
+ VIR_ERROR(_("Failed to get version information from libxenlight"));
+ goto error;
}
libxl_driver->version = (ver_info->xen_version_major * 1000000) +
(ver_info->xen_version_minor * 1000);
@@ -956,7 +975,6 @@ libxlStateInitialize(bool privileged,
error:
ret = -1;
-fail:
VIR_FREE(log_file);
if (libxl_driver)
libxlDriverUnlock(libxl_driver);
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 03/12] libxl: Earlier detection of not running on Xen
On 30.08.2013 23:46, Jim Fehlig wrote:
Detect early on in libxl driver initialization if the driver
should be loaded at all, avoiding needless initialization steps
that only have to be undone later. While at it, move th
s/th/the/
detection to a helper function to improve readability.
After detecting that the driver should be loaded, subsequent
failures such as initializing the log stream, allocating libxl
ctx, etc. should be treated as failure to initialize the driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 22 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ff4f6be..759fed5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -775,34 +775,54 @@ libxlStateCleanup(void)
return 0;
}
-static int
-libxlStateInitialize(bool privileged,
- virStateInhibitCallback callback ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
+static bool
+libxlDriverShouldLoad(bool privileged)
{
- const libxl_version_info *ver_info;
- char *log_file = NULL;
+ bool ret = false;
virCommandPtr cmd;
- int status, ret = 0;
- unsigned int free_mem;
- char ebuf[1024];
+ int status;
- /* Disable libxl driver if non-root */
+ /* Don't load if non-root */
if (!privileged) {
VIR_INFO("Not running privileged, disabling libxenlight driver");
- return 0;
+ return ret;
+ }
+
+ /* Don't load if not running on a Xen control domain (dom0) */
+ if (!virFileExists("/proc/xen/capabilities")) {
+ VIR_INFO("No Xen capabilities detected, probably not running "
+ "in a Xen Dom0. Disabling libxenlight driver");
+
+ return ret;
}
- /* Disable driver if legacy xen toolstack (xend) is in use */
+ /* Don't load if legacy xen toolstack (xend) is in use */
Indentation off.
cmd = virCommandNewArgList("/usr/sbin/xend",
"status", NULL);
if (virCommandRun(cmd, &status) == 0 && status == 0) {
VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
"libxenlight driver.");
- virCommandFree(cmd);
- return 0;
+ } else {
+ ret = true;
}
virCommandFree(cmd);
+ return ret;
+}
+
+static int
+libxlStateInitialize(bool privileged,
+ virStateInhibitCallback callback ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ const libxl_version_info *ver_info;
+ char *log_file = NULL;
+ int ret = 0;
+ unsigned int free_mem;
+ char ebuf[1024];
+
+ if (!libxlDriverShouldLoad(privileged))
+ return 0;
+
if (VIR_ALLOC(libxl_driver) < 0)
return -1;
@@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged,
libxl_driver->logger =
(xentoollog_logger
*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
if (!libxl_driver->logger) {
- VIR_INFO("cannot create logger for libxenlight, disabling driver");
- goto fail;
+ VIR_ERROR(_("Failed to create logger for libxenlight"));
+ goto error;
}
if (libxl_ctx_alloc(&libxl_driver->ctx,
LIBXL_VERSION, 0,
libxl_driver->logger)) {
- VIR_INFO("cannot initialize libxenlight context, probably not running
"
- "in a Xen Dom0, disabling driver");
- goto fail;
+ VIR_ERROR(_("Failed to initialize libxenlight context"));
+ goto error;
}
if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
- VIR_INFO("cannot version information from libxenlight, disabling
driver");
- goto fail;
+ VIR_ERROR(_("Failed to get version information from libxenlight"));
+ goto error;
}
libxl_driver->version = (ver_info->xen_version_major * 1000000) +
(ver_info->xen_version_minor * 1000);
@@ -956,7 +975,6 @@ libxlStateInitialize(bool privileged,
error:
ret = -1;
-fail:
VIR_FREE(log_file);
if (libxl_driver)
libxlDriverUnlock(libxl_driver);
ACK with those two nits fixed.
Michal
6:24 a.m.
New subject: [libvirt] [PATCH 03/12] libxl: Earlier detection of not running on Xen
On Fri, Aug 30, 2013 at 03:46:49PM -0600, Jim Fehlig wrote:
Detect early on in libxl driver initialization if the driver
should be loaded at all, avoiding needless initialization steps
that only have to be undone later. While at it, move th
detection to a helper function to improve readability.
After detecting that the driver should be loaded, subsequent
failures such as initializing the log stream, allocating libxl
ctx, etc. should be treated as failure to initialize the driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 22 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ff4f6be..759fed5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -775,34 +775,54 @@ libxlStateCleanup(void)
return 0;
}
-static int
-libxlStateInitialize(bool privileged,
- virStateInhibitCallback callback ATTRIBUTE_UNUSED,
- void *opaque ATTRIBUTE_UNUSED)
+static bool
+libxlDriverShouldLoad(bool privileged)
{
- const libxl_version_info *ver_info;
- char *log_file = NULL;
+ bool ret = false;
virCommandPtr cmd;
- int status, ret = 0;
- unsigned int free_mem;
- char ebuf[1024];
+ int status;
- /* Disable libxl driver if non-root */
+ /* Don't load if non-root */
if (!privileged) {
VIR_INFO("Not running privileged, disabling libxenlight driver");
- return 0;
+ return ret;
+ }
+
+ /* Don't load if not running on a Xen control domain (dom0) */
+ if (!virFileExists("/proc/xen/capabilities")) {
+ VIR_INFO("No Xen capabilities detected, probably not running "
+ "in a Xen Dom0. Disabling libxenlight driver");
+
+ return ret;
}
- /* Disable driver if legacy xen toolstack (xend) is in use */
+ /* Don't load if legacy xen toolstack (xend) is in use */
Indentation typo
cmd = virCommandNewArgList("/usr/sbin/xend",
"status", NULL);
if (virCommandRun(cmd, &status) == 0 && status == 0) {
VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
"libxenlight driver.");
- virCommandFree(cmd);
- return 0;
+ } else {
+ ret = true;
}
virCommandFree(cmd);
+ return ret;
+}
+
+static int
+libxlStateInitialize(bool privileged,
+ virStateInhibitCallback callback ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ const libxl_version_info *ver_info;
+ char *log_file = NULL;
+ int ret = 0;
+ unsigned int free_mem;
+ char ebuf[1024];
+
+ if (!libxlDriverShouldLoad(privileged))
+ return 0;
+
if (VIR_ALLOC(libxl_driver) < 0)
return -1;
@@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged,
libxl_driver->logger =
(xentoollog_logger
*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
if (!libxl_driver->logger) {
- VIR_INFO("cannot create logger for libxenlight, disabling driver");
- goto fail;
+ VIR_ERROR(_("Failed to create logger for libxenlight"));
+ goto error;
}
if (libxl_ctx_alloc(&libxl_driver->ctx,
LIBXL_VERSION, 0,
libxl_driver->logger)) {
- VIR_INFO("cannot initialize libxenlight context, probably not running
"
- "in a Xen Dom0, disabling driver");
- goto fail;
+ VIR_ERROR(_("Failed to initialize libxenlight context"));
+ goto error;
}
if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
- VIR_INFO("cannot version information from libxenlight, disabling
driver");
- goto fail;
+ VIR_ERROR(_("Failed to get version information from libxenlight"));
For the errors here, it is preferrable to use virReportError() which will
in turn call VIR_ERROR anywway.
+ goto error;
}
libxl_driver->version = (ver_info->xen_version_major * 1000000) +
(ver_info->xen_version_minor * 1000);
@@ -956,7 +975,6 @@ libxlStateInitialize(bool privileged,
error:
ret = -1;
-fail:
VIR_FREE(log_file);
if (libxl_driver)
libxlDriverUnlock(libxl_driver);
ACK
if the error reporting is changed
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:07 p.m.
New subject: [libvirt] [PATCH 03/12] libxl: Earlier detection of not running on Xen
Daniel P. Berrange wrote:
On Fri, Aug 30, 2013 at 03:46:49PM -0600, Jim Fehlig wrote:
> Detect early on in libxl driver initialization if the driver
> should be loaded at all, avoiding needless initialization steps
> that only have to be undone later. While at it, move th
> detection to a helper function to improve readability.
>
> After detecting that the driver should be loaded, subsequent
> failures such as initializing the log stream, allocating libxl
> ctx, etc. should be treated as failure to initialize the driver.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 22 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index ff4f6be..759fed5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -775,34 +775,54 @@ libxlStateCleanup(void)
> return 0;
> }
>
> -static int
> -libxlStateInitialize(bool privileged,
> - virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> - void *opaque ATTRIBUTE_UNUSED)
> +static bool
> +libxlDriverShouldLoad(bool privileged)
> {
> - const libxl_version_info *ver_info;
> - char *log_file = NULL;
> + bool ret = false;
> virCommandPtr cmd;
> - int status, ret = 0;
> - unsigned int free_mem;
> - char ebuf[1024];
> + int status;
>
> - /* Disable libxl driver if non-root */
> + /* Don't load if non-root */
> if (!privileged) {
> VIR_INFO("Not running privileged, disabling libxenlight driver");
> - return 0;
> + return ret;
> + }
> +
> + /* Don't load if not running on a Xen control domain (dom0) */
> + if (!virFileExists("/proc/xen/capabilities")) {
> + VIR_INFO("No Xen capabilities detected, probably not running "
> + "in a Xen Dom0. Disabling libxenlight driver");
> +
> + return ret;
> }
>
> - /* Disable driver if legacy xen toolstack (xend) is in use */
> + /* Don't load if legacy xen toolstack (xend) is in use */
>
Indentation typo
> cmd = virCommandNewArgList("/usr/sbin/xend", "status",
NULL);
> if (virCommandRun(cmd, &status) == 0 && status == 0) {
> VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
> "libxenlight driver.");
> - virCommandFree(cmd);
> - return 0;
> + } else {
> + ret = true;
> }
> virCommandFree(cmd);
>
> + return ret;
> +}
> +
> +static int
> +libxlStateInitialize(bool privileged,
> + virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> + void *opaque ATTRIBUTE_UNUSED)
> +{
> + const libxl_version_info *ver_info;
> + char *log_file = NULL;
> + int ret = 0;
> + unsigned int free_mem;
> + char ebuf[1024];
> +
> + if (!libxlDriverShouldLoad(privileged))
> + return 0;
> +
> if (VIR_ALLOC(libxl_driver) < 0)
> return -1;
>
> @@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged,
> libxl_driver->logger =
> (xentoollog_logger
*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
> if (!libxl_driver->logger) {
> - VIR_INFO("cannot create logger for libxenlight, disabling
driver");
> - goto fail;
> + VIR_ERROR(_("Failed to create logger for libxenlight"));
> + goto error;
> }
>
> if (libxl_ctx_alloc(&libxl_driver->ctx,
> LIBXL_VERSION, 0,
> libxl_driver->logger)) {
> - VIR_INFO("cannot initialize libxenlight context, probably not running
"
> - "in a Xen Dom0, disabling driver");
> - goto fail;
> + VIR_ERROR(_("Failed to initialize libxenlight context"));
> + goto error;
> }
>
> if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
> - VIR_INFO("cannot version information from libxenlight, disabling
driver");
> - goto fail;
> + VIR_ERROR(_("Failed to get version information from
libxenlight"));
>
For the errors here, it is preferrable to use virReportError() which will
in turn call VIR_ERROR anywway.
I'll change these to virReportError(). Is it generally preferable to
use virReportError() over VIR_ERROR? If so, I'll send a followup patch
to replace any remaining uses of VIR_ERROR.
Regards,
Jim
4:52 a.m.
New subject: [libvirt] [PATCH 03/12] libxl: Earlier detection of not running on Xen
On Tue, Sep 03, 2013 at 05:07:25PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
> On Fri, Aug 30, 2013 at 03:46:49PM -0600, Jim Fehlig wrote:
>
>> Detect early on in libxl driver initialization if the driver
>> should be loaded at all, avoiding needless initialization steps
>> that only have to be undone later. While at it, move th
>> detection to a helper function to improve readability.
>>
>> After detecting that the driver should be loaded, subsequent
>> failures such as initializing the log stream, allocating libxl
>> ctx, etc. should be treated as failure to initialize the driver.
>>
>> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
>> ---
>> src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 40 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index ff4f6be..759fed5 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -775,34 +775,54 @@ libxlStateCleanup(void)
>> return 0;
>> }
>>
>> -static int
>> -libxlStateInitialize(bool privileged,
>> - virStateInhibitCallback callback ATTRIBUTE_UNUSED,
>> - void *opaque ATTRIBUTE_UNUSED)
>> +static bool
>> +libxlDriverShouldLoad(bool privileged)
>> {
>> - const libxl_version_info *ver_info;
>> - char *log_file = NULL;
>> + bool ret = false;
>> virCommandPtr cmd;
>> - int status, ret = 0;
>> - unsigned int free_mem;
>> - char ebuf[1024];
>> + int status;
>>
>> - /* Disable libxl driver if non-root */
>> + /* Don't load if non-root */
>> if (!privileged) {
>> VIR_INFO("Not running privileged, disabling libxenlight
driver");
>> - return 0;
>> + return ret;
>> + }
>> +
>> + /* Don't load if not running on a Xen control domain (dom0) */
>> + if (!virFileExists("/proc/xen/capabilities")) {
>> + VIR_INFO("No Xen capabilities detected, probably not running
"
>> + "in a Xen Dom0. Disabling libxenlight driver");
>> +
>> + return ret;
>> }
>>
>> - /* Disable driver if legacy xen toolstack (xend) is in use */
>> + /* Don't load if legacy xen toolstack (xend) is in use */
>>
>
> Indentation typo
>
>
>> cmd = virCommandNewArgList("/usr/sbin/xend", "status",
NULL);
>> if (virCommandRun(cmd, &status) == 0 && status == 0) {
>> VIR_INFO("Legacy xen tool stack seems to be in use, disabling
"
>> "libxenlight driver.");
>> - virCommandFree(cmd);
>> - return 0;
>> + } else {
>> + ret = true;
>> }
>> virCommandFree(cmd);
>>
>> + return ret;
>> +}
>> +
>> +static int
>> +libxlStateInitialize(bool privileged,
>> + virStateInhibitCallback callback ATTRIBUTE_UNUSED,
>> + void *opaque ATTRIBUTE_UNUSED)
>> +{
>> + const libxl_version_info *ver_info;
>> + char *log_file = NULL;
>> + int ret = 0;
>> + unsigned int free_mem;
>> + char ebuf[1024];
>> +
>> + if (!libxlDriverShouldLoad(privileged))
>> + return 0;
>> +
>> if (VIR_ALLOC(libxl_driver) < 0)
>> return -1;
>>
>> @@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged,
>> libxl_driver->logger =
>> (xentoollog_logger
*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
>> if (!libxl_driver->logger) {
>> - VIR_INFO("cannot create logger for libxenlight, disabling
driver");
>> - goto fail;
>> + VIR_ERROR(_("Failed to create logger for libxenlight"));
>> + goto error;
>> }
>>
>> if (libxl_ctx_alloc(&libxl_driver->ctx,
>> LIBXL_VERSION, 0,
>> libxl_driver->logger)) {
>> - VIR_INFO("cannot initialize libxenlight context, probably not
running "
>> - "in a Xen Dom0, disabling driver");
>> - goto fail;
>> + VIR_ERROR(_("Failed to initialize libxenlight context"));
>> + goto error;
>> }
>>
>> if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
>> - VIR_INFO("cannot version information from libxenlight, disabling
driver");
>> - goto fail;
>> + VIR_ERROR(_("Failed to get version information from
libxenlight"));
>>
>
>
> For the errors here, it is preferrable to use virReportError() which will
> in turn call VIR_ERROR anywway.
>
I'll change these to virReportError(). Is it generally preferable to
use virReportError() over VIR_ERROR? If so, I'll send a followup patch
to replace any remaining uses of VIR_ERROR.
Yep, originally we used VIR_ERROR in anything server side which was not
directly related to an RPC call, but we stopped having that requirement
a while back. So basically everything should use virReportError these
days and not VIR_ERROR.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 04/12] libxl: Add libxl_version_info to libxlDriverPrivate
libxl version info is static data as far as the libxl driver
is concerned, so retrieve this info when the driver is initialized
and stash it in the libxlDriverPrivate object. Subsequently use
the stashed info instead of repeatedly calling libxl_get_version_info().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +------
src/libxl/libxl_conf.h | 1 +
src/libxl/libxl_driver.c | 39 ++++++++-------------------------------
3 files changed, 10 insertions(+), 37 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f9ffe5d..81b4af4 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -981,21 +981,16 @@ error:
bool
libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
{
- const libxl_version_info *info;
regex_t regex;
int ret;
- info = libxl_get_version_info(driver->ctx);
- if (!info)
- return true; /* default to on */
-
ret = regcomp(®ex,
"(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
REG_NOSUB | REG_EXTENDED);
if (ret)
return true;
- ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ ret = regexec(®ex, driver->verInfo->commandline, 0, NULL, 0);
regfree(®ex);
return ret == REG_NOMATCH;
}
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 68e770c..be3a473 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -55,6 +55,7 @@ struct _libxlDriverPrivate {
virMutex lock;
virCapsPtr caps;
virDomainXMLOptionPtr xmlopt;
+ const libxl_version_info *verInfo;
unsigned int version;
/* log stream for driver-wide libxl ctx */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 759fed5..6fd9178 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -125,7 +125,6 @@ static int
libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
{
libxl_physinfo phy_info;
- const libxl_version_info* ver_info;
virArch hostarch = virArchFromHost();
if (libxl_get_physinfo(driver->ctx, &phy_info)) {
@@ -134,12 +133,6 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr
info)
return -1;
}
- if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("libxl_get_version_info failed"));
- return -1;
- }
-
if (virStrcpyStatic(info->model, virArchToString(hostarch)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("machine type %s too big for destination"),
@@ -147,7 +140,7 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
return -1;
}
- info->memory = phy_info.total_pages * (ver_info->pagesize / 1024);
+ info->memory = phy_info.total_pages * (driver->verInfo->pagesize / 1024);
info->cpus = phy_info.nr_cpus;
info->nodes = phy_info.nr_nodes;
info->cores = phy_info.cores_per_socket;
@@ -918,8 +911,9 @@ libxlStateInitialize(bool privileged,
VIR_ERROR(_("Failed to get version information from libxenlight"));
goto error;
}
+ libxl_driver->verInfo = ver_info;
libxl_driver->version = (ver_info->xen_version_major * 1000000) +
- (ver_info->xen_version_minor * 1000);
+ (ver_info->xen_version_minor * 1000);
if ((libxl_driver->caps =
libxlMakeCapabilities(libxl_driver->ctx)) == NULL) {
@@ -2711,7 +2705,6 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, const char *
nativeFormat,
unsigned int flags)
{
libxlDriverPrivatePtr driver = conn->privateData;
- const libxl_version_info *ver_info;
virDomainDefPtr def = NULL;
virConfPtr conf = NULL;
char *xml = NULL;
@@ -2727,15 +2720,12 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, const char *
nativeFormat,
goto cleanup;
}
- if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) {
- VIR_ERROR(_("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))) {
+ if (!(def = xenParseXM(conf,
+ driver->verInfo->xen_version_major,
+ driver->caps))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing xm config
failed"));
goto cleanup;
}
@@ -2756,7 +2746,6 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char *
nativeFormat,
unsigned int flags)
{
libxlDriverPrivatePtr driver = conn->privateData;
- const libxl_version_info *ver_info;
virDomainDefPtr def = NULL;
virConfPtr conf = NULL;
int len = MAX_CONFIG_SIZE;
@@ -2773,17 +2762,12 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char *
nativeFormat,
goto cleanup;
}
- if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) {
- VIR_ERROR(_("cannot get version information from libxenlight"));
- goto cleanup;
- }
-
if (!(def = virDomainDefParseString(domainXml,
driver->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN, 0)))
goto cleanup;
- if (!(conf = xenFormatXM(conn, def, ver_info->xen_version_major)))
+ if (!(conf = xenFormatXM(conn, def, driver->verInfo->xen_version_major)))
goto cleanup;
if (VIR_ALLOC_N(ret, len) < 0)
@@ -3685,7 +3669,6 @@ static unsigned long long
libxlNodeGetFreeMemory(virConnectPtr conn)
{
libxl_physinfo phy_info;
- const libxl_version_info* ver_info;
libxlDriverPrivatePtr driver = conn->privateData;
if (virNodeGetFreeMemoryEnsureACL(conn) < 0)
@@ -3697,13 +3680,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
return 0;
}
- if ((ver_info = libxl_get_version_info(driver->ctx)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("libxl_get_version_info failed"));
- return 0;
- }
-
- return phy_info.free_pages * ver_info->pagesize;
+ return phy_info.free_pages * driver->verInfo->pagesize;
}
static int
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 04/12] libxl: Add libxl_version_info to libxlDriverPrivate
On 30.08.2013 23:46, Jim Fehlig wrote:
libxl version info is static data as far as the libxl driver
is concerned, so retrieve this info when the driver is initialized
and stash it in the libxlDriverPrivate object. Subsequently use
the stashed info instead of repeatedly calling libxl_get_version_info().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +------
src/libxl/libxl_conf.h | 1 +
src/libxl/libxl_driver.c | 39 ++++++++-------------------------------
3 files changed, 10 insertions(+), 37 deletions(-)
ACK
Michal
6:26 a.m.
New subject: [libvirt] [PATCH 04/12] libxl: Add libxl_version_info to libxlDriverPrivate
On Fri, Aug 30, 2013 at 03:46:50PM -0600, Jim Fehlig wrote:
libxl version info is static data as far as the libxl driver
is concerned, so retrieve this info when the driver is initialized
and stash it in the libxlDriverPrivate object. Subsequently use
the stashed info instead of repeatedly calling libxl_get_version_info().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +------
src/libxl/libxl_conf.h | 1 +
src/libxl/libxl_driver.c | 39 ++++++++-------------------------------
3 files changed, 10 insertions(+), 37 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 05/12] libxl: libxl: Use per-domain ctx in libxlMakeDomCreateInfo
libxlMakeDomCreateInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 81b4af4..231a53d 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -395,7 +395,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
}
static int
-libxlMakeDomCreateInfo(libxlDriverPrivatePtr driver,
+libxlMakeDomCreateInfo(libxl_ctx *ctx,
virDomainDefPtr def,
libxl_domain_create_info *c_info)
{
@@ -413,7 +413,7 @@ libxlMakeDomCreateInfo(libxlDriverPrivatePtr driver,
if (def->nseclabels &&
def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_STATIC) {
- if (libxl_flask_context_to_sid(driver->ctx,
+ if (libxl_flask_context_to_sid(ctx,
def->seclabels[0]->label,
strlen(def->seclabels[0]->label),
&c_info->ssidref)) {
@@ -1024,10 +1024,11 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainObjPtr vm, libxl_domain_config *d_config)
{
virDomainDefPtr def = vm->def;
+ libxlDomainObjPrivatePtr priv = vm->privateData;
libxl_domain_config_init(d_config);
- if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
+ if (libxlMakeDomCreateInfo(priv->ctx, def, &d_config->c_info) < 0)
return -1;
if (libxlMakeDomBuildInfo(vm, d_config) < 0)
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 05/12] libxl: libxl: Use per-domain ctx in libxlMakeDomCreateInfo
On 30.08.2013 23:46, Jim Fehlig wrote:
libxlMakeDomCreateInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 81b4af4..231a53d 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -395,7 +395,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
}
static int
-libxlMakeDomCreateInfo(libxlDriverPrivatePtr driver,
+libxlMakeDomCreateInfo(libxl_ctx *ctx,
virDomainDefPtr def,
libxl_domain_create_info *c_info)
{
@@ -413,7 +413,7 @@ libxlMakeDomCreateInfo(libxlDriverPrivatePtr driver,
if (def->nseclabels &&
def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_STATIC) {
- if (libxl_flask_context_to_sid(driver->ctx,
+ if (libxl_flask_context_to_sid(ctx,
def->seclabels[0]->label,
strlen(def->seclabels[0]->label),
&c_info->ssidref)) {
@@ -1024,10 +1024,11 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainObjPtr vm, libxl_domain_config *d_config)
{
virDomainDefPtr def = vm->def;
+ libxlDomainObjPrivatePtr priv = vm->privateData;
libxl_domain_config_init(d_config);
- if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
+ if (libxlMakeDomCreateInfo(priv->ctx, def, &d_config->c_info) < 0)
return -1;
if (libxlMakeDomBuildInfo(vm, d_config) < 0)
ACK
Michal
6:28 a.m.
New subject: [libvirt] [PATCH 05/12] libxl: libxl: Use per-domain ctx in libxlMakeDomCreateInfo
On Fri, Aug 30, 2013 at 03:46:51PM -0600, Jim Fehlig wrote:
libxlMakeDomCreateInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
ACK
This would help any future work to make locking more fine grained.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 06/12] libxl: User per-domain ctx in libxlDomainGetInfo
libxlDomainGetInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6fd9178..a26fbf6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1818,6 +1818,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
libxl_dominfo d_info;
+ libxlDomainObjPrivatePtr priv;
int ret = -1;
libxlDriverLock(driver);
@@ -1833,12 +1834,13 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
if (virDomainGetInfoEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
+ priv = vm->privateData;
if (!virDomainObjIsActive(vm)) {
info->cpuTime = 0;
info->memory = vm->def->mem.cur_balloon;
info->maxMem = vm->def->mem.max_balloon;
} else {
- if (libxl_domain_info(driver->ctx, &d_info, dom->id) != 0) {
+ if (libxl_domain_info(priv->ctx, &d_info, dom->id) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxl_domain_info failed for domain
'%d'"), dom->id);
goto cleanup;
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 06/12] libxl: User per-domain ctx in libxlDomainGetInfo
On 30.08.2013 23:46, Jim Fehlig wrote:
libxlDomainGetInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6fd9178..a26fbf6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1818,6 +1818,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
libxl_dominfo d_info;
+ libxlDomainObjPrivatePtr priv;
int ret = -1;
libxlDriverLock(driver);
@@ -1833,12 +1834,13 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
if (virDomainGetInfoEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
+ priv = vm->privateData;
if (!virDomainObjIsActive(vm)) {
info->cpuTime = 0;
info->memory = vm->def->mem.cur_balloon;
info->maxMem = vm->def->mem.max_balloon;
} else {
- if (libxl_domain_info(driver->ctx, &d_info, dom->id) != 0) {
+ if (libxl_domain_info(priv->ctx, &d_info, dom->id) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxl_domain_info failed for domain
'%d'"), dom->id);
goto cleanup;
ACK
Michal
6:28 a.m.
New subject: [libvirt] [PATCH 06/12] libxl: User per-domain ctx in libxlDomainGetInfo
On Fri, Aug 30, 2013 at 03:46:52PM -0600, Jim Fehlig wrote:
libxlDomainGetInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object
The libxlDriverPrivate struct contains an variety of data with
varying access needs. Similar to the QEMU and LXC drivers,
move all the static config data into a dedicated libxlDriverConfig
object. The only locking requirement is to hold the driver lock
while obtaining an instance of libxlDriverConfig. Once a reference
is held on the config object, it can be used completely lockless
since it is immutable.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 124 ++++++++++++++++++-
src/libxl/libxl_conf.h | 52 +++++---
src/libxl/libxl_driver.c | 313 +++++++++++++++++++++++------------------------
3 files changed, 309 insertions(+), 180 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 231a53d..19fd8a6 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -64,6 +64,41 @@ static const char *xen_cap_re =
"(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(x86_32|x
static regex_t xen_cap_rec;
+static virClassPtr libxlDriverConfigClass;
+static void libxlDriverConfigDispose(void *obj);
+
+static int libxlConfigOnceInit(void)
+{
+ if (!(libxlDriverConfigClass = virClassNew(virClassForObject(),
+ "libxlDriverConfig",
+ sizeof(libxlDriverConfig),
+ libxlDriverConfigDispose)))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(libxlConfig)
+
+static void
+libxlDriverConfigDispose(void *obj)
+{
+ libxlDriverConfigPtr cfg = obj;
+
+ virObjectUnref(cfg->caps);
+ libxl_ctx_free(cfg->ctx);
+ xtl_logger_destroy(cfg->logger);
+ if (cfg->logger_file)
+ VIR_FORCE_FCLOSE(cfg->logger_file);
+
+ VIR_FREE(cfg->configDir);
+ VIR_FREE(cfg->autostartDir);
+ VIR_FREE(cfg->logDir);
+ VIR_FREE(cfg->stateDir);
+ VIR_FREE(cfg->libDir);
+ VIR_FREE(cfg->saveDir);
+}
+
static int
libxlCapsInitHost(libxl_ctx *ctx, virCapsPtr caps)
{
@@ -978,8 +1013,8 @@ error:
return -1;
}
-bool
-libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
+static bool
+libxlGetAutoballoonConf(libxlDriverConfigPtr cfg)
{
regex_t regex;
int ret;
@@ -990,11 +1025,94 @@ libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
if (ret)
return true;
- ret = regexec(®ex, driver->verInfo->commandline, 0, NULL, 0);
+ ret = regexec(®ex, cfg->verInfo->commandline, 0, NULL, 0);
regfree(®ex);
return ret == REG_NOMATCH;
}
+libxlDriverConfigPtr
+libxlDriverConfigNew(void)
+{
+ libxlDriverConfigPtr cfg;
+ char *log_file = NULL;
+ char ebuf[1024];
+ unsigned int free_mem;
+
+ if (libxlConfigInitialize() < 0)
+ return NULL;
+
+ if (!(cfg = virObjectNew(libxlDriverConfigClass)))
+ return NULL;
+
+ if (VIR_STRDUP(cfg->configDir, LIBXL_CONFIG_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->autostartDir, LIBXL_AUTOSTART_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->logDir, LIBXL_LOG_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->stateDir, LIBXL_STATE_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->libDir, LIBXL_LIB_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->saveDir, LIBXL_SAVE_DIR) < 0)
+ goto error;
+
+ if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) <
0)
+ goto error;
+
+ if ((cfg->logger_file = fopen(log_file, "a")) == NULL) {
+ VIR_ERROR(_("Failed to create log file '%s': %s"),
+ log_file, virStrerror(errno, ebuf, sizeof(ebuf)));
+ goto error;
+ }
+ VIR_FREE(log_file);
+
+ cfg->logger =
+ (xentoollog_logger *)xtl_createlogger_stdiostream(cfg->logger_file,
+ XTL_DEBUG, 0);
+ if (!cfg->logger) {
+ VIR_ERROR(_("cannot create logger for libxenlight, disabling
driver"));
+ goto error;
+ }
+
+ if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, cfg->logger)) {
+ VIR_ERROR(_("cannot initialize libxenlight context, probably not "
+ "running in a Xen Dom0, disabling driver"));
+ goto error;
+ }
+
+ if ((cfg->verInfo = libxl_get_version_info(cfg->ctx)) == NULL) {
+ VIR_ERROR(_("cannot version information from libxenlight, "
+ "disabling driver"));
+ goto error;
+ }
+ cfg->version = (cfg->verInfo->xen_version_major * 1000000) +
+ (cfg->verInfo->xen_version_minor * 1000);
+
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(cfg->ctx, &free_mem)) {
+ VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
+ goto error;
+ }
+
+ /* setup autoballoon */
+ cfg->autoballoon = libxlGetAutoballoonConf(cfg);
+
+ return cfg;
+
+error:
+ VIR_FREE(log_file);
+ virObjectUnref(cfg);
+ return NULL;
+}
+
+libxlDriverConfigPtr
+libxlDriverConfigGet(libxlDriverPrivatePtr driver)
+{
+ return virObjectRef(driver->config);
+}
+
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx)
{
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index be3a473..e3875ba 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -51,10 +51,13 @@
typedef struct _libxlDriverPrivate libxlDriverPrivate;
typedef libxlDriverPrivate *libxlDriverPrivatePtr;
-struct _libxlDriverPrivate {
- virMutex lock;
- virCapsPtr caps;
- virDomainXMLOptionPtr xmlopt;
+
+typedef struct _libxlDriverConfig libxlDriverConfig;
+typedef libxlDriverConfig *libxlDriverConfigPtr;
+
+struct _libxlDriverConfig {
+ virObject parent;
+
const libxl_version_info *verInfo;
unsigned int version;
@@ -64,27 +67,43 @@ struct _libxlDriverPrivate {
/* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */
libxl_ctx *ctx;
- virPortAllocatorPtr reservedVNCPorts;
-
/* Controls automatic ballooning of domain0. If true, attempt to get
* memory for new domains from domain0. */
bool autoballoon;
+ /* Once created, caps are immutable */
+ virCapsPtr caps;
+
+ char *configDir;
+ char *autostartDir;
+ char *logDir;
+ char *stateDir;
+ char *libDir;
+ char *saveDir;
+};
+
+
+struct _libxlDriverPrivate {
+ virMutex lock;
+
+ /* Require lock to get reference on 'config',
+ * then lockless thereafter */
+ libxlDriverConfigPtr config;
+
size_t nactive;
+
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
virDomainObjListPtr domains;
+ virDomainXMLOptionPtr xmlopt;
+
virDomainEventStatePtr domainEventState;
- virSysinfoDefPtr hostsysinfo;
- char *configDir;
- char *autostartDir;
- char *logDir;
- char *stateDir;
- char *libDir;
- char *saveDir;
+ virPortAllocatorPtr reservedVNCPorts;
+
+ virSysinfoDefPtr hostsysinfo;
};
typedef struct _libxlEventHookInfo libxlEventHookInfo;
@@ -103,8 +122,11 @@ struct _libxlSavefileHeader {
uint32_t unused[10];
};
-bool
-libxlGetAutoballoonConf(libxlDriverPrivatePtr driver);
+libxlDriverConfigPtr
+libxlDriverConfigNew(void);
+
+libxlDriverConfigPtr
+libxlDriverConfigGet(libxlDriverPrivatePtr driver);
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a26fbf6..e604899 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -126,35 +126,44 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr
info)
{
libxl_physinfo phy_info;
virArch hostarch = virArchFromHost();
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ int ret = -1;
- if (libxl_get_physinfo(driver->ctx, &phy_info)) {
+ if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("libxl_get_physinfo_info failed"));
- return -1;
+ goto cleanup;
}
if (virStrcpyStatic(info->model, virArchToString(hostarch)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("machine type %s too big for destination"),
virArchToString(hostarch));
- return -1;
+ goto cleanup;
}
- info->memory = phy_info.total_pages * (driver->verInfo->pagesize / 1024);
+ info->memory = phy_info.total_pages * (cfg->verInfo->pagesize / 1024);
info->cpus = phy_info.nr_cpus;
info->nodes = phy_info.nr_nodes;
info->cores = phy_info.cores_per_socket;
info->threads = phy_info.threads_per_core;
info->sockets = 1;
info->mhz = phy_info.cpu_khz / 1000;
- return 0;
+
+ ret = 0;
+
+cleanup:
+ virObjectUnref(cfg);
+ return ret;
}
static char *
libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) {
char *ret;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- ignore_value(virAsprintf(&ret, "%s/%s.save", driver->saveDir,
vm->def->name));
+ ignore_value(virAsprintf(&ret, "%s/%s.save", cfg->saveDir,
vm->def->name));
+ virObjectUnref(cfg);
return ret;
}
@@ -168,6 +177,8 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
char *xml = NULL;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ int ret = -1;
if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
virReportSystemError(-fd,
@@ -207,23 +218,25 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
goto error;
}
- if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
+ if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
goto error;
- VIR_FREE(xml);
-
*ret_def = def;
*ret_hdr = hdr;
- return fd;
+ ret = fd;
+ goto cleanup;
error:
- VIR_FREE(xml);
virDomainDefFree(def);
VIR_FORCE_CLOSE(fd);
- return -1;
+
+cleanup:
+ VIR_FREE(xml);
+ virObjectUnref(cfg);
+ return ret;
}
/*
@@ -237,6 +250,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
virDomainShutoffReason reason)
{
libxlDomainObjPrivatePtr priv = vm->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
int vnc_port;
char *file;
size_t i;
@@ -276,7 +290,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
vm->def->cputune.nvcpupin = 0;
}
- if (virAsprintf(&file, "%s/%s.xml", driver->stateDir,
vm->def->name) > 0) {
+ if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir,
vm->def->name) > 0) {
if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
VIR_DEBUG("Failed to remove domain XML for %s",
vm->def->name);
VIR_FREE(file);
@@ -290,6 +304,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
}
libxlDomainObjRegisteredTimeoutsCleanup(priv);
+ virObjectUnref(cfg);
}
/*
@@ -533,6 +548,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
char *managed_save_path = NULL;
int managed_save_fd = -1;
libxlDomainObjPrivatePtr priv = vm->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
if (libxlDomainObjPrivateInitCtx(vm) < 0)
goto error;
@@ -583,7 +599,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver, vm, &d_config) < 0)
goto error;
- if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
+ if (cfg->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to get free memory for domain
'%s'"),
d_config.c_info.name);
@@ -636,7 +652,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
}
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto error;
if (!driver->nactive && driver->inhibitCallback)
@@ -653,6 +669,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
libxl_domain_config_dispose(&d_config);
VIR_FREE(dom_xml);
VIR_FORCE_CLOSE(managed_save_fd);
+ virObjectUnref(cfg);
return 0;
error:
@@ -666,6 +683,7 @@ error:
VIR_FREE(managed_save_path);
virDomainDefFree(def);
VIR_FORCE_CLOSE(managed_save_fd);
+ virObjectUnref(cfg);
return -1;
}
@@ -740,28 +758,14 @@ libxlStateCleanup(void)
if (!libxl_driver)
return -1;
- libxlDriverLock(libxl_driver);
- virObjectUnref(libxl_driver->caps);
+ virObjectUnref(libxl_driver->config);
virObjectUnref(libxl_driver->xmlopt);
virObjectUnref(libxl_driver->domains);
- libxl_ctx_free(libxl_driver->ctx);
- xtl_logger_destroy(libxl_driver->logger);
- if (libxl_driver->logger_file)
- VIR_FORCE_FCLOSE(libxl_driver->logger_file);
-
virObjectUnref(libxl_driver->reservedVNCPorts);
- VIR_FREE(libxl_driver->configDir);
- VIR_FREE(libxl_driver->autostartDir);
- VIR_FREE(libxl_driver->logDir);
- VIR_FREE(libxl_driver->stateDir);
- VIR_FREE(libxl_driver->libDir);
- VIR_FREE(libxl_driver->saveDir);
-
virDomainEventStateFree(libxl_driver->domainEventState);
virSysinfoDefFree(libxl_driver->hostsysinfo);
- libxlDriverUnlock(libxl_driver);
virMutexDestroy(&libxl_driver->lock);
VIR_FREE(libxl_driver);
@@ -807,10 +811,7 @@ libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
{
- const libxl_version_info *ver_info;
- char *log_file = NULL;
- int ret = 0;
- unsigned int free_mem;
+ libxlDriverConfigPtr cfg;
char ebuf[1024];
if (!libxlDriverShouldLoad(privileged))
@@ -835,56 +836,31 @@ libxlStateInitialize(bool privileged,
if (!(libxl_driver->domains = virDomainObjListNew()))
goto error;
- if (VIR_STRDUP(libxl_driver->configDir, LIBXL_CONFIG_DIR) < 0)
- goto error;
-
- if (VIR_STRDUP(libxl_driver->autostartDir, LIBXL_AUTOSTART_DIR) < 0)
- goto error;
-
- if (VIR_STRDUP(libxl_driver->logDir, LIBXL_LOG_DIR) < 0)
- goto error;
-
- if (VIR_STRDUP(libxl_driver->stateDir, LIBXL_STATE_DIR) < 0)
- goto error;
-
- if (VIR_STRDUP(libxl_driver->libDir, LIBXL_LIB_DIR) < 0)
- goto error;
-
- if (VIR_STRDUP(libxl_driver->saveDir, LIBXL_SAVE_DIR) < 0)
+ if (!(cfg = libxlDriverConfigNew()))
goto error;
- if (virFileMakePath(libxl_driver->logDir) < 0) {
+ libxl_driver->config = cfg;
+ if (virFileMakePath(cfg->logDir) < 0) {
VIR_ERROR(_("Failed to create log dir '%s': %s"),
- libxl_driver->logDir, virStrerror(errno, ebuf, sizeof(ebuf)));
+ cfg->logDir, virStrerror(errno, ebuf, sizeof(ebuf)));
goto error;
}
- if (virFileMakePath(libxl_driver->stateDir) < 0) {
+ if (virFileMakePath(cfg->stateDir) < 0) {
VIR_ERROR(_("Failed to create state dir '%s': %s"),
- libxl_driver->stateDir, virStrerror(errno, ebuf, sizeof(ebuf)));
+ cfg->stateDir, virStrerror(errno, ebuf, sizeof(ebuf)));
goto error;
}
- if (virFileMakePath(libxl_driver->libDir) < 0) {
+ if (virFileMakePath(cfg->libDir) < 0) {
VIR_ERROR(_("Failed to create lib dir '%s': %s"),
- libxl_driver->libDir, virStrerror(errno, ebuf, sizeof(ebuf)));
+ cfg->libDir, virStrerror(errno, ebuf, sizeof(ebuf)));
goto error;
}
- if (virFileMakePath(libxl_driver->saveDir) < 0) {
+ if (virFileMakePath(cfg->saveDir) < 0) {
VIR_ERROR(_("Failed to create save dir '%s': %s"),
- libxl_driver->saveDir, virStrerror(errno, ebuf, sizeof(ebuf)));
+ cfg->saveDir, virStrerror(errno, ebuf, sizeof(ebuf)));
goto error;
}
- if (virAsprintf(&log_file, "%s/libxl-driver.log",
libxl_driver->logDir) < 0)
- goto error;
-
- if ((libxl_driver->logger_file = fopen(log_file, "a")) == NULL) {
- virReportSystemError(errno,
- _("failed to create logfile %s"),
- log_file);
- goto error;
- }
- VIR_FREE(log_file);
-
/* read the host sysinfo */
if (privileged)
libxl_driver->hostsysinfo = virSysinfoRead();
@@ -893,30 +869,7 @@ libxlStateInitialize(bool privileged,
if (!libxl_driver->domainEventState)
goto error;
- libxl_driver->logger =
- (xentoollog_logger
*)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
- if (!libxl_driver->logger) {
- VIR_ERROR(_("Failed to create logger for libxenlight"));
- goto error;
- }
-
- if (libxl_ctx_alloc(&libxl_driver->ctx,
- LIBXL_VERSION, 0,
- libxl_driver->logger)) {
- VIR_ERROR(_("Failed to initialize libxenlight context"));
- goto error;
- }
-
- if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
- VIR_ERROR(_("Failed to get version information from libxenlight"));
- goto error;
- }
- libxl_driver->verInfo = ver_info;
- libxl_driver->version = (ver_info->xen_version_major * 1000000) +
- (ver_info->xen_version_minor * 1000);
-
- if ((libxl_driver->caps =
- libxlMakeCapabilities(libxl_driver->ctx)) == NULL) {
+ if ((cfg->caps = libxlMakeCapabilities(cfg->ctx)) == NULL) {
VIR_ERROR(_("cannot create capabilities for libxenlight"));
goto error;
}
@@ -926,22 +879,12 @@ libxlStateInitialize(bool privileged,
NULL)))
goto error;
- /* This will fill xenstore info about free and dom0 memory if missing,
- * should be called before starting first domain */
- if (libxl_get_free_memory(libxl_driver->ctx, &free_mem)) {
- VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
- goto error;
- }
-
- /* setup autoballoon */
- libxl_driver->autoballoon = libxlGetAutoballoonConf(libxl_driver);
-
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
- libxl_driver->stateDir,
- libxl_driver->autostartDir,
+ cfg->stateDir,
+ cfg->autostartDir,
1,
- libxl_driver->caps,
+ cfg->caps,
libxl_driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
NULL, NULL) < 0)
@@ -951,10 +894,10 @@ libxlStateInitialize(bool privileged,
/* Then inactive persistent configs */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
- libxl_driver->configDir,
- libxl_driver->autostartDir,
+ cfg->configDir,
+ cfg->autostartDir,
0,
- libxl_driver->caps,
+ cfg->caps,
libxl_driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
NULL, NULL) < 0)
@@ -968,12 +911,10 @@ libxlStateInitialize(bool privileged,
return 0;
error:
- ret = -1;
- VIR_FREE(log_file);
if (libxl_driver)
libxlDriverUnlock(libxl_driver);
libxlStateCleanup();
- return ret;
+ return -1;
}
static void
@@ -991,15 +932,19 @@ libxlStateAutoStart(void)
static int
libxlStateReload(void)
{
+ libxlDriverConfigPtr cfg;
+
if (!libxl_driver)
return 0;
libxlDriverLock(libxl_driver);
+ cfg = libxlDriverConfigGet(libxl_driver);
+
virDomainObjListLoadAllConfigs(libxl_driver->domains,
- libxl_driver->configDir,
- libxl_driver->autostartDir,
+ cfg->configDir,
+ cfg->autostartDir,
1,
- libxl_driver->caps,
+ cfg->caps,
libxl_driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
NULL, libxl_driver);
@@ -1007,6 +952,7 @@ libxlStateReload(void)
virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
libxl_driver);
+ virObjectUnref(cfg);
libxlDriverUnlock(libxl_driver);
return 0;
@@ -1082,12 +1028,15 @@ static int
libxlConnectGetVersion(virConnectPtr conn, unsigned long *version)
{
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg;
if (virConnectGetVersionEnsureACL(conn) < 0)
return 0;
libxlDriverLock(driver);
- *version = driver->version;
+ cfg = libxlDriverConfigGet(driver);
+ *version = cfg->version;
+ virObjectUnref(cfg);
libxlDriverUnlock(driver);
return 0;
}
@@ -1132,16 +1081,19 @@ libxlConnectGetMaxVcpus(virConnectPtr conn, const char *type
ATTRIBUTE_UNUSED)
{
int ret;
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg;
if (virConnectGetMaxVcpusEnsureACL(conn) < 0)
return -1;
- ret = libxl_get_max_cpus(driver->ctx);
+ cfg = libxlDriverConfigGet(driver);
+ ret = libxl_get_max_cpus(cfg->ctx);
/* libxl_get_max_cpus() will return 0 if there were any failures,
e.g. xc_physinfo() failing */
if (ret == 0)
- return -1;
+ ret = -1;
+ virObjectUnref(cfg);
return ret;
}
@@ -1159,15 +1111,18 @@ libxlConnectGetCapabilities(virConnectPtr conn)
{
libxlDriverPrivatePtr driver = conn->privateData;
char *xml;
+ libxlDriverConfigPtr cfg;
if (virConnectGetCapabilitiesEnsureACL(conn) < 0)
return NULL;
libxlDriverLock(driver);
- if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL)
+ cfg = libxlDriverConfigGet(driver);
+ if ((xml = virCapabilitiesFormatXML(cfg->caps)) == NULL)
virReportOOMError();
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return xml;
}
@@ -1213,11 +1168,12 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
virDomainDefPtr def;
virDomainObjPtr vm = NULL;
virDomainPtr dom = NULL;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
libxlDriverLock(driver);
- if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
+ if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
@@ -1248,6 +1204,7 @@ cleanup:
if (vm)
virObjectUnlock(vm);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return dom;
}
@@ -1342,6 +1299,7 @@ static int
libxlDomainSuspend(virDomainPtr dom)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm;
libxlDomainObjPrivatePtr priv;
virDomainEventPtr event = NULL;
@@ -1383,7 +1341,7 @@ libxlDomainSuspend(virDomainPtr dom)
VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
}
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto cleanup;
ret = 0;
@@ -1396,6 +1354,7 @@ cleanup:
libxlDomainEventQueue(driver, event);
libxlDriverUnlock(driver);
}
+ virObjectUnref(cfg);
return ret;
}
@@ -1404,6 +1363,7 @@ static int
libxlDomainResume(virDomainPtr dom)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm;
libxlDomainObjPrivatePtr priv;
virDomainEventPtr event = NULL;
@@ -1446,7 +1406,7 @@ libxlDomainResume(virDomainPtr dom)
VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
}
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto cleanup;
ret = 0;
@@ -1459,6 +1419,7 @@ cleanup:
libxlDomainEventQueue(driver, event);
libxlDriverUnlock(driver);
}
+ virObjectUnref(cfg);
return ret;
}
@@ -1686,6 +1647,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
virDomainDefPtr persistentDef = NULL;
@@ -1735,7 +1697,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
_("cannot change persistent config of a transient
domain"));
goto cleanup;
}
- if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps,
+ if (!(persistentDef = virDomainObjGetPersistentDef(cfg->caps,
driver->xmlopt,
vm)))
goto cleanup;
@@ -1760,7 +1722,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
persistentDef->mem.max_balloon = newmem;
if (persistentDef->mem.cur_balloon > newmem)
persistentDef->mem.cur_balloon = newmem;
- ret = virDomainSaveConfig(driver->configDir, persistentDef);
+ ret = virDomainSaveConfig(cfg->configDir, persistentDef);
goto cleanup;
}
@@ -1787,7 +1749,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
if (flags & VIR_DOMAIN_MEM_CONFIG) {
sa_assert(persistentDef);
persistentDef->mem.cur_balloon = newmem;
- ret = virDomainSaveConfig(driver->configDir, persistentDef);
+ ret = virDomainSaveConfig(cfg->configDir, persistentDef);
goto cleanup;
}
}
@@ -1797,6 +1759,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
cleanup:
if (vm)
virObjectUnlock(vm);
+ virObjectUnref(cfg);
return ret;
}
@@ -2336,6 +2299,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
libxlDomainObjPrivatePtr priv;
virDomainDefPtr def;
virDomainObjPtr vm;
@@ -2409,7 +2373,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
priv = vm->privateData;
- if (!(def = virDomainObjGetPersistentDef(driver->caps, driver->xmlopt, vm)))
+ if (!(def = virDomainObjGetPersistentDef(cfg->caps, driver->xmlopt, vm)))
goto cleanup;
maplen = VIR_CPU_MAPLEN(nvcpus);
@@ -2458,12 +2422,13 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
ret = 0;
if (flags & VIR_DOMAIN_VCPU_CONFIG)
- ret = virDomainSaveConfig(driver->configDir, def);
+ ret = virDomainSaveConfig(cfg->configDir, def);
cleanup:
VIR_FREE(bitmask);
if (vm)
virObjectUnlock(vm);
+ virObjectUnref(cfg);
return ret;
}
@@ -2541,6 +2506,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned
char *cpumap,
int maplen)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
int ret = -1;
@@ -2589,7 +2555,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned
char *cpumap,
goto cleanup;
}
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto cleanup;
ret = 0;
@@ -2597,6 +2563,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned
char *cpumap,
cleanup:
if (vm)
virObjectUnlock(vm);
+ virObjectUnref(cfg);
return ret;
}
@@ -2707,6 +2674,7 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, const char *
nativeFormat,
unsigned int flags)
{
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainDefPtr def = NULL;
virConfPtr conf = NULL;
char *xml = NULL;
@@ -2726,8 +2694,8 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, const char *
nativeFormat,
goto cleanup;
if (!(def = xenParseXM(conf,
- driver->verInfo->xen_version_major,
- driver->caps))) {
+ cfg->verInfo->xen_version_major,
+ cfg->caps))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing xm config
failed"));
goto cleanup;
}
@@ -2738,6 +2706,7 @@ cleanup:
virDomainDefFree(def);
if (conf)
virConfFree(conf);
+ virObjectUnref(cfg);
return xml;
}
@@ -2748,6 +2717,7 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char *
nativeFormat,
unsigned int flags)
{
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainDefPtr def = NULL;
virConfPtr conf = NULL;
int len = MAX_CONFIG_SIZE;
@@ -2765,11 +2735,11 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char *
nativeFormat,
}
if (!(def = virDomainDefParseString(domainXml,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN, 0)))
goto cleanup;
- if (!(conf = xenFormatXM(conn, def, driver->verInfo->xen_version_major)))
+ if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major)))
goto cleanup;
if (VIR_ALLOC_N(ret, len) < 0)
@@ -2784,6 +2754,7 @@ cleanup:
virDomainDefFree(def);
if (conf)
virConfFree(conf);
+ virObjectUnref(cfg);
return ret;
}
@@ -2870,6 +2841,7 @@ static virDomainPtr
libxlDomainDefineXML(virConnectPtr conn, const char *xml)
{
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainDefPtr def = NULL;
virDomainObjPtr vm = NULL;
virDomainPtr dom = NULL;
@@ -2877,7 +2849,7 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml)
virDomainDefPtr oldDef = NULL;
libxlDriverLock(driver);
- if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
+ if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
@@ -2893,7 +2865,7 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml)
def = NULL;
vm->persistent = 1;
- if (virDomainSaveConfig(driver->configDir,
+ if (virDomainSaveConfig(cfg->configDir,
vm->newDef ? vm->newDef : vm->def) < 0) {
virDomainObjListRemove(driver->domains, vm);
vm = NULL;
@@ -2917,6 +2889,7 @@ cleanup:
if (event)
libxlDomainEventQueue(driver, event);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return dom;
}
@@ -2925,6 +2898,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm;
virDomainEventPtr event = NULL;
char *name = NULL;
@@ -2972,9 +2946,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
}
}
- if (virDomainDeleteConfig(driver->configDir,
- driver->autostartDir,
- vm) < 0)
+ if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
goto cleanup;
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED,
@@ -2996,6 +2968,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
if (event)
libxlDomainEventQueue(driver, event);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return ret;
}
@@ -3355,6 +3328,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm = NULL;
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL;
@@ -3399,12 +3373,12 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
/* Make a copy for updated domain. */
- if (!(vmdef = virDomainObjCopyPersistentDef(vm, driver->caps,
+ if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps,
driver->xmlopt)))
goto cleanup;
@@ -3418,7 +3392,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
/* If dev exists it was created to modify the domain config. Free it. */
virDomainDeviceDefFree(dev);
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
@@ -3429,13 +3403,13 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
* update domain status forcibly because the domain status may be
* changed even if we attach the device failed.
*/
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
ret = -1;
}
/* Finally, if no error until here, we can save config. */
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
- ret = virDomainSaveConfig(driver->configDir, vmdef);
+ ret = virDomainSaveConfig(cfg->configDir, vmdef);
if (!ret) {
virDomainObjAssignDef(vm, vmdef, false, NULL);
vmdef = NULL;
@@ -3448,6 +3422,7 @@ cleanup:
if (vm)
virObjectUnlock(vm);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return ret;
}
@@ -3463,6 +3438,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm = NULL;
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL;
@@ -3507,12 +3483,12 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
/* Make a copy for updated domain. */
- if (!(vmdef = virDomainObjCopyPersistentDef(vm, driver->caps,
+ if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps,
driver->xmlopt)))
goto cleanup;
@@ -3526,7 +3502,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
/* If dev exists it was created to modify the domain config. Free it. */
virDomainDeviceDefFree(dev);
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
@@ -3537,13 +3513,13 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
* update domain status forcibly because the domain status may be
* changed even if we attach the device failed.
*/
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
ret = -1;
}
/* Finally, if no error until here, we can save config. */
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
- ret = virDomainSaveConfig(driver->configDir, vmdef);
+ ret = virDomainSaveConfig(cfg->configDir, vmdef);
if (!ret) {
virDomainObjAssignDef(vm, vmdef, false, NULL);
vmdef = NULL;
@@ -3556,6 +3532,7 @@ cleanup:
if (vm)
virObjectUnlock(vm);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return ret;
}
@@ -3571,6 +3548,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm = NULL;
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL;
@@ -3615,12 +3593,12 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
/* Make a copy for updated domain. */
- if (!(vmdef = virDomainObjCopyPersistentDef(vm, driver->caps,
+ if (!(vmdef = virDomainObjCopyPersistentDef(vm, cfg->caps,
driver->xmlopt)))
goto cleanup;
@@ -3634,7 +3612,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
/* If dev exists it was created to modify the domain config. Free it. */
virDomainDeviceDefFree(dev);
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
- driver->caps, driver->xmlopt,
+ cfg->caps, driver->xmlopt,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
@@ -3645,13 +3623,13 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
* update domain status forcibly because the domain status may be
* changed even if we attach the device failed.
*/
- if (virDomainSaveStatus(driver->xmlopt, driver->stateDir, vm) < 0)
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
ret = -1;
}
/* Finally, if no error until here, we can save config. */
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
- ret = virDomainSaveConfig(driver->configDir, vmdef);
+ ret = virDomainSaveConfig(cfg->configDir, vmdef);
if (!ret) {
virDomainObjAssignDef(vm, vmdef, false, NULL);
vmdef = NULL;
@@ -3664,6 +3642,7 @@ cleanup:
if (vm)
virObjectUnlock(vm);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return ret;
}
@@ -3672,17 +3651,23 @@ libxlNodeGetFreeMemory(virConnectPtr conn)
{
libxl_physinfo phy_info;
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ unsigned long long ret = 0;
if (virNodeGetFreeMemoryEnsureACL(conn) < 0)
- return 0;
+ goto cleanup;
- if (libxl_get_physinfo(driver->ctx, &phy_info)) {
+ if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("libxl_get_physinfo_info failed"));
- return 0;
+ goto cleanup;
}
- return phy_info.free_pages * driver->verInfo->pagesize;
+ ret = phy_info.free_pages * cfg->verInfo->pagesize;
+
+cleanup:
+ virObjectUnref(cfg);
+ return ret;
}
static int
@@ -3695,11 +3680,12 @@ libxlNodeGetCellsFreeMemory(virConnectPtr conn,
int ret = -1, nr_nodes = 0;
libxl_numainfo *numa_info = NULL;
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
if (virNodeGetCellsFreeMemoryEnsureACL(conn) < 0)
- return -1;
+ goto cleanup;
- numa_info = libxl_get_numainfo(driver->ctx, &nr_nodes);
+ numa_info = libxl_get_numainfo(cfg->ctx, &nr_nodes);
if (numa_info == NULL || nr_nodes == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("libxl_get_numainfo failed"));
@@ -3728,6 +3714,7 @@ libxlNodeGetCellsFreeMemory(virConnectPtr conn,
cleanup:
libxl_numainfo_list_free(numa_info, nr_nodes);
+ virObjectUnref(cfg);
return ret;
}
@@ -3806,6 +3793,7 @@ static int
libxlDomainSetAutostart(virDomainPtr dom, int autostart)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm;
char *configFile = NULL, *autostartLink = NULL;
int ret = -1;
@@ -3833,16 +3821,16 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
autostart = (autostart != 0);
if (vm->autostart != autostart) {
- if (!(configFile = virDomainConfigFile(driver->configDir,
vm->def->name)))
+ if (!(configFile = virDomainConfigFile(cfg->configDir, vm->def->name)))
goto cleanup;
- if (!(autostartLink = virDomainConfigFile(driver->autostartDir,
vm->def->name)))
+ if (!(autostartLink = virDomainConfigFile(cfg->autostartDir,
vm->def->name)))
goto cleanup;
if (autostart) {
- if (virFileMakePath(driver->autostartDir) < 0) {
+ if (virFileMakePath(cfg->autostartDir) < 0) {
virReportSystemError(errno,
_("cannot create autostart directory
%s"),
- driver->autostartDir);
+ cfg->autostartDir);
goto cleanup;
}
@@ -3871,6 +3859,7 @@ cleanup:
if (vm)
virObjectUnlock(vm);
libxlDriverUnlock(driver);
+ virObjectUnref(cfg);
return ret;
}
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object
On 30.08.2013 23:46, Jim Fehlig wrote:
The libxlDriverPrivate struct contains an variety of data with
varying access needs. Similar to the QEMU and LXC drivers,
move all the static config data into a dedicated libxlDriverConfig
object. The only locking requirement is to hold the driver lock
while obtaining an instance of libxlDriverConfig. Once a reference
is held on the config object, it can be used completely lockless
since it is immutable.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 124 ++++++++++++++++++-
src/libxl/libxl_conf.h | 52 +++++---
src/libxl/libxl_driver.c | 313 +++++++++++++++++++++++------------------------
3 files changed, 309 insertions(+), 180 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 231a53d..19fd8a6 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -64,6 +64,41 @@ static const char *xen_cap_re =
"(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(x86_32|x
static regex_t xen_cap_rec;
+static virClassPtr libxlDriverConfigClass;
+static void libxlDriverConfigDispose(void *obj);
+
+static int libxlConfigOnceInit(void)
+{
+ if (!(libxlDriverConfigClass = virClassNew(virClassForObject(),
+ "libxlDriverConfig",
+ sizeof(libxlDriverConfig),
+ libxlDriverConfigDispose)))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(libxlConfig)
+
+static void
+libxlDriverConfigDispose(void *obj)
+{
+ libxlDriverConfigPtr cfg = obj;
+
+ virObjectUnref(cfg->caps);
+ libxl_ctx_free(cfg->ctx);
+ xtl_logger_destroy(cfg->logger);
+ if (cfg->logger_file)
+ VIR_FORCE_FCLOSE(cfg->logger_file);
+
+ VIR_FREE(cfg->configDir);
+ VIR_FREE(cfg->autostartDir);
+ VIR_FREE(cfg->logDir);
+ VIR_FREE(cfg->stateDir);
+ VIR_FREE(cfg->libDir);
+ VIR_FREE(cfg->saveDir);
+}
+
static int
libxlCapsInitHost(libxl_ctx *ctx, virCapsPtr caps)
{
@@ -978,8 +1013,8 @@ error:
return -1;
}
-bool
-libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
+static bool
+libxlGetAutoballoonConf(libxlDriverConfigPtr cfg)
{
regex_t regex;
int ret;
@@ -990,11 +1025,94 @@ libxlGetAutoballoonConf(libxlDriverPrivatePtr driver)
if (ret)
return true;
- ret = regexec(®ex, driver->verInfo->commandline, 0, NULL, 0);
+ ret = regexec(®ex, cfg->verInfo->commandline, 0, NULL, 0);
regfree(®ex);
return ret == REG_NOMATCH;
}
+libxlDriverConfigPtr
+libxlDriverConfigNew(void)
+{
+ libxlDriverConfigPtr cfg;
+ char *log_file = NULL;
+ char ebuf[1024];
+ unsigned int free_mem;
+
+ if (libxlConfigInitialize() < 0)
+ return NULL;
+
+ if (!(cfg = virObjectNew(libxlDriverConfigClass)))
+ return NULL;
+
+ if (VIR_STRDUP(cfg->configDir, LIBXL_CONFIG_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->autostartDir, LIBXL_AUTOSTART_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->logDir, LIBXL_LOG_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->stateDir, LIBXL_STATE_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->libDir, LIBXL_LIB_DIR) < 0)
+ goto error;
+ if (VIR_STRDUP(cfg->saveDir, LIBXL_SAVE_DIR) < 0)
+ goto error;
+
+ if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) <
0)
+ goto error;
+
+ if ((cfg->logger_file = fopen(log_file, "a")) == NULL) {
+ VIR_ERROR(_("Failed to create log file '%s': %s"),
+ log_file, virStrerror(errno, ebuf, sizeof(ebuf)));
+ goto error;
+ }
+ VIR_FREE(log_file);
+
+ cfg->logger =
+ (xentoollog_logger *)xtl_createlogger_stdiostream(cfg->logger_file,
+ XTL_DEBUG, 0);
+ if (!cfg->logger) {
+ VIR_ERROR(_("cannot create logger for libxenlight, disabling
driver"));
+ goto error;
+ }
+
+ if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, cfg->logger)) {
+ VIR_ERROR(_("cannot initialize libxenlight context, probably not "
+ "running in a Xen Dom0, disabling driver"));
+ goto error;
+ }
+
+ if ((cfg->verInfo = libxl_get_version_info(cfg->ctx)) == NULL) {
+ VIR_ERROR(_("cannot version information from libxenlight, "
+ "disabling driver"));
+ goto error;
+ }
+ cfg->version = (cfg->verInfo->xen_version_major * 1000000) +
+ (cfg->verInfo->xen_version_minor * 1000);
+
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(cfg->ctx, &free_mem)) {
+ VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
+ goto error;
+ }
+
+ /* setup autoballoon */
+ cfg->autoballoon = libxlGetAutoballoonConf(cfg);
+
+ return cfg;
+
+error:
+ VIR_FREE(log_file);
+ virObjectUnref(cfg);
+ return NULL;
+}
+
+libxlDriverConfigPtr
+libxlDriverConfigGet(libxlDriverPrivatePtr driver)
+{
+ return virObjectRef(driver->config);
+}
+
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx)
{
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index be3a473..e3875ba 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -51,10 +51,13 @@
typedef struct _libxlDriverPrivate libxlDriverPrivate;
typedef libxlDriverPrivate *libxlDriverPrivatePtr;
-struct _libxlDriverPrivate {
- virMutex lock;
- virCapsPtr caps;
- virDomainXMLOptionPtr xmlopt;
+
+typedef struct _libxlDriverConfig libxlDriverConfig;
+typedef libxlDriverConfig *libxlDriverConfigPtr;
+
+struct _libxlDriverConfig {
+ virObject parent;
+
const libxl_version_info *verInfo;
unsigned int version;
@@ -64,27 +67,43 @@ struct _libxlDriverPrivate {
/* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */
libxl_ctx *ctx;
- virPortAllocatorPtr reservedVNCPorts;
-
/* Controls automatic ballooning of domain0. If true, attempt to get
* memory for new domains from domain0. */
bool autoballoon;
+ /* Once created, caps are immutable */
+ virCapsPtr caps;
+
+ char *configDir;
+ char *autostartDir;
+ char *logDir;
+ char *stateDir;
+ char *libDir;
+ char *saveDir;
+};
+
+
+struct _libxlDriverPrivate {
+ virMutex lock;
+
+ /* Require lock to get reference on 'config',
+ * then lockless thereafter */
+ libxlDriverConfigPtr config;
+
size_t nactive;
+
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
virDomainObjListPtr domains;
+ virDomainXMLOptionPtr xmlopt;
+
virDomainEventStatePtr domainEventState;
- virSysinfoDefPtr hostsysinfo;
- char *configDir;
- char *autostartDir;
- char *logDir;
- char *stateDir;
- char *libDir;
- char *saveDir;
+ virPortAllocatorPtr reservedVNCPorts;
+
+ virSysinfoDefPtr hostsysinfo;
};
typedef struct _libxlEventHookInfo libxlEventHookInfo;
@@ -103,8 +122,11 @@ struct _libxlSavefileHeader {
uint32_t unused[10];
};
-bool
-libxlGetAutoballoonConf(libxlDriverPrivatePtr driver);
+libxlDriverConfigPtr
+libxlDriverConfigNew(void);
+
+libxlDriverConfigPtr
+libxlDriverConfigGet(libxlDriverPrivatePtr driver);
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a26fbf6..e604899 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -126,35 +126,44 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr
info)
{
libxl_physinfo phy_info;
virArch hostarch = virArchFromHost();
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ int ret = -1;
- if (libxl_get_physinfo(driver->ctx, &phy_info)) {
+ if (libxl_get_physinfo(cfg->ctx, &phy_info)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("libxl_get_physinfo_info failed"));
- return -1;
+ goto cleanup;
}
if (virStrcpyStatic(info->model, virArchToString(hostarch)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("machine type %s too big for destination"),
virArchToString(hostarch));
- return -1;
+ goto cleanup;
}
- info->memory = phy_info.total_pages * (driver->verInfo->pagesize / 1024);
+ info->memory = phy_info.total_pages * (cfg->verInfo->pagesize / 1024);
info->cpus = phy_info.nr_cpus;
info->nodes = phy_info.nr_nodes;
info->cores = phy_info.cores_per_socket;
info->threads = phy_info.threads_per_core;
info->sockets = 1;
info->mhz = phy_info.cpu_khz / 1000;
- return 0;
+
+ ret = 0;
+
+cleanup:
+ virObjectUnref(cfg);
+ return ret;
}
static char *
libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) {
char *ret;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- ignore_value(virAsprintf(&ret, "%s/%s.save", driver->saveDir,
vm->def->name));
+ ignore_value(virAsprintf(&ret, "%s/%s.save", cfg->saveDir,
vm->def->name));
+ virObjectUnref(cfg);
return ret;
}
@@ -168,6 +177,8 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
char *xml = NULL;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ int ret = -1;
if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
virReportSystemError(-fd,
@@ -207,23 +218,25 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
goto error;
}
- if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
+ if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
goto error;
- VIR_FREE(xml);
-
*ret_def = def;
*ret_hdr = hdr;
- return fd;
+ ret = fd;
+ goto cleanup;
error:
- VIR_FREE(xml);
virDomainDefFree(def);
VIR_FORCE_CLOSE(fd);
- return -1;
+
+cleanup:
+ VIR_FREE(xml);
+ virObjectUnref(cfg);
+ return ret;
In libvirt we rather have the 'error' label below the 'cleanup'. It's
more common to jump from the 'error' to 'cleanup' then. So can you
please swap these two?
ACK if you fix this.
Michal
6:16 p.m.
New subject: [libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object
Michal Privoznik wrote:
On 30.08.2013 23:46, Jim Fehlig wrote:
> The libxlDriverPrivate struct contains an variety of data with
> varying access needs. Similar to the QEMU and LXC drivers,
> move all the static config data into a dedicated libxlDriverConfig
> object. The only locking requirement is to hold the driver lock
> while obtaining an instance of libxlDriverConfig. Once a reference
> is held on the config object, it can be used completely lockless
> since it is immutable.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_conf.c | 124 ++++++++++++++++++-
> src/libxl/libxl_conf.h | 52 +++++---
> src/libxl/libxl_driver.c | 313 +++++++++++++++++++++++------------------------
> 3 files changed, 309 insertions(+), 180 deletions(-)
>
[...]
>
> @@ -168,6 +177,8 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char
*from,
> virDomainDefPtr def = NULL;
> libxlSavefileHeader hdr;
> char *xml = NULL;
> + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> + int ret = -1;
>
> if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
> virReportSystemError(-fd,
> @@ -207,23 +218,25 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char
*from,
> goto error;
> }
>
> - if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
> + if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
> 1 << VIR_DOMAIN_VIRT_XEN,
> VIR_DOMAIN_XML_INACTIVE)))
> goto error;
> [...]
>
>
> - VIR_FREE(xml);
> -
> *ret_def = def;
> *ret_hdr = hdr;
>
> - return fd;
> + ret = fd;
> + goto cleanup;
>
> error:
> - VIR_FREE(xml);
> virDomainDefFree(def);
> VIR_FORCE_CLOSE(fd);
> - return -1;
> +
> +cleanup:
> + VIR_FREE(xml);
> + virObjectUnref(cfg);
> + return ret;
>
In libvirt we rather have the 'error' label below the 'cleanup'.
It's
more common to jump from the 'error' to 'cleanup' then. So can you
please swap these two?
Hmm, looking at this again I think adding the cleanup label was
overkill. I've changed the above two hunks to the following, which is a
bit simpler.
@@ -168,6 +177,7 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
char *xml = NULL;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
virReportSystemError(-fd,
@@ -207,12 +217,13 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
goto error;
}
- if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt,
+ if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
goto error;
VIR_FREE(xml);
+ virObjectUnref(cfg);
*ret_def = def;
*ret_hdr = hdr;
@@ -221,6 +232,7 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
error:
VIR_FREE(xml);
+ virObjectUnref(cfg);
virDomainDefFree(def);
VIR_FORCE_CLOSE(fd);
return -1;
Regards,
Jim
6:30 a.m.
New subject: [libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object
On Fri, Aug 30, 2013 at 03:46:53PM -0600, Jim Fehlig wrote:
The libxlDriverPrivate struct contains an variety of data with
varying access needs. Similar to the QEMU and LXC drivers,
move all the static config data into a dedicated libxlDriverConfig
object. The only locking requirement is to hold the driver lock
while obtaining an instance of libxlDriverConfig. Once a reference
is held on the config object, it can be used completely lockless
since it is immutable.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 124 ++++++++++++++++++-
src/libxl/libxl_conf.h | 52 +++++---
src/libxl/libxl_driver.c | 313 +++++++++++++++++++++++------------------------
3 files changed, 309 insertions(+), 180 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 08/12] libxl: Use atomic ops for driver->nactive
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 2 +-
src/libxl/libxl_driver.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index e3875ba..83bb6b9 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -90,7 +90,7 @@ struct _libxlDriverPrivate {
* then lockless thereafter */
libxlDriverConfigPtr config;
- size_t nactive;
+ unsigned int nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e604899..7615cdd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -50,6 +50,7 @@
#include "virstring.h"
#include "virsysinfo.h"
#include "viraccessapicheck.h"
+#include "viratomic.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -265,8 +266,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
}
- driver->nactive--;
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntDecAndTest(&driver->nactive) &&
driver->inhibitCallback)
driver->inhibitCallback(false, driver->inhibitOpaque);
if ((vm->def->ngraphics == 1) &&
@@ -655,9 +655,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto error;
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
- driver->nactive++;
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
restore_fd < 0 ?
@@ -727,9 +726,8 @@ libxlReconnectDomain(virDomainObjPtr vm,
vm->def->id = d_info.domid;
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN);
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
- driver->nactive++;
/* Recreate domain death et. al. events */
libxlCreateDomEvents(vm);
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 08/12] libxl: Use atomic ops for driver->nactive
On 30.08.2013 23:46, Jim Fehlig wrote:
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 2 +-
src/libxl/libxl_driver.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index e3875ba..83bb6b9 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -90,7 +90,7 @@ struct _libxlDriverPrivate {
* then lockless thereafter */
libxlDriverConfigPtr config;
- size_t nactive;
+ unsigned int nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e604899..7615cdd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -50,6 +50,7 @@
#include "virstring.h"
#include "virsysinfo.h"
#include "viraccessapicheck.h"
+#include "viratomic.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -265,8 +266,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
}
- driver->nactive--;
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntDecAndTest(&driver->nactive) &&
driver->inhibitCallback)
driver->inhibitCallback(false, driver->inhibitOpaque);
if ((vm->def->ngraphics == 1) &&
@@ -655,9 +655,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto error;
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
Not exactly the same as previous code. Previously, the callback was
called iff nactive == 0; However, with your change the callback is
invoked each time the control gets to the 'if' (as virAtomicIntInc()
returns the *new* value after the increment). I think we want this to be:
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
- driver->nactive++;
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
restore_fd < 0 ?
@@ -727,9 +726,8 @@ libxlReconnectDomain(virDomainObjPtr vm,
vm->def->id = d_info.domid;
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN);
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
Same applies here.
driver->inhibitCallback(true, driver->inhibitOpaque);
- driver->nactive++;
/* Recreate domain death et. al. events */
libxlCreateDomEvents(vm);
ACK if you fix the issue.
Michal
6:18 p.m.
New subject: [libvirt] [PATCH 08/12] libxl: Use atomic ops for driver->nactive
Michal Privoznik wrote:
On 30.08.2013 23:46, Jim Fehlig wrote:
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_conf.h | 2 +-
> src/libxl/libxl_driver.c | 10 ++++------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index e3875ba..83bb6b9 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -90,7 +90,7 @@ struct _libxlDriverPrivate {
> * then lockless thereafter */
> libxlDriverConfigPtr config;
>
> - size_t nactive;
> + unsigned int nactive;
>
> virStateInhibitCallback inhibitCallback;
> void *inhibitOpaque;
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index e604899..7615cdd 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -50,6 +50,7 @@
> #include "virstring.h"
> #include "virsysinfo.h"
> #include "viraccessapicheck.h"
> +#include "viratomic.h"
>
> #define VIR_FROM_THIS VIR_FROM_LIBXL
>
> @@ -265,8 +266,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
> virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
> }
>
> - driver->nactive--;
> - if (!driver->nactive && driver->inhibitCallback)
> + if (virAtomicIntDecAndTest(&driver->nactive) &&
driver->inhibitCallback)
> driver->inhibitCallback(false, driver->inhibitOpaque);
>
> if ((vm->def->ngraphics == 1) &&
> @@ -655,9 +655,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> goto error;
>
> - if (!driver->nactive && driver->inhibitCallback)
> + if (virAtomicIntInc(&driver->nactive) &&
driver->inhibitCallback)
> driver->inhibitCallback(true, driver->inhibitOpaque);
>
Not exactly the same as previous code. Previously, the callback was
called iff nactive == 0; However, with your change the callback is
invoked each time the control gets to the 'if' (as virAtomicIntInc()
returns the *new* value after the increment). I think we want this to be:
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
Opps, thanks for catching that. I've fixed both instances you pointed out.
Regards,
Jim
6:30 a.m.
New subject: [libvirt] [PATCH 08/12] libxl: Use atomic ops for driver->nactive
On Fri, Aug 30, 2013 at 03:46:54PM -0600, Jim Fehlig wrote:
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 2 +-
src/libxl/libxl_driver.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index e3875ba..83bb6b9 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -90,7 +90,7 @@ struct _libxlDriverPrivate {
* then lockless thereafter */
libxlDriverConfigPtr config;
- size_t nactive;
+ unsigned int nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e604899..7615cdd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -50,6 +50,7 @@
#include "virstring.h"
#include "virsysinfo.h"
#include "viraccessapicheck.h"
+#include "viratomic.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -265,8 +266,7 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
}
- driver->nactive--;
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntDecAndTest(&driver->nactive) &&
driver->inhibitCallback)
driver->inhibitCallback(false, driver->inhibitOpaque);
if ((vm->def->ngraphics == 1) &&
@@ -655,9 +655,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
goto error;
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
- driver->nactive++;
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
restore_fd < 0 ?
@@ -727,9 +726,8 @@ libxlReconnectDomain(virDomainObjPtr vm,
vm->def->id = d_info.domid;
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN);
- if (!driver->nactive && driver->inhibitCallback)
+ if (virAtomicIntInc(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
- driver->nactive++;
/* Recreate domain death et. al. events */
libxlCreateDomEvents(vm);
ACK if you fix Michael's comments
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 09/12] libxl: Add comments to libxlDriverPrivate fields
Similar to the QEMU and LXC drivers, annotate the fields of
libxlDriverPrivate struct to indicate the locking rules for
their use.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 83bb6b9..95e0983 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -90,19 +90,26 @@ struct _libxlDriverPrivate {
* then lockless thereafter */
libxlDriverConfigPtr config;
+ /* Atomic inc/dec only */
unsigned int nactive;
+ /* Immutable pointers. Caller must provide locking */
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
+ /* Immutable pointer, self-locking APIs */
virDomainObjListPtr domains;
+ /* Immutable pointer, immutable object */
virDomainXMLOptionPtr xmlopt;
+ /* Immutable pointer, self-locking APIs */
virDomainEventStatePtr domainEventState;
+ /* Immutable pointer, self-locking APIs */
virPortAllocatorPtr reservedVNCPorts;
+ /* Immutable pointer, lockless APIs*/
virSysinfoDefPtr hostsysinfo;
};
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 09/12] libxl: Add comments to libxlDriverPrivate fields
On 30.08.2013 23:46, Jim Fehlig wrote:
Similar to the QEMU and LXC drivers, annotate the fields of
libxlDriverPrivate struct to indicate the locking rules for
their use.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 83bb6b9..95e0983 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -90,19 +90,26 @@ struct _libxlDriverPrivate {
* then lockless thereafter */
libxlDriverConfigPtr config;
+ /* Atomic inc/dec only */
unsigned int nactive;
+ /* Immutable pointers. Caller must provide locking */
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
+ /* Immutable pointer, self-locking APIs */
virDomainObjListPtr domains;
+ /* Immutable pointer, immutable object */
virDomainXMLOptionPtr xmlopt;
+ /* Immutable pointer, self-locking APIs */
virDomainEventStatePtr domainEventState;
+ /* Immutable pointer, self-locking APIs */
virPortAllocatorPtr reservedVNCPorts;
+ /* Immutable pointer, lockless APIs*/
virSysinfoDefPtr hostsysinfo;
};
ACK
Michal
6:31 a.m.
New subject: [libvirt] [PATCH 09/12] libxl: Add comments to libxlDriverPrivate fields
On Fri, Aug 30, 2013 at 03:46:55PM -0600, Jim Fehlig wrote:
Similar to the QEMU and LXC drivers, annotate the fields of
libxlDriverPrivate struct to indicate the locking rules for
their use.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 7 +++++++
1 file changed, 7 insertions(+)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 10/12] libxl: Move driver lock/unlock to libxl_conf
Move the libxl driver lock/unlock functions from libxl_driver.c
to libxl_conf.h so they can be used by other source files.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 12 ++++++++++++
src/libxl/libxl_driver.c | 12 ------------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 95e0983..174a759 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -150,4 +150,16 @@ int
libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainObjPtr vm, libxl_domain_config *d_config);
+static inline void
+libxlDriverLock(libxlDriverPrivatePtr driver)
+{
+ virMutexLock(&driver->lock);
+}
+
+static inline void
+libxlDriverUnlock(libxlDriverPrivatePtr driver)
+{
+ virMutexUnlock(&driver->lock);
+}
+
#endif /* LIBXL_CONF_H */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7615cdd..8ece4c9 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -77,18 +77,6 @@ static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
-static void
-libxlDriverLock(libxlDriverPrivatePtr driver)
-{
- virMutexLock(&driver->lock);
-}
-
-static void
-libxlDriverUnlock(libxlDriverPrivatePtr driver)
-{
- virMutexUnlock(&driver->lock);
-}
-
/* driver must be locked before calling */
static void
libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
--
1.8.1.4
6:08 a.m.
New subject: [libvirt] [PATCH 10/12] libxl: Move driver lock/unlock to libxl_conf
On 30.08.2013 23:46, Jim Fehlig wrote:
Move the libxl driver lock/unlock functions from libxl_driver.c
to libxl_conf.h so they can be used by other source files.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 12 ++++++++++++
src/libxl/libxl_driver.c | 12 ------------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 95e0983..174a759 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -150,4 +150,16 @@ int
libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainObjPtr vm, libxl_domain_config *d_config);
+static inline void
+libxlDriverLock(libxlDriverPrivatePtr driver)
+{
+ virMutexLock(&driver->lock);
+}
+
+static inline void
+libxlDriverUnlock(libxlDriverPrivatePtr driver)
+{
+ virMutexUnlock(&driver->lock);
+}
+
#endif /* LIBXL_CONF_H */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7615cdd..8ece4c9 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -77,18 +77,6 @@ static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
-static void
-libxlDriverLock(libxlDriverPrivatePtr driver)
-{
- virMutexLock(&driver->lock);
-}
-
-static void
-libxlDriverUnlock(libxlDriverPrivatePtr driver)
-{
- virMutexUnlock(&driver->lock);
-}
-
/* driver must be locked before calling */
static void
libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
ACK
Michal
6:31 a.m.
New subject: [libvirt] [PATCH 10/12] libxl: Move driver lock/unlock to libxl_conf
On Fri, Aug 30, 2013 at 03:46:56PM -0600, Jim Fehlig wrote:
Move the libxl driver lock/unlock functions from libxl_driver.c
to libxl_conf.h so they can be used by other source files.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 12 ++++++++++++
src/libxl/libxl_driver.c | 12 ------------
2 files changed, 12 insertions(+), 12 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:46 p.m.
New subject: [libvirt] [PATCH 11/12] libxl: Remove unnecessary driver locking
Now that most fields of libxlDriverPrivate struct are immutable
or self-locking, there is no need to acquire the driver lock in
much of the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +-
src/libxl/libxl_driver.c | 217 +++++++++--------------------------------------
2 files changed, 46 insertions(+), 178 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 19fd8a6..34f6bc1 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1110,7 +1110,12 @@ error:
libxlDriverConfigPtr
libxlDriverConfigGet(libxlDriverPrivatePtr driver)
{
- return virObjectRef(driver->config);
+ libxlDriverConfigPtr cfg;
+
+ libxlDriverLock(driver);
+ cfg = virObjectRef(driver->config);
+ libxlDriverUnlock(driver);
+ return cfg;
}
virCapsPtr
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8ece4c9..22bd26f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -77,7 +77,6 @@ static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
-/* driver must be locked before calling */
static void
libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
{
@@ -156,17 +155,21 @@ libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
virDomainObjPtr vm) {
return ret;
}
-/* This internal function expects the driver lock to already be held on
- * entry. */
-static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
- virDomainDefPtr *ret_def, libxlSavefileHeaderPtr ret_hdr)
+/*
+ * This internal function expects the driver lock to already be held on
+ * entry.
+ */
+static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
+libxlSaveImageOpen(libxlDriverPrivatePtr driver,
+ libxlDriverConfigPtr cfg,
+ const char *from,
+ virDomainDefPtr *ret_def,
+ libxlSavefileHeaderPtr ret_hdr)
{
int fd;
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
char *xml = NULL;
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
int ret = -1;
if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
@@ -350,10 +353,7 @@ libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event
*event)
if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND)
goto cleanup;
- libxlDriverLock(driver);
vm = virDomainObjListFindByID(driver->domains, event->domid);
- libxlDriverUnlock(driver);
-
if (!vm)
goto cleanup;
@@ -387,11 +387,8 @@ libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event
*event)
cleanup:
if (vm)
virObjectUnlock(vm);
- if (dom_event) {
- libxlDriverLock(driver);
+ if (dom_event)
libxlDomainEventQueue(driver, dom_event);
- libxlDriverUnlock(driver);
- }
/* Cast away any const */
libxl_event_free(priv->ctx, (libxl_event *)event);
}
@@ -550,7 +547,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (virFileExists(managed_save_path)) {
- managed_save_fd = libxlSaveImageOpen(driver, managed_save_path,
+ managed_save_fd = libxlSaveImageOpen(driver, cfg,
+ managed_save_path,
&def, &hdr);
if (managed_save_fd < 0)
goto error;
@@ -811,7 +809,6 @@ libxlStateInitialize(bool privileged,
VIR_FREE(libxl_driver);
return -1;
}
- libxlDriverLock(libxl_driver);
/* Allocate bitmap for vnc port reservation */
if (!(libxl_driver->reservedVNCPorts =
@@ -892,13 +889,9 @@ libxlStateInitialize(bool privileged,
virDomainObjListForEach(libxl_driver->domains, libxlDomainManagedSaveLoad,
libxl_driver);
- libxlDriverUnlock(libxl_driver);
-
return 0;
error:
- if (libxl_driver)
- libxlDriverUnlock(libxl_driver);
libxlStateCleanup();
return -1;
}
@@ -909,10 +902,8 @@ libxlStateAutoStart(void)
if (!libxl_driver)
return;
- libxlDriverLock(libxl_driver);
virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
libxl_driver);
- libxlDriverUnlock(libxl_driver);
}
static int
@@ -923,7 +914,6 @@ libxlStateReload(void)
if (!libxl_driver)
return 0;
- libxlDriverLock(libxl_driver);
cfg = libxlDriverConfigGet(libxl_driver);
virDomainObjListLoadAllConfigs(libxl_driver->domains,
@@ -939,8 +929,6 @@ libxlStateReload(void)
libxl_driver);
virObjectUnref(cfg);
- libxlDriverUnlock(libxl_driver);
-
return 0;
}
@@ -1019,11 +1007,9 @@ libxlConnectGetVersion(virConnectPtr conn, unsigned long *version)
if (virConnectGetVersionEnsureACL(conn) < 0)
return 0;
- libxlDriverLock(driver);
cfg = libxlDriverConfigGet(driver);
*version = cfg->version;
virObjectUnref(cfg);
- libxlDriverUnlock(driver);
return 0;
}
@@ -1102,11 +1088,9 @@ libxlConnectGetCapabilities(virConnectPtr conn)
if (virConnectGetCapabilitiesEnsureACL(conn) < 0)
return NULL;
- libxlDriverLock(driver);
cfg = libxlDriverConfigGet(driver);
if ((xml = virCapabilitiesFormatXML(cfg->caps)) == NULL)
virReportOOMError();
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return xml;
@@ -1121,10 +1105,8 @@ libxlConnectListDomains(virConnectPtr conn, int *ids, int nids)
if (virConnectListDomainsEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
n = virDomainObjListGetActiveIDs(driver->domains, ids, nids,
virConnectListDomainsCheckACL, conn);
- libxlDriverUnlock(driver);
return n;
}
@@ -1138,10 +1120,8 @@ libxlConnectNumOfDomains(virConnectPtr conn)
if (virConnectNumOfDomainsEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
n = virDomainObjListNumOfDomains(driver->domains, true,
virConnectNumOfDomainsCheckACL, conn);
- libxlDriverUnlock(driver);
return n;
}
@@ -1158,7 +1138,6 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL);
- libxlDriverLock(driver);
if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
@@ -1189,7 +1168,6 @@ cleanup:
virDomainDefFree(def);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return dom;
}
@@ -1201,10 +1179,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- libxlDriverLock(driver);
vm = virDomainObjListFindByID(driver->domains, id);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -1230,10 +1205,7 @@ libxlDomainLookupByUUID(virConnectPtr conn, const unsigned char
*uuid)
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -1259,10 +1231,7 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name)
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- libxlDriverLock(driver);
vm = virDomainObjListFindByName(driver->domains, name);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -1291,10 +1260,7 @@ libxlDomainSuspend(virDomainPtr dom)
virDomainEventPtr event = NULL;
int ret = -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -1335,11 +1301,8 @@ libxlDomainSuspend(virDomainPtr dom)
cleanup:
if (vm)
virObjectUnlock(vm);
- if (event) {
- libxlDriverLock(driver);
+ if (event)
libxlDomainEventQueue(driver, event);
- libxlDriverUnlock(driver);
- }
virObjectUnref(cfg);
return ret;
}
@@ -1355,10 +1318,7 @@ libxlDomainResume(virDomainPtr dom)
virDomainEventPtr event = NULL;
int ret = -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -1400,11 +1360,8 @@ libxlDomainResume(virDomainPtr dom)
cleanup:
if (vm)
virObjectUnlock(vm);
- if (event) {
- libxlDriverLock(driver);
+ if (event)
libxlDomainEventQueue(driver, event);
- libxlDriverUnlock(driver);
- }
virObjectUnref(cfg);
return ret;
}
@@ -1419,7 +1376,6 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1454,7 +1410,6 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
cleanup:
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
}
@@ -1475,7 +1430,6 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1506,7 +1460,6 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
cleanup:
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
}
@@ -1521,7 +1474,6 @@ libxlDomainDestroyFlags(virDomainPtr dom,
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1561,7 +1513,6 @@ cleanup:
virObjectUnlock(vm);
if (event)
libxlDomainEventQueue(driver, event);
- libxlDriverUnlock(driver);
return ret;
}
@@ -1578,9 +1529,7 @@ libxlDomainGetOSType(virDomainPtr dom)
virDomainObjPtr vm;
char *type = NULL;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -1608,10 +1557,7 @@ libxlDomainGetMaxMemory(virDomainPtr dom)
virDomainObjPtr vm;
unsigned long long ret = 0;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -1644,10 +1590,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
VIR_DOMAIN_MEM_CONFIG |
VIR_DOMAIN_MEM_MAXIMUM, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -1770,10 +1713,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
libxlDomainObjPrivatePtr priv;
int ret = -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s",
_("no domain with matching uuid"));
@@ -1821,10 +1761,7 @@ libxlDomainGetState(virDomainPtr dom,
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s",
_("no domain with matching uuid"));
@@ -1935,9 +1872,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char
*dxml,
return -1;
}
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -1967,7 +1902,6 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char
*dxml,
cleanup:
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
}
@@ -1982,6 +1916,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
const char *dxml, unsigned int flags)
{
libxlDriverPrivatePtr driver = conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm = NULL;
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
@@ -1995,22 +1930,26 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
return -1;
}
+ /* Lock the driver until domain def is read from the saved
+ image and a virDomainObj is created and locked.
+ */
libxlDriverLock(driver);
- fd = libxlSaveImageOpen(driver, from, &def, &hdr);
+ fd = libxlSaveImageOpen(driver, cfg, from, &def, &hdr);
if (fd < 0)
- goto cleanup;
+ goto cleanup_unlock;
if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
- goto cleanup;
+ goto cleanup_unlock;
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
- goto cleanup;
+ goto cleanup_unlock;
+ libxlDriverUnlock(driver);
def = NULL;
ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, fd);
@@ -2025,8 +1964,11 @@ cleanup:
virDomainDefFree(def);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
+
+cleanup_unlock:
+ libxlDriverUnlock(driver);
+ goto cleanup;
}
static int
@@ -2047,10 +1989,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int
flags)
virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -2089,12 +2028,11 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int
flags)
goto cleanup_unpause;
}
- libxlDriverLock(driver);
if (flags & VIR_DUMP_CRASH) {
if (libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to destroy domain '%d'"),
dom->id);
- goto cleanup_unlock;
+ goto cleanup_unpause;
}
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
@@ -2108,8 +2046,6 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int
flags)
ret = 0;
-cleanup_unlock:
- libxlDriverUnlock(driver);
cleanup_unpause:
if (virDomainObjIsActive(vm) && paused) {
if (libxl_domain_unpause(priv->ctx, dom->id) != 0) {
@@ -2124,11 +2060,8 @@ cleanup_unpause:
cleanup:
if (vm)
virObjectUnlock(vm);
- if (event) {
- libxlDriverLock(driver);
+ if (event)
libxlDomainEventQueue(driver, event);
- libxlDriverUnlock(driver);
- }
return ret;
}
@@ -2142,7 +2075,6 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -2184,7 +2116,6 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
cleanup:
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
VIR_FREE(name);
return ret;
}
@@ -2220,7 +2151,6 @@ libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int
flags)
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -2238,7 +2168,6 @@ libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int
flags)
cleanup:
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
}
@@ -2252,7 +2181,6 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -2276,7 +2204,6 @@ cleanup:
VIR_FREE(name);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
}
@@ -2316,10 +2243,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
return -1;
}
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -2437,10 +2361,7 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
VIR_DOMAIN_VCPU_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -2498,10 +2419,7 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned
char *cpumap,
int ret = -1;
libxl_bitmap map;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -2567,10 +2485,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int
maxinfo,
size_t i;
unsigned char *cpumap;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -2633,10 +2548,7 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
/* Flags checked by virDomainDefFormat */
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s",
_("no domain with matching uuid"));
@@ -2754,10 +2666,8 @@ libxlConnectListDefinedDomains(virConnectPtr conn,
if (virConnectListDefinedDomainsEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
n = virDomainObjListGetInactiveNames(driver->domains, names, nnames,
virConnectListDefinedDomainsCheckACL, conn);
- libxlDriverUnlock(driver);
return n;
}
@@ -2770,12 +2680,9 @@ libxlConnectNumOfDefinedDomains(virConnectPtr conn)
if (virConnectNumOfDefinedDomainsEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
n = virDomainObjListNumOfDomains(driver->domains, false,
virConnectNumOfDefinedDomainsCheckACL,
conn);
- libxlDriverUnlock(driver);
-
return n;
}
@@ -2789,7 +2696,6 @@ libxlDomainCreateWithFlags(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_START_PAUSED, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -2813,7 +2719,6 @@ libxlDomainCreateWithFlags(virDomainPtr dom,
cleanup:
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
return ret;
}
@@ -2834,22 +2739,25 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml)
virDomainEventPtr event = NULL;
virDomainDefPtr oldDef = NULL;
+ /* Lock the driver until the virDomainObj is created and locked */
libxlDriverLock(driver);
if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt,
1 << VIR_DOMAIN_VIRT_XEN,
VIR_DOMAIN_XML_INACTIVE)))
- goto cleanup;
+ goto cleanup_unlock;
if (virDomainDefineXMLEnsureACL(conn, def) < 0)
- goto cleanup;
+ goto cleanup_unlock;
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
0,
&oldDef)))
- goto cleanup;
+ goto cleanup_unlock;
+
def = NULL;
vm->persistent = 1;
+ libxlDriverUnlock(driver);
if (virDomainSaveConfig(cfg->configDir,
vm->newDef ? vm->newDef : vm->def) < 0) {
@@ -2874,9 +2782,12 @@ cleanup:
virObjectUnlock(vm);
if (event)
libxlDomainEventQueue(driver, event);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return dom;
+
+cleanup_unlock:
+ libxlDriverUnlock(driver);
+ goto cleanup;
}
static int
@@ -2892,9 +2803,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -2953,7 +2862,6 @@ libxlDomainUndefineFlags(virDomainPtr dom,
virObjectUnlock(vm);
if (event)
libxlDomainEventQueue(driver, event);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return ret;
}
@@ -3324,9 +3232,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -3407,7 +3313,6 @@ cleanup:
virDomainDeviceDefFree(dev);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return ret;
}
@@ -3434,9 +3339,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -3517,7 +3420,6 @@ cleanup:
virDomainDeviceDefFree(dev);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return ret;
}
@@ -3544,9 +3446,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -3627,7 +3527,6 @@ cleanup:
virDomainDeviceDefFree(dev);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return ret;
}
@@ -3715,11 +3614,9 @@ libxlConnectDomainEventRegister(virConnectPtr conn,
if (virConnectDomainEventRegisterEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
ret = virDomainEventStateRegister(conn,
driver->domainEventState,
callback, opaque, freecb);
- libxlDriverUnlock(driver);
return ret;
}
@@ -3735,11 +3632,9 @@ libxlConnectDomainEventDeregister(virConnectPtr conn,
if (virConnectDomainEventDeregisterEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
ret = virDomainEventStateDeregister(conn,
driver->domainEventState,
callback);
- libxlDriverUnlock(driver);
return ret;
}
@@ -3751,10 +3646,7 @@ libxlDomainGetAutostart(virDomainPtr dom, int *autostart)
virDomainObjPtr vm;
int ret = -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -3784,9 +3676,7 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
char *configFile = NULL, *autostartLink = NULL;
int ret = -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
-
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -3844,7 +3734,6 @@ cleanup:
VIR_FREE(autostartLink);
if (vm)
virObjectUnlock(vm);
- libxlDriverUnlock(driver);
virObjectUnref(cfg);
return ret;
}
@@ -3859,10 +3748,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
const char *name = NULL;
libxl_scheduler sched_id;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -3929,10 +3815,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
/* We don't return strings, and thus trivially support this flag. */
flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s",
_("no domain with matching uuid"));
@@ -4015,10 +3898,7 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
NULL) < 0)
return -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
goto cleanup;
@@ -4096,9 +3976,7 @@ libxlDomainOpenConsole(virDomainPtr dom,
goto cleanup;
}
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->uuid, uuidstr);
@@ -4199,10 +4077,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
* the filtering on behalf of older clients that can't parse it. */
flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
-
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, "%s",
_("no domain with matching uuid"));
@@ -4312,9 +4187,7 @@ libxlDomainIsActive(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
- libxlDriverLock(driver);
obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
if (!obj) {
virReportError(VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -4338,9 +4211,7 @@ libxlDomainIsPersistent(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
- libxlDriverLock(driver);
obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
if (!obj) {
virReportError(VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -4364,9 +4235,7 @@ libxlDomainIsUpdated(virDomainPtr dom)
virDomainObjPtr vm;
int ret = -1;
- libxlDriverLock(driver);
vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- libxlDriverUnlock(driver);
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -4394,13 +4263,11 @@ libxlConnectDomainEventRegisterAny(virConnectPtr conn,
virDomainPtr dom, int eve
if (virConnectDomainEventRegisterAnyEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
if (virDomainEventStateRegisterID(conn,
driver->domainEventState,
dom, eventID, callback, opaque,
freecb, &ret) < 0)
ret = -1;
- libxlDriverUnlock(driver);
return ret;
}
@@ -4415,11 +4282,9 @@ libxlConnectDomainEventDeregisterAny(virConnectPtr conn, int
callbackID)
if (virConnectDomainEventDeregisterAnyEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
ret = virDomainEventStateDeregisterID(conn,
driver->domainEventState,
callbackID);
- libxlDriverUnlock(driver);
return ret;
}
@@ -4444,10 +4309,8 @@ libxlConnectListAllDomains(virConnectPtr conn,
if (virConnectListAllDomainsEnsureACL(conn) < 0)
return -1;
- libxlDriverLock(driver);
ret = virDomainObjListExport(driver->domains, conn, domains,
virConnectListAllDomainsCheckACL, flags);
- libxlDriverUnlock(driver);
return ret;
}
--
1.8.1.4
6:07 a.m.
New subject: [libvirt] [PATCH 11/12] libxl: Remove unnecessary driver locking
On 30.08.2013 23:46, Jim Fehlig wrote:
Now that most fields of libxlDriverPrivate struct are immutable
or self-locking, there is no need to acquire the driver lock in
much of the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +-
src/libxl/libxl_driver.c | 217 +++++++++--------------------------------------
2 files changed, 46 insertions(+), 178 deletions(-)
ACK
Michal
6:32 a.m.
New subject: [libvirt] [PATCH 11/12] libxl: Remove unnecessary driver locking
On Fri, Aug 30, 2013 at 03:46:57PM -0600, Jim Fehlig wrote:
Now that most fields of libxlDriverPrivate struct are immutable
or self-locking, there is no need to acquire the driver lock in
much of the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +-
src/libxl/libxl_driver.c | 217 +++++++++--------------------------------------
2 files changed, 46 insertions(+), 178 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:25 p.m.
New subject: [libvirt] [PATCH 11/12] libxl: Remove unnecessary driver locking
Jim Fehlig wrote:
Now that most fields of libxlDriverPrivate struct are immutable
or self-locking, there is no need to acquire the driver lock in
much of the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 7 +-
src/libxl/libxl_driver.c | 217 +++++++++--------------------------------------
2 files changed, 46 insertions(+), 178 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 19fd8a6..34f6bc1 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1110,7 +1110,12 @@ error:
libxlDriverConfigPtr
libxlDriverConfigGet(libxlDriverPrivatePtr driver)
{
- return virObjectRef(driver->config);
+ libxlDriverConfigPtr cfg;
+
+ libxlDriverLock(driver);
+ cfg = virObjectRef(driver->config);
+ libxlDriverUnlock(driver);
+ return cfg;
}
virCapsPtr
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8ece4c9..22bd26f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -77,7 +77,6 @@ static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
-/* driver must be locked before calling */
static void
libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
{
@@ -156,17 +155,21 @@ libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
virDomainObjPtr vm) {
return ret;
}
-/* This internal function expects the driver lock to already be held on
- * entry. */
-static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
- virDomainDefPtr *ret_def, libxlSavefileHeaderPtr ret_hdr)
+/*
+ * This internal function expects the driver lock to already be held on
+ * entry.
+ */
+static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
+libxlSaveImageOpen(libxlDriverPrivatePtr driver,
+ libxlDriverConfigPtr cfg,
+ const char *from,
+ virDomainDefPtr *ret_def,
+ libxlSavefileHeaderPtr ret_hdr)
{
int fd;
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
char *xml = NULL;
- libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
While rebasing this patch after making the changes to 7 suggested by
Michal, I realized that unref'ing the cfg object also needs to be
removed since it is now passed to this function and unref'ed by its
callers. Squashing in the following hunks before pushing
@@ -212,7 +215,6 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
goto error;
VIR_FREE(xml);
- virObjectUnref(cfg);
*ret_def = def;
*ret_hdr = hdr;
@@ -221,7 +223,6 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
error:
VIR_FREE(xml);
- virObjectUnref(cfg);
virDomainDefFree(def);
VIR_FORCE_CLOSE(fd);
return -1;
Regards,
Jim
4:46 p.m.
New subject: [libvirt] [PATCH 12/12] libxl: Add libxlDomObjFromDomain
Similar to the QEMU and LXC drivers, add a helper function to
lookup a domain, and use it instead of much copy and paste.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 269 ++++++++++-------------------------------------
1 file changed, 56 insertions(+), 213 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 22bd26f..17d1960 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -77,6 +77,27 @@ static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
+
+/* Function definitions */
+static virDomainObjPtr
+libxlDomObjFromDomain(virDomainPtr dom)
+{
+ virDomainObjPtr vm;
+ libxlDriverPrivatePtr driver = dom->conn->privateData;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
+ if (!vm) {
+ virUUIDFormat(dom->uuid, uuidstr);
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s' (%s)"),
+ uuidstr, dom->name);
+ return NULL;
+ }
+
+ return vm;
+}
+
static void
libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
{
@@ -1260,14 +1281,8 @@ libxlDomainSuspend(virDomainPtr dom)
virDomainEventPtr event = NULL;
int ret = -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1318,14 +1333,8 @@ libxlDomainResume(virDomainPtr dom)
virDomainEventPtr event = NULL;
int ret = -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1369,21 +1378,14 @@ cleanup:
static int
libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
libxlDomainObjPrivatePtr priv;
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1423,21 +1425,14 @@ libxlDomainShutdown(virDomainPtr dom)
static int
libxlDomainReboot(virDomainPtr dom, unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
libxlDomainObjPrivatePtr priv;
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainRebootEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1474,14 +1469,8 @@ libxlDomainDestroyFlags(virDomainPtr dom,
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1525,18 +1514,11 @@ libxlDomainDestroy(virDomainPtr dom)
static char *
libxlDomainGetOSType(virDomainPtr dom)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
char *type = NULL;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetOSTypeEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1553,15 +1535,11 @@ cleanup:
static unsigned long long
libxlDomainGetMaxMemory(virDomainPtr dom)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
unsigned long long ret = 0;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1590,11 +1568,8 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
VIR_DOMAIN_MEM_CONFIG |
VIR_DOMAIN_MEM_MAXIMUM, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainSetMemoryFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -1707,18 +1682,13 @@ libxlDomainSetMaxMemory(virDomainPtr dom, unsigned long memory)
static int
libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
libxl_dominfo d_info;
libxlDomainObjPrivatePtr priv;
int ret = -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetInfoEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1755,18 +1725,13 @@ libxlDomainGetState(virDomainPtr dom,
int *reason,
unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetStateEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1872,14 +1837,8 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char
*dxml,
return -1;
}
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainSaveFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -1989,14 +1948,8 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int
flags)
virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainCoreDumpEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2075,14 +2028,8 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainManagedSaveEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2145,20 +2092,13 @@ cleanup:
static int
libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm = NULL;
int ret = -1;
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainHasManagedSaveImageEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2181,14 +2121,8 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
virCheckFlags(0, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainManagedSaveRemoveEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2243,11 +2177,8 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
return -1;
}
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -2351,7 +2282,6 @@ libxlDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
static int
libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
virDomainDefPtr def;
int ret = -1;
@@ -2361,11 +2291,8 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
VIR_DOMAIN_VCPU_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2419,11 +2346,8 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned
char *cpumap,
int ret = -1;
libxl_bitmap map;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainPinVcpuEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2476,7 +2400,6 @@ static int
libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo,
unsigned char *cpumaps, int maplen)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
int ret = -1;
@@ -2485,11 +2408,8 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int
maxinfo,
size_t i;
unsigned char *cpumap;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetVcpusEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2542,18 +2462,13 @@ cleanup:
static char *
libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
char *ret = NULL;
/* Flags checked by virDomainDefFormat */
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -2696,14 +2611,8 @@ libxlDomainCreateWithFlags(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_START_PAUSED, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -2803,15 +2712,8 @@ libxlDomainUndefineFlags(virDomainPtr dom,
virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
-
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -3232,11 +3134,8 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -3339,11 +3238,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -3446,11 +3342,8 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -3642,18 +3535,11 @@ libxlConnectDomainEventDeregister(virConnectPtr conn,
static int
libxlDomainGetAutostart(virDomainPtr dom, int *autostart)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetAutostartEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -3676,14 +3562,8 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart)
char *configFile = NULL, *autostartLink = NULL;
int ret = -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainSetAutostartEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -3741,18 +3621,14 @@ cleanup:
static char *
libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
char * ret = NULL;
const char *name = NULL;
libxl_scheduler sched_id;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetSchedulerTypeEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -3803,7 +3679,6 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
int *nparams,
unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
libxl_domain_sched_params sc_info;
@@ -3815,12 +3690,8 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
/* We don't return strings, and thus trivially support this flag. */
flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -3881,7 +3752,6 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
int nparams,
unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
libxl_domain_sched_params sc_info;
@@ -3898,11 +3768,8 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
NULL) < 0)
return -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching
uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainSetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def, flags)
< 0)
goto cleanup;
@@ -3960,7 +3827,6 @@ libxlDomainOpenConsole(virDomainPtr dom,
virStreamPtr st,
unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm = NULL;
int ret = -1;
virDomainChrDefPtr chr = NULL;
@@ -3976,14 +3842,8 @@ libxlDomainOpenConsole(virDomainPtr dom,
goto cleanup;
}
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("No domain with matching uuid '%s'"),
uuidstr);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -4059,7 +3919,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
int *nparams,
unsigned int flags)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDomainObjPrivatePtr priv;
virDomainObjPtr vm;
libxl_bitmap nodemap;
@@ -4077,12 +3936,8 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
* the filtering on behalf of older clients that can't parse it. */
flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, "%s",
- _("no domain with matching uuid"));
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -4183,15 +4038,11 @@ cleanup:
static int
libxlDomainIsActive(virDomainPtr dom)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr obj;
int ret = -1;
- obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
+ if (!(obj = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainIsActiveEnsureACL(dom->conn, obj->def) < 0)
goto cleanup;
@@ -4207,15 +4058,11 @@ libxlDomainIsActive(virDomainPtr dom)
static int
libxlDomainIsPersistent(virDomainPtr dom)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr obj;
int ret = -1;
- obj = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
+ if (!(obj = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainIsPersistentEnsureACL(dom->conn, obj->def) < 0)
goto cleanup;
@@ -4231,15 +4078,11 @@ libxlDomainIsPersistent(virDomainPtr dom)
static int
libxlDomainIsUpdated(virDomainPtr dom)
{
- libxlDriverPrivatePtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
- vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
- if (!vm) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
+ if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- }
if (virDomainIsUpdatedEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
--
1.8.1.4
6:09 a.m.
New subject: [libvirt] [PATCH 12/12] libxl: Add libxlDomObjFromDomain
On 30.08.2013 23:46, Jim Fehlig wrote:
Similar to the QEMU and LXC drivers, add a helper function to
lookup a domain, and use it instead of much copy and paste.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 269 ++++++++++-------------------------------------
1 file changed, 56 insertions(+), 213 deletions(-)
ACK
Michal
6:33 a.m.
New subject: [libvirt] [PATCH 12/12] libxl: Add libxlDomObjFromDomain
On Fri, Aug 30, 2013 at 03:46:58PM -0600, Jim Fehlig wrote:
Similar to the QEMU and LXC drivers, add a helper function to
lookup a domain, and use it instead of much copy and paste.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 269 ++++++++++-------------------------------------
1 file changed, 56 insertions(+), 213 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:57 p.m.
Jim Fehlig wrote:
This series contains several improvements for the libxl driver.
Patches 1 and 2 are primarily code movement, putting code in
files that are more consistent with other drivers.
Patches 3 through 12 improve locking of the libxlDriverPrivate
struct, similar to the work done in the QEMU and LXC drivers
Obviously post-1.1.2 material, but would be nice to get it in
soon thereafter for testing throughout the 1.1.3 cycle...
Jim Fehlig (12):
libxl: Move detection of autoballoon to libxl_conf
libxl: Introduce libxl_domain.[ch]
libxl: Earlier detection of not running on Xen
libxl: Add libxl_version_info to libxlDriverPrivate
libxl: libxl: Use per-domain ctx in libxlMakeDomCreateInfo
libxl: User per-domain ctx in libxlDomainGetInfo
libxl: Introduce libxlDriverConfig object
libxl: Use atomic ops for driver->nactive
libxl: Add comments to libxlDriverPrivate fields
libxl: Move driver lock/unlock to libxl_conf
libxl: Remove unnecessary driver locking
libxl: Add libxlDomObjFromDomain
I addressed the review comments from the various patches and pushed the
series. Thanks for the reviews!
Regards,
Jim
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libxl/libxl_conf.c | 149 +++++-
src/libxl/libxl_conf.h | 87 +--
src/libxl/libxl_domain.c | 469 ++++++++++++++++
src/libxl/libxl_domain.h | 61 +++
src/libxl/libxl_driver.c | 1329 ++++++++++------------------------------------
7 files changed, 1010 insertions(+), 1087 deletions(-)
create mode 100644 src/libxl/libxl_domain.c
create mode 100644 src/libxl/libxl_domain.h
6:20 a.m.
On 09/03/2013 07:57 PM, Jim Fehlig wrote:
Jim Fehlig wrote:
<...snip...>
I addressed the review comments from the various patches and pushed the
series. Thanks for the reviews!
Regards,
Jim
The changes have resulted in one Coverity found issue that seems to
have been there "for a while" (at least as far as git blame is concerned).
In libxlDomainCoreDump() Coverity has noted a FORWARD_NULL reference:
2004 if ((flags & VIR_DUMP_CRASH) && !vm->persistent) {
2005 virDomainObjListRemove(driver->domains, vm);
(20) Event assign_zero: Assigning: "vm" = "NULL".
Also see events: [var_deref_model]
2006 vm = NULL;
2007 }
2008
2009 ret = 0;
2010
2011 cleanup_unpause:
(21) Event var_deref_model: Passing null pointer "vm" to function
"virDomainObjIsActive(virDomainObjPtr)", which dereferences it. [details]
Also see events: [assign_zero]
2012 if (virDomainObjIsActive(vm) && paused) {
2013 if (libxl_domain_unpause(priv->ctx, dom->id) != 0) {
2014 virReportError(VIR_ERR_INTERNAL_ERROR,
2015 _("After dumping core, failed to resume domain
'%d' with"
Not quite sure which is the best course of action, but it seems
there needs to be at least a "if (vm && virDomainObjIsActive(vm)..."
Also, the ListRemove call could probably be moved up into the
previous if condition since they both require VIR_DUMP_CRASH
John
4:09 p.m.
John Ferlan wrote:
On 09/03/2013 07:57 PM, Jim Fehlig wrote:
> Jim Fehlig wrote:
>
<...snip...>
> I addressed the review comments from the various patches and pushed the
> series. Thanks for the reviews!
>
> Regards,
> Jim
>
>
The changes have resulted in one Coverity found issue that seems to
have been there "for a while" (at least as far as git blame is concerned).
In libxlDomainCoreDump() Coverity has noted a FORWARD_NULL reference:
2004 if ((flags & VIR_DUMP_CRASH) && !vm->persistent) {
2005 virDomainObjListRemove(driver->domains, vm);
(20) Event assign_zero: Assigning: "vm" = "NULL".
Also see events: [var_deref_model]
2006 vm = NULL;
2007 }
2008
2009 ret = 0;
2010
2011 cleanup_unpause:
(21) Event var_deref_model: Passing null pointer "vm" to function
"virDomainObjIsActive(virDomainObjPtr)", which dereferences it. [details]
Also see events: [assign_zero]
2012 if (virDomainObjIsActive(vm) && paused) {
2013 if (libxl_domain_unpause(priv->ctx, dom->id) != 0) {
2014 virReportError(VIR_ERR_INTERNAL_ERROR,
2015 _("After dumping core, failed to resume domain
'%d' with"
Not quite sure which is the best course of action, but it seems
there needs to be at least a "if (vm && virDomainObjIsActive(vm)..."
It took me a bit to figure out why the IsActive test is even needed, but
I think it is to handle !VIR_DUMP_LIVE and VIR_DUMP_CRASH. I.e. we
don't want to unpause a domain we just crashed. Checking for non-NULL
vm is certainly one way to fix the coverity warning.
Also, the ListRemove call could probably be moved up into the
previous if condition since they both require VIR_DUMP_CRASH
Thanks for pointing that out. What do you think of the attached patch?
Regards,
Jim
4:21 p.m.
On 09/04/2013 03:09 PM, Jim Fehlig wrote:
> >
Thanks for pointing that out. What do you think of the attached patch?
Regards,
Jim
libxl-coverity-fix.patch
>From fe87e804478689b7cdf0ab417b459cb0d2f6118e Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig(a)suse.com>
Date: Wed, 4 Sep 2013 14:27:20 -0600
Subject: [PATCH] libxl: Fix Coverity warning
John Ferlan reported the following Coverity warning:
In libxlDomainCoreDump() Coverity has noted a FORWARD_NULL reference:
Removing the vm from domain obj list and setting it to NULL can be
done in the previous 'if (flags & VIR_DUMP_CRASH)' conditional. Fix
the Coverity warning by ensuring vm is not NULL before testing if it
is still active.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:35 p.m.
Eric Blake wrote:
On 09/04/2013 03:09 PM, Jim Fehlig wrote:
>>>
>>>
> Thanks for pointing that out. What do you think of the attached patch?
>
> Regards,
> Jim
>
>
> libxl-coverity-fix.patch
>
>
> >From fe87e804478689b7cdf0ab417b459cb0d2f6118e Mon Sep 17 00:00:00 2001
> From: Jim Fehlig <jfehlig(a)suse.com>
> Date: Wed, 4 Sep 2013 14:27:20 -0600
> Subject: [PATCH] libxl: Fix Coverity warning
>
> John Ferlan reported the following Coverity warning:
>
> In libxlDomainCoreDump() Coverity has noted a FORWARD_NULL reference:
>
>
> Removing the vm from domain obj list and setting it to NULL can be
> done in the previous 'if (flags & VIR_DUMP_CRASH)' conditional. Fix
> the Coverity warning by ensuring vm is not NULL before testing if it
> is still active.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_driver.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
ACK.
Thanks, pushed.
Regards,
Jim
4096
days inactive
4101
days old
49 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Jim Fehlig -
John Ferlan -
Michal Privoznik