[libvirt] [PATCH 0/3] Support libvirt's lock manager in the libxl driver
Patch 1 relaxes the requirement of PID != 0 in the locking code. danpb and I discussed this on IRC: danpb: jimfehlig: it wouldn't be the end of the world if you leave pid as 0 danpb: jimfehlig: it'd just loose some safety sanity check in that scenario Patch 2 introduces a config file for the libxl driver, with the lone config entry 'autoballoon'. I thought this would make review easier, vs adding all the config file support in patch 3, which is fairly large as is. Jim Fehlig (3): locking: relax PID requirement libxl: Introduce configuration file for libxl driver libxl: provide integration with lock manager libvirt.spec.in | 8 ++++ src/Makefile.am | 36 ++++++++++++++++- src/libxl/libvirtd_libxl.aug | 44 +++++++++++++++++++++ src/libxl/libxl.conf | 22 +++++++++++ src/libxl/libxl_conf.c | 75 +++++++++++++++++++++++++++++++++--- src/libxl/libxl_conf.h | 11 ++++++ src/libxl/libxl_domain.c | 47 +++++++++++++++++++++- src/libxl/libxl_domain.h | 1 + src/libxl/libxl_driver.c | 34 ++++++++++++++++ src/libxl/libxl_migration.c | 6 +++ src/libxl/test_libvirtd_libxl.aug.in | 6 +++ src/locking/lock_daemon.c | 2 +- src/locking/lock_daemon_dispatch.c | 49 +++++++---------------- src/locking/lock_driver_lockd.c | 7 +--- 14 files changed, 298 insertions(+), 50 deletions(-) create mode 100644 src/libxl/libvirtd_libxl.aug create mode 100644 src/libxl/libxl.conf create mode 100644 src/libxl/test_libvirtd_libxl.aug.in -- 1.8.4.5
Some hypervisors like Xen do not have PIDs associated with domains. Relax the requirement for PID != 0 in the locking code so it can be used by hypervisors that do not represent domains as a process running on the host. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/locking/lock_daemon.c | 2 +- src/locking/lock_daemon_dispatch.c | 49 +++++++++++--------------------------- src/locking/lock_driver_lockd.c | 7 ++---- 3 files changed, 17 insertions(+), 41 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index bb165c0..042ff94 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -678,7 +678,7 @@ virLockDaemonClientFree(void *opaque) signum = SIGKILL; else signum = 0; - if (virProcessKill(priv->clientPid, signum) < 0) { + if (priv->clientPid != 0 && virProcessKill(priv->clientPid, signum) < 0) { if (errno == ESRCH) break; diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index a7cee9d..f2704ec 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -62,11 +62,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU goto cleanup; } - if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -120,11 +117,8 @@ virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server ATTRIBUTE_UNUS goto cleanup; } - if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -169,11 +163,8 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server ATTRIBUTE_UNUS goto cleanup; } - if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -218,11 +209,8 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); if (!args->path || STREQ(args->path, "")) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -273,11 +261,8 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - if (priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have already been registered")); - goto cleanup; - } + if (priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); if (VIR_STRDUP(priv->ownerName, args->owner.name) < 0) goto cleanup; @@ -320,11 +305,8 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server ATTRIBUTE_UNU goto cleanup; } - if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -370,11 +352,8 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; } - if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered"); priv->restricted = true; rv = 0; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 3a48a6a..109a79b 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -466,11 +466,8 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, _("Missing ID parameter for domain object")); return -1; } - if (priv->pid == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing PID parameter for domain object")); - return -1; - } + if (priv->pid == 0) + VIR_WARN("Missing PID parameter for domain object"); if (!priv->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing name parameter for domain object")); -- 1.8.4.5
On Fri, Apr 17, 2015 at 03:36:20PM -0600, Jim Fehlig wrote:
Some hypervisors like Xen do not have PIDs associated with domains. Relax the requirement for PID != 0 in the locking code so it can be used by hypervisors that do not represent domains as a process running on the host.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/locking/lock_daemon.c | 2 +- src/locking/lock_daemon_dispatch.c | 49 +++++++++++--------------------------- src/locking/lock_driver_lockd.c | 7 ++---- 3 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index bb165c0..042ff94 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -678,7 +678,7 @@ virLockDaemonClientFree(void *opaque) signum = SIGKILL; else signum = 0; - if (virProcessKill(priv->clientPid, signum) < 0) { + if (priv->clientPid != 0 && virProcessKill(priv->clientPid, signum) < 0) { if (errno == ESRCH) break;
diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index a7cee9d..f2704ec 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -62,11 +62,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU goto cleanup; }
- if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
In this file, all these ownerPid checks are really there to validate that the virLockSpaceProtocolDispatchRegister() method has been called first. I think we need to check that check, so rather than removing all these error reports, lets change them all to check priv->ownerId instead of priv->ownerPid.
if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -120,11 +117,8 @@ virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server ATTRIBUTE_UNUS goto cleanup; }
- if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -169,11 +163,8 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server ATTRIBUTE_UNUS goto cleanup; }
- if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -218,11 +209,8 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; }
- if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
if (!args->path || STREQ(args->path, "")) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -273,11 +261,8 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; }
- if (priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have already been registered")); - goto cleanup; - } + if (priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
if (VIR_STRDUP(priv->ownerName, args->owner.name) < 0) goto cleanup; @@ -320,11 +305,8 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server ATTRIBUTE_UNU goto cleanup; }
- if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -370,11 +352,8 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; }
- if (!priv->ownerPid) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("lock owner details have not been registered")); - goto cleanup; - } + if (!priv->ownerPid) + VIR_WARN("lock owner PID has not been registered");
priv->restricted = true; rv = 0; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 3a48a6a..109a79b 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -466,11 +466,8 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, _("Missing ID parameter for domain object")); return -1; } - if (priv->pid == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing PID parameter for domain object")); - return -1; - } + if (priv->pid == 0) + VIR_WARN("Missing PID parameter for domain object");
Lets just make that a VIR_DEBUG, since you don't want the log polluted with warnings every time a xen guest is started. Regards, 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 :|
Introduce libxl.conf configuration file, adding the 'autoballoon' setting as the first knob for controlling the libxl driver. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- libvirt.spec.in | 8 +++++ src/Makefile.am | 24 +++++++++++++- src/libxl/libvirtd_libxl.aug | 42 +++++++++++++++++++++++++ src/libxl/libxl.conf | 12 +++++++ src/libxl/libxl_conf.c | 61 ++++++++++++++++++++++++++++++++---- src/libxl/libxl_conf.h | 5 +++ src/libxl/libxl_driver.c | 9 ++++++ src/libxl/test_libvirtd_libxl.aug.in | 5 +++ 8 files changed, 159 insertions(+), 7 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index e08c9e7..5d8a9d4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1592,6 +1592,11 @@ rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/libvirtd.qemu rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/lxc.conf rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/libvirtd.lxc %endif +%if ! %{with_libxl} +rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/libxl.conf +rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirtd_libxl.aug +rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug +%endif %if ! %{with_uml} rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/libvirtd.uml %endif @@ -1997,9 +2002,12 @@ exit 0 %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/uml/ %endif %if %{with_libxl} +%config(noreplace) %{_sysconfdir}/libvirt/libxl.conf %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/libxl/ %ghost %dir %{_localstatedir}/run/libvirt/libxl/ %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/libxl/ +%{_datadir}/augeas/lenses/libvirtd_libxl.aug +%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug %endif %if %{with_xen} %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/xen/ diff --git a/src/Makefile.am b/src/Makefile.am index 3c9eac6..9a5f16c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1222,7 +1222,15 @@ libvirt_driver_libxl_impl_la_CFLAGS = \ libvirt_driver_libxl_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_libxl_impl_la_LIBADD = $(LIBXL_LIBS) libvirt_xenconfig.la libvirt_driver_libxl_impl_la_SOURCES = $(LIBXL_DRIVER_SOURCES) + +conf_DATA += libxl/libxl.conf +augeas_DATA += libxl/libvirtd_libxl.aug +augeastest_DATA += test_libvirtd_libxl.aug +CLEANFILES += test_libvirtd_libxl.aug + endif WITH_LIBXL +EXTRA_DIST += libxl/libxl.conf libxl/libvirtd_libxl.aug \ + libxl/test_libvirtd_libxl.aug.in if WITH_QEMU noinst_LTLIBRARIES += libvirt_driver_qemu_impl.la @@ -1785,10 +1793,11 @@ check-local: check-augeas check-augeas-lxc \ check-augeas-sanlock \ check-augeas-lockd \ + check-augeas-libxl \ $(NULL) check-augeas: check-augeas-qemu check-augeas-lxc check-augeas-sanlock \ - check-augeas-lockd check-augeas-virtlockd + check-augeas-lockd check-augeas-virtlockd check-augeas-libxl AUG_GENTEST = $(PERL) $(top_srcdir)/build-aux/augeas-gentest.pl EXTRA_DIST += $(top_srcdir)/build-aux/augeas-gentest.pl @@ -1858,6 +1867,19 @@ check-augeas-virtlockd: test_virtlockd.aug '$(AUGPARSE)' -I $(srcdir)/locking test_virtlockd.aug; \ fi +if WITH_LIBXL +test_libvirtd_libxl.aug: libxl/test_libvirtd_libxl.aug.in \ + $(srcdir)/libxl/libxl.conf $(AUG_GENTEST) + $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/libxl/libxl.conf $< $@ + +check-augeas-libxl: test_libvirtd_libxl.aug + $(AM_V_GEN)if test -x '$(AUGPARSE)'; then \ + '$(AUGPARSE)' -I $(srcdir)/libxl test_libvirtd_libxl.aug; \ + fi +else ! WITH_LIBXL +check-augeas-libxl: +endif ! WITH_LIBXL + # # Build our version script. This is composed of three parts: # diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug new file mode 100644 index 0000000..f225954 --- /dev/null +++ b/src/libxl/libvirtd_libxl.aug @@ -0,0 +1,42 @@ +(* /etc/libvirt/libxl.conf *) + +module Libvirtd_libxl = + autoload xfm + + let eol = del /[ \t]*\n/ "\n" + let value_sep = del /[ \t]*=[ \t]*/ " = " + let indent = del /[ \t]*/ "" + + let array_sep = del /,[ \t\n]*/ ", " + let array_start = del /\[[ \t\n]*/ "[ " + let array_end = del /\]/ "]" + + let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\"" + let bool_val = store /0|1/ + let int_val = store /[0-9]+/ + let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" + let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + + let str_entry (kw:string) = [ key kw . value_sep . str_val ] + let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] + let int_entry (kw:string) = [ key kw . value_sep . int_val ] + let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ] + + + (* Config entry grouped by function - same order as example config *) + let autoballoon_entry = bool_entry "autoballoon" + + (* Each entry in the config is one of the following ... *) + let entry = autoballoon_entry + + let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] + let empty = [ label "#empty" . eol ] + + let record = indent . entry . eol + + let lns = ( record | comment | empty ) * + + let filter = incl "/etc/libvirt/libxl.conf" + . Util.stdexcl + + let xfm = transform lns filter diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf new file mode 100644 index 0000000..c104d40 --- /dev/null +++ b/src/libxl/libxl.conf @@ -0,0 +1,12 @@ +# Master configuration file for the libxl driver. +# All settings described here are optional. If omitted, sensible +# defaults are used. + +# Enable autoballooning of domain0 +# +# By default, autoballooning of domain0 is enabled unless its memory +# is already limited with Xen's "dom0_mem=" parameter, in which case +# autoballooning is disabled. Override the default behavior with the +# autoballoon setting. +# +#autoballoon = 1 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 230daec..6ea2889 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -35,6 +35,7 @@ #include "virlog.h" #include "virerror.h" #include "datatypes.h" +#include "virconf.h" #include "virfile.h" #include "virstring.h" #include "viralloc.h" @@ -1348,12 +1349,33 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports, return -1; } +/* + * Get domain0 autoballoon configuration. Honor user-specified + * setting in libxl.conf first. If not specified, autoballooning + * is disabled when domain0's memory is set with 'dom0_mem'. + * Otherwise autoballooning is enabled. + */ static int -libxlGetAutoballoonConf(libxlDriverConfigPtr cfg, bool *autoballoon) +libxlGetAutoballoonConf(libxlDriverConfigPtr cfg, + virConfPtr conf) { + virConfValuePtr p; regex_t regex; int res; + p = virConfGetValue(conf, "autoballoon"); + if (p) { + if (p->type != VIR_CONF_ULONG) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Unexpected type for 'autoballoon' setting")); + + return -1; + } + cfg->autoballoon = p->l != 0; + return 0; + } + if ((res = regcomp(®ex, "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )", REG_NOSUB | REG_EXTENDED)) != 0) { @@ -1368,7 +1390,7 @@ libxlGetAutoballoonConf(libxlDriverConfigPtr cfg, bool *autoballoon) res = regexec(®ex, cfg->verInfo->commandline, 0, NULL, 0); regfree(®ex); - *autoballoon = res == REG_NOMATCH; + cfg->autoballoon = res == REG_NOMATCH; return 0; } @@ -1386,6 +1408,8 @@ libxlDriverConfigNew(void) if (!(cfg = virObjectNew(libxlDriverConfigClass))) return NULL; + if (VIR_STRDUP(cfg->configBaseDir, LIBXL_CONFIG_BASE_DIR) < 0) + goto error; if (VIR_STRDUP(cfg->configDir, LIBXL_CONFIG_DIR) < 0) goto error; if (VIR_STRDUP(cfg->autostartDir, LIBXL_AUTOSTART_DIR) < 0) @@ -1448,10 +1472,6 @@ libxlDriverConfigNew(void) goto error; } - /* setup autoballoon */ - if (libxlGetAutoballoonConf(cfg, &cfg->autoballoon) < 0) - goto error; - return cfg; error: @@ -1471,6 +1491,35 @@ libxlDriverConfigGet(libxlDriverPrivatePtr driver) return cfg; } +int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, + const char *filename) +{ + virConfPtr conf = NULL; + int ret = -1; + + /* Check the file is readable before opening it, otherwise + * libvirt emits an error. + */ + if (access(filename, R_OK) == -1) { + VIR_INFO("Could not read libxl config file %s", filename); + return 0; + } + + if (!(conf = virConfReadFile(filename, 0))) + goto cleanup; + + /* setup autoballoon */ + if (libxlGetAutoballoonConf(cfg, conf) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virConfFree(conf); + return ret; + +} + int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) { diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 59389d1..5ba1a71 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -46,6 +46,7 @@ # define LIBXL_MIGRATION_PORT_MIN 49152 # define LIBXL_MIGRATION_PORT_MAX 49216 +# define LIBXL_CONFIG_BASE_DIR SYSCONFDIR "/libvirt" # define LIBXL_CONFIG_DIR SYSCONFDIR "/libvirt/libxl" # define LIBXL_AUTOSTART_DIR LIBXL_CONFIG_DIR "/autostart" # define LIBXL_STATE_DIR LOCALSTATEDIR "/run/libvirt/libxl" @@ -100,6 +101,7 @@ struct _libxlDriverConfig { /* Once created, caps are immutable */ virCapsPtr caps; + char *configBaseDir; char *configDir; char *autostartDir; char *logDir; @@ -167,6 +169,9 @@ int libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info); +int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, + const char *filename); + virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9eb071e..d4be8b7 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -504,6 +504,7 @@ libxlStateInitialize(bool privileged, void *opaque ATTRIBUTE_UNUSED) { libxlDriverConfigPtr cfg; + char *driverConf = NULL; char ebuf[1024]; if (!libxlDriverShouldLoad(privileged)) @@ -543,6 +544,13 @@ libxlStateInitialize(bool privileged, if (!(cfg = libxlDriverConfigNew())) goto error; + if (virAsprintf(&driverConf, "%s/libxl.conf", cfg->configBaseDir) < 0) + goto error; + + if (libxlDriverConfigLoadFile(cfg, driverConf) < 0) + goto error; + VIR_FREE(driverConf); + /* Register the callbacks providing access to libvirt's event loop */ libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx); @@ -628,6 +636,7 @@ libxlStateInitialize(bool privileged, return 0; error: + VIR_FREE(driverConf); libxlStateCleanup(); return -1; } diff --git a/src/libxl/test_libvirtd_libxl.aug.in b/src/libxl/test_libvirtd_libxl.aug.in new file mode 100644 index 0000000..23e667c --- /dev/null +++ b/src/libxl/test_libvirtd_libxl.aug.in @@ -0,0 +1,5 @@ +module Test_libvirtd_libxl = + ::CONFIG:: + + test Libvirtd_libxl.lns get conf = +{ "autoballoon" = "1" } -- 1.8.4.5
On Fri, Apr 17, 2015 at 03:36:21PM -0600, Jim Fehlig wrote:
Introduce libxl.conf configuration file, adding the 'autoballoon' setting as the first knob for controlling the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- libvirt.spec.in | 8 +++++ src/Makefile.am | 24 +++++++++++++- src/libxl/libvirtd_libxl.aug | 42 +++++++++++++++++++++++++ src/libxl/libxl.conf | 12 +++++++ src/libxl/libxl_conf.c | 61 ++++++++++++++++++++++++++++++++---- src/libxl/libxl_conf.h | 5 +++ src/libxl/libxl_driver.c | 9 ++++++ src/libxl/test_libvirtd_libxl.aug.in | 5 +++ 8 files changed, 159 insertions(+), 7 deletions(-)
ACK Regards, 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 :|
Daniel P. Berrange wrote:
On Fri, Apr 17, 2015 at 03:36:21PM -0600, Jim Fehlig wrote:
Introduce libxl.conf configuration file, adding the 'autoballoon' setting as the first knob for controlling the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- libvirt.spec.in | 8 +++++ src/Makefile.am | 24 +++++++++++++- src/libxl/libvirtd_libxl.aug | 42 +++++++++++++++++++++++++ src/libxl/libxl.conf | 12 +++++++ src/libxl/libxl_conf.c | 61 ++++++++++++++++++++++++++++++++---- src/libxl/libxl_conf.h | 5 +++ src/libxl/libxl_driver.c | 9 ++++++ src/libxl/test_libvirtd_libxl.aug.in | 5 +++ 8 files changed, 159 insertions(+), 7 deletions(-)
ACK
Thanks! I've pushed this patch since it is independent of the other two. Regards, Jim
Provide integration with libvirt's lock manager in the libxl driver. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 12 +++++++++ src/libxl/libvirtd_libxl.aug | 2 ++ src/libxl/libxl.conf | 10 ++++++++ src/libxl/libxl_conf.c | 14 +++++++++++ src/libxl/libxl_conf.h | 6 +++++ src/libxl/libxl_domain.c | 47 ++++++++++++++++++++++++++++++++++-- src/libxl/libxl_domain.h | 1 + src/libxl/libxl_driver.c | 25 +++++++++++++++++++ src/libxl/libxl_migration.c | 6 +++++ src/libxl/test_libvirtd_libxl.aug.in | 1 + 10 files changed, 122 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 9a5f16c..1438174 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf DISTCLEANFILES += locking/qemu-lockd.conf endif WITH_QEMU +if WITH_LIBXL +nodist_conf_DATA += locking/libxl-lockd.conf +BUILT_SOURCES += locking/libxl-lockd.conf +DISTCLEANFILES += locking/libxl-lockd.conf +endif WITH_LIBXL + locking/%-lockd.conf: $(srcdir)/locking/lockd.conf $(AM_V_GEN)$(MKDIR_P) locking ; \ cp $< $@ @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf BUILT_SOURCES += locking/qemu-sanlock.conf DISTCLEANFILES += locking/qemu-sanlock.conf endif WITH_QEMU + +if WITH_LIBXL +nodist_conf_DATA += locking/libxl-sanlock.conf +BUILT_SOURCES += locking/libxl-sanlock.conf +DISTCLEANFILES += locking/libxl-sanlock.conf +endif WITH_LIBXL else ! WITH_SANLOCK EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) endif ! WITH_SANLOCK diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug index f225954..d5aa150 100644 --- a/src/libxl/libvirtd_libxl.aug +++ b/src/libxl/libvirtd_libxl.aug @@ -25,9 +25,11 @@ module Libvirtd_libxl = (* Config entry grouped by function - same order as example config *) let autoballoon_entry = bool_entry "autoballoon" + let lock_entry = str_entry "lock_manager" (* Each entry in the config is one of the following ... *) let entry = autoballoon_entry + | lock_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf index c104d40..ba3de7a 100644 --- a/src/libxl/libxl.conf +++ b/src/libxl/libxl.conf @@ -10,3 +10,13 @@ # autoballoon setting. # #autoballoon = 1 + + +# In order to prevent accidentally starting two domains that +# share one writable disk, libvirt offers two approaches for +# locking files: sanlock and virtlockd. sanlock is an external +# project which libvirt integrates with via the libvirt-lock-sanlock +# package. virtlockd is a libvirt implementation that is enabled with +# "lockd". Accepted values are "sanlock" and "lockd". +# +#lock_manager = "lockd" diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6ea2889..503e8a4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->libDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); + VIR_FREE(cfg->lockManagerName); } @@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, const char *filename) { virConfPtr conf = NULL; + virConfValuePtr p; int ret = -1; /* Check the file is readable before opening it, otherwise @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, if (libxlGetAutoballoonConf(cfg, conf) < 0) goto cleanup; + if ((p = virConfGetValue(conf, "lock_manager"))) { + if (p->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Unexpected type for 'lock_manager' setting")); + goto cleanup; + } + + if (VIR_STRDUP(cfg->lockManagerName, p->str) < 0) + goto cleanup; + } + ret = 0; cleanup: diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 5ba1a71..0a1c0db 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -38,6 +38,7 @@ # include "virobject.h" # include "virchrdev.h" # include "virhostdev.h" +# include "locking/lock_manager.h" # define LIBXL_DRIVER_NAME "xenlight" # define LIBXL_VNC_PORT_MIN 5900 @@ -98,6 +99,8 @@ struct _libxlDriverConfig { * memory for new domains from domain0. */ bool autoballoon; + char *lockManagerName; + /* Once created, caps are immutable */ virCapsPtr caps; @@ -144,6 +147,9 @@ struct _libxlDriverPrivate { /* Immutable pointer, lockless APIs*/ virSysinfoDefPtr hostsysinfo; + + /* Immutable pointer. lockless access */ + virLockManagerPluginPtr lockManager; }; # define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 77d46d0..70247f5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -34,6 +34,7 @@ #include "virlog.h" #include "virstring.h" #include "virtime.h" +#include "locking/domain_lock.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -217,12 +218,36 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data; + VIR_FREE(priv->lockState); virObjectUnref(priv); } +static int +libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + priv->lockState = virXPathString("string(./lockstate)", ctxt); + + return 0; +} + +static int +libxlDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + if (priv->lockState) + virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState); + + return 0; +} + virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = { .alloc = libxlDomainObjPrivateAlloc, .free = libxlDomainObjPrivateFree, + .parse = libxlDomainObjPrivateXMLParse, + .format = libxlDomainObjPrivateXMLFormat, }; @@ -668,6 +693,11 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, VIR_HOSTDEV_SP_PCI, NULL); + VIR_FREE(priv->lockState); + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); + vm->def->id = -1; if (priv->deathW) { @@ -961,6 +991,14 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, vm->def, VIR_HOSTDEV_SP_PCI) < 0) goto cleanup; + VIR_FREE(priv->lockState); + if (virDomainLockProcessStart(driver->lockManager, + "xen:///system", + vm, + false, + NULL) < 0) + goto cleanup; + /* Unlock virDomainObj while creating the domain */ virObjectUnlock(vm); @@ -991,7 +1029,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to restore domain '%s'"), d_config.c_info.name); - goto cleanup; + goto release_dom; } /* @@ -1004,6 +1042,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW)) goto cleanup_dom; + if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL) goto cleanup_dom; @@ -1025,7 +1064,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) - goto cleanup_dom; + goto release_dom; if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); @@ -1041,6 +1080,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup; cleanup_dom: + ret = -1; if (priv->deathW) { libxl_evdisable_domain_death(cfg->ctx, priv->deathW); priv->deathW = NULL; @@ -1049,6 +1089,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, vm->def->id = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); + release_dom: + virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState); + cleanup: libxl_domain_config_dispose(&d_config); VIR_FREE(dom_xml); diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index aa647b8..8c73cc4 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -64,6 +64,7 @@ struct _libxlDomainObjPrivate { virChrdevsPtr devs; libxl_evgen_domain_death *deathW; unsigned short migrationPort; + char *lockState; struct libxlDomainJobObj job; }; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d4be8b7..9cfb4e3 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -57,6 +57,7 @@ #include "viratomic.h" #include "virhostdev.h" #include "network/bridge_driver.h" +#include "locking/domain_lock.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -411,6 +412,7 @@ libxlStateCleanup(void) virObjectUnref(libxl_driver->domains); virObjectUnref(libxl_driver->reservedVNCPorts); virObjectUnref(libxl_driver->migrationPorts); + virLockManagerPluginUnref(libxl_driver->lockManager); virObjectEventStateFree(libxl_driver->domainEventState); virSysinfoDefFree(libxl_driver->hostsysinfo); @@ -590,6 +592,14 @@ libxlStateInitialize(bool privileged, goto error; } + if (!(libxl_driver->lockManager = + virLockManagerPluginNew(cfg->lockManagerName ? + cfg->lockManagerName : "nop", + "libxl", + cfg->configBaseDir, + 0))) + goto error; + /* read the host sysinfo */ libxl_driver->hostsysinfo = virSysinfoRead(); @@ -2866,11 +2876,21 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) if (libxlMakeDisk(l_disk, &x_disk) < 0) goto cleanup; + if (virDomainLockDiskAttach(libxl_driver->lockManager, + "xen:///system", + vm, l_disk) < 0) + goto cleanup; + if ((ret = libxl_device_disk_add(cfg->ctx, vm->def->id, &x_disk, NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to attach disk '%s'"), l_disk->dst); + if (virDomainLockDiskDetach(libxl_driver->lockManager, + vm, l_disk) < 0) { + VIR_WARN("Unable to release lock on %s", + virDomainDiskGetSource(l_disk)); + } goto cleanup; } @@ -3011,6 +3031,11 @@ libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) goto cleanup; } + if (virDomainLockDiskDetach(libxl_driver->lockManager, + vm, l_disk) < 0) + VIR_WARN("Unable to release lock on %s", + virDomainDiskGetSource(l_disk)); + virDomainDiskRemove(vm->def, idx); virDomainDiskDefFree(l_disk); diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 4010506..e7f1511 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -41,6 +41,7 @@ #include "libxl_driver.h" #include "libxl_conf.h" #include "libxl_migration.h" +#include "locking/domain_lock.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -471,6 +472,7 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, const char *dname ATTRIBUTE_UNUSED, unsigned int flags) { + libxlDomainObjPrivatePtr priv = vm->privateData; char *hostname = NULL; unsigned short port = 0; char portstr[100]; @@ -505,6 +507,10 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, sockfd = virNetSocketDupFD(sock, true); virObjectUnref(sock); + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); + /* suspend vm and send saved data to dst through socket fd */ virObjectUnlock(vm); ret = libxlDoMigrateSend(driver, vm, flags, sockfd); diff --git a/src/libxl/test_libvirtd_libxl.aug.in b/src/libxl/test_libvirtd_libxl.aug.in index 23e667c..baa8c79 100644 --- a/src/libxl/test_libvirtd_libxl.aug.in +++ b/src/libxl/test_libvirtd_libxl.aug.in @@ -3,3 +3,4 @@ module Test_libvirtd_libxl = test Libvirtd_libxl.lns get conf = { "autoballoon" = "1" } +{ "lock_manager" = "lockd" } -- 1.8.4.5
On Fri, Apr 17, 2015 at 03:36:22PM -0600, Jim Fehlig wrote:
Provide integration with libvirt's lock manager in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 12 +++++++++ src/libxl/libvirtd_libxl.aug | 2 ++ src/libxl/libxl.conf | 10 ++++++++ src/libxl/libxl_conf.c | 14 +++++++++++ src/libxl/libxl_conf.h | 6 +++++ src/libxl/libxl_domain.c | 47 ++++++++++++++++++++++++++++++++++-- src/libxl/libxl_domain.h | 1 + src/libxl/libxl_driver.c | 25 +++++++++++++++++++ src/libxl/libxl_migration.c | 6 +++++ src/libxl/test_libvirtd_libxl.aug.in | 1 + 10 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index 9a5f16c..1438174 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf DISTCLEANFILES += locking/qemu-lockd.conf endif WITH_QEMU
+if WITH_LIBXL +nodist_conf_DATA += locking/libxl-lockd.conf +BUILT_SOURCES += locking/libxl-lockd.conf +DISTCLEANFILES += locking/libxl-lockd.conf +endif WITH_LIBXL + locking/%-lockd.conf: $(srcdir)/locking/lockd.conf $(AM_V_GEN)$(MKDIR_P) locking ; \ cp $< $@ @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf BUILT_SOURCES += locking/qemu-sanlock.conf DISTCLEANFILES += locking/qemu-sanlock.conf endif WITH_QEMU + +if WITH_LIBXL +nodist_conf_DATA += locking/libxl-sanlock.conf +BUILT_SOURCES += locking/libxl-sanlock.conf +DISTCLEANFILES += locking/libxl-sanlock.conf +endif WITH_LIBXL else ! WITH_SANLOCK EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) endif ! WITH_SANLOCK diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug index f225954..d5aa150 100644 --- a/src/libxl/libvirtd_libxl.aug +++ b/src/libxl/libvirtd_libxl.aug @@ -25,9 +25,11 @@ module Libvirtd_libxl =
(* Config entry grouped by function - same order as example config *) let autoballoon_entry = bool_entry "autoballoon" + let lock_entry = str_entry "lock_manager"
(* Each entry in the config is one of the following ... *) let entry = autoballoon_entry + | lock_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf index c104d40..ba3de7a 100644 --- a/src/libxl/libxl.conf +++ b/src/libxl/libxl.conf @@ -10,3 +10,13 @@ # autoballoon setting. # #autoballoon = 1 + + +# In order to prevent accidentally starting two domains that +# share one writable disk, libvirt offers two approaches for +# locking files: sanlock and virtlockd. sanlock is an external +# project which libvirt integrates with via the libvirt-lock-sanlock +# package. virtlockd is a libvirt implementation that is enabled with +# "lockd". Accepted values are "sanlock" and "lockd". +# +#lock_manager = "lockd" diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6ea2889..503e8a4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->libDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); + VIR_FREE(cfg->lockManagerName); }
@@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, const char *filename) { virConfPtr conf = NULL; + virConfValuePtr p; int ret = -1;
/* Check the file is readable before opening it, otherwise @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, if (libxlGetAutoballoonConf(cfg, conf) < 0) goto cleanup;
+ if ((p = virConfGetValue(conf, "lock_manager"))) { + if (p->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Unexpected type for 'lock_manager' setting")); + goto cleanup; + } + + if (VIR_STRDUP(cfg->lockManagerName, p->str) < 0) + goto cleanup; + } + ret = 0;
cleanup: diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 5ba1a71..0a1c0db 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -38,6 +38,7 @@ # include "virobject.h" # include "virchrdev.h" # include "virhostdev.h" +# include "locking/lock_manager.h"
# define LIBXL_DRIVER_NAME "xenlight" # define LIBXL_VNC_PORT_MIN 5900 @@ -98,6 +99,8 @@ struct _libxlDriverConfig { * memory for new domains from domain0. */ bool autoballoon;
+ char *lockManagerName; + /* Once created, caps are immutable */ virCapsPtr caps;
@@ -144,6 +147,9 @@ struct _libxlDriverPrivate {
/* Immutable pointer, lockless APIs*/ virSysinfoDefPtr hostsysinfo; + + /* Immutable pointer. lockless access */ + virLockManagerPluginPtr lockManager; };
# define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 77d46d0..70247f5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -34,6 +34,7 @@ #include "virlog.h" #include "virstring.h" #include "virtime.h" +#include "locking/domain_lock.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -217,12 +218,36 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data;
+ VIR_FREE(priv->lockState); virObjectUnref(priv); }
+static int +libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + priv->lockState = virXPathString("string(./lockstate)", ctxt); + + return 0; +} + +static int +libxlDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + if (priv->lockState) + virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState); + + return 0; +} + virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = { .alloc = libxlDomainObjPrivateAlloc, .free = libxlDomainObjPrivateFree, + .parse = libxlDomainObjPrivateXMLParse, + .format = libxlDomainObjPrivateXMLFormat, };
@@ -668,6 +693,11 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, VIR_HOSTDEV_SP_PCI, NULL);
+ VIR_FREE(priv->lockState); + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
I see a few calls to ProcessPause, but I'm not seeing any to ProcessResume, so I think you might have missed a few places needing hooks. Regards, 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 :|
Daniel P. Berrange wrote:
On Fri, Apr 17, 2015 at 03:36:22PM -0600, Jim Fehlig wrote:
Provide integration with libvirt's lock manager in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 12 +++++++++ src/libxl/libvirtd_libxl.aug | 2 ++ src/libxl/libxl.conf | 10 ++++++++ src/libxl/libxl_conf.c | 14 +++++++++++ src/libxl/libxl_conf.h | 6 +++++ src/libxl/libxl_domain.c | 47 ++++++++++++++++++++++++++++++++++-- src/libxl/libxl_domain.h | 1 + src/libxl/libxl_driver.c | 25 +++++++++++++++++++ src/libxl/libxl_migration.c | 6 +++++ src/libxl/test_libvirtd_libxl.aug.in | 1 + 10 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index 9a5f16c..1438174 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf DISTCLEANFILES += locking/qemu-lockd.conf endif WITH_QEMU
+if WITH_LIBXL +nodist_conf_DATA += locking/libxl-lockd.conf +BUILT_SOURCES += locking/libxl-lockd.conf +DISTCLEANFILES += locking/libxl-lockd.conf +endif WITH_LIBXL + locking/%-lockd.conf: $(srcdir)/locking/lockd.conf $(AM_V_GEN)$(MKDIR_P) locking ; \ cp $< $@ @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf BUILT_SOURCES += locking/qemu-sanlock.conf DISTCLEANFILES += locking/qemu-sanlock.conf endif WITH_QEMU + +if WITH_LIBXL +nodist_conf_DATA += locking/libxl-sanlock.conf +BUILT_SOURCES += locking/libxl-sanlock.conf +DISTCLEANFILES += locking/libxl-sanlock.conf +endif WITH_LIBXL else ! WITH_SANLOCK EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) endif ! WITH_SANLOCK diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug index f225954..d5aa150 100644 --- a/src/libxl/libvirtd_libxl.aug +++ b/src/libxl/libvirtd_libxl.aug @@ -25,9 +25,11 @@ module Libvirtd_libxl =
(* Config entry grouped by function - same order as example config *) let autoballoon_entry = bool_entry "autoballoon" + let lock_entry = str_entry "lock_manager"
(* Each entry in the config is one of the following ... *) let entry = autoballoon_entry + | lock_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf index c104d40..ba3de7a 100644 --- a/src/libxl/libxl.conf +++ b/src/libxl/libxl.conf @@ -10,3 +10,13 @@ # autoballoon setting. # #autoballoon = 1 + + +# In order to prevent accidentally starting two domains that +# share one writable disk, libvirt offers two approaches for +# locking files: sanlock and virtlockd. sanlock is an external +# project which libvirt integrates with via the libvirt-lock-sanlock +# package. virtlockd is a libvirt implementation that is enabled with +# "lockd". Accepted values are "sanlock" and "lockd". +# +#lock_manager = "lockd" diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6ea2889..503e8a4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->libDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); + VIR_FREE(cfg->lockManagerName); }
@@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, const char *filename) { virConfPtr conf = NULL; + virConfValuePtr p; int ret = -1;
/* Check the file is readable before opening it, otherwise @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, if (libxlGetAutoballoonConf(cfg, conf) < 0) goto cleanup;
+ if ((p = virConfGetValue(conf, "lock_manager"))) { + if (p->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Unexpected type for 'lock_manager' setting")); + goto cleanup; + } + + if (VIR_STRDUP(cfg->lockManagerName, p->str) < 0) + goto cleanup; + } + ret = 0;
cleanup: diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 5ba1a71..0a1c0db 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -38,6 +38,7 @@ # include "virobject.h" # include "virchrdev.h" # include "virhostdev.h" +# include "locking/lock_manager.h"
# define LIBXL_DRIVER_NAME "xenlight" # define LIBXL_VNC_PORT_MIN 5900 @@ -98,6 +99,8 @@ struct _libxlDriverConfig { * memory for new domains from domain0. */ bool autoballoon;
+ char *lockManagerName; + /* Once created, caps are immutable */ virCapsPtr caps;
@@ -144,6 +147,9 @@ struct _libxlDriverPrivate {
/* Immutable pointer, lockless APIs*/ virSysinfoDefPtr hostsysinfo; + + /* Immutable pointer. lockless access */ + virLockManagerPluginPtr lockManager; };
# define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 77d46d0..70247f5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -34,6 +34,7 @@ #include "virlog.h" #include "virstring.h" #include "virtime.h" +#include "locking/domain_lock.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -217,12 +218,36 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data;
+ VIR_FREE(priv->lockState); virObjectUnref(priv); }
+static int +libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + priv->lockState = virXPathString("string(./lockstate)", ctxt); + + return 0; +} + +static int +libxlDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + if (priv->lockState) + virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState); + + return 0; +} + virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = { .alloc = libxlDomainObjPrivateAlloc, .free = libxlDomainObjPrivateFree, + .parse = libxlDomainObjPrivateXMLParse, + .format = libxlDomainObjPrivateXMLFormat, };
@@ -668,6 +693,11 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, VIR_HOSTDEV_SP_PCI, NULL);
+ VIR_FREE(priv->lockState); + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
I see a few calls to ProcessPause, but I'm not seeing any to ProcessResume, so I think you might have missed a few places needing hooks.
Currently I'm not using ProcessResume and instead calling ProcessStart with paused param set to false. ProcessStart is called in libxlDomainStart, which is called by all paths that start a domain, e.g. start, create, restore, migrate, etc. I was under the impression that ProcessResume is not needed if ProcessStart is called with paused=false. Regards, Jim
On Tue, Apr 21, 2015 at 03:47:34PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Fri, Apr 17, 2015 at 03:36:22PM -0600, Jim Fehlig wrote:
Provide integration with libvirt's lock manager in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/Makefile.am | 12 +++++++++ src/libxl/libvirtd_libxl.aug | 2 ++ src/libxl/libxl.conf | 10 ++++++++ src/libxl/libxl_conf.c | 14 +++++++++++ src/libxl/libxl_conf.h | 6 +++++ src/libxl/libxl_domain.c | 47 ++++++++++++++++++++++++++++++++++-- src/libxl/libxl_domain.h | 1 + src/libxl/libxl_driver.c | 25 +++++++++++++++++++ src/libxl/libxl_migration.c | 6 +++++ src/libxl/test_libvirtd_libxl.aug.in | 1 + 10 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index 9a5f16c..1438174 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf DISTCLEANFILES += locking/qemu-lockd.conf endif WITH_QEMU
+if WITH_LIBXL +nodist_conf_DATA += locking/libxl-lockd.conf +BUILT_SOURCES += locking/libxl-lockd.conf +DISTCLEANFILES += locking/libxl-lockd.conf +endif WITH_LIBXL + locking/%-lockd.conf: $(srcdir)/locking/lockd.conf $(AM_V_GEN)$(MKDIR_P) locking ; \ cp $< $@ @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf BUILT_SOURCES += locking/qemu-sanlock.conf DISTCLEANFILES += locking/qemu-sanlock.conf endif WITH_QEMU + +if WITH_LIBXL +nodist_conf_DATA += locking/libxl-sanlock.conf +BUILT_SOURCES += locking/libxl-sanlock.conf +DISTCLEANFILES += locking/libxl-sanlock.conf +endif WITH_LIBXL else ! WITH_SANLOCK EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) endif ! WITH_SANLOCK diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug index f225954..d5aa150 100644 --- a/src/libxl/libvirtd_libxl.aug +++ b/src/libxl/libvirtd_libxl.aug @@ -25,9 +25,11 @@ module Libvirtd_libxl =
(* Config entry grouped by function - same order as example config *) let autoballoon_entry = bool_entry "autoballoon" + let lock_entry = str_entry "lock_manager"
(* Each entry in the config is one of the following ... *) let entry = autoballoon_entry + | lock_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf index c104d40..ba3de7a 100644 --- a/src/libxl/libxl.conf +++ b/src/libxl/libxl.conf @@ -10,3 +10,13 @@ # autoballoon setting. # #autoballoon = 1 + + +# In order to prevent accidentally starting two domains that +# share one writable disk, libvirt offers two approaches for +# locking files: sanlock and virtlockd. sanlock is an external +# project which libvirt integrates with via the libvirt-lock-sanlock +# package. virtlockd is a libvirt implementation that is enabled with +# "lockd". Accepted values are "sanlock" and "lockd". +# +#lock_manager = "lockd" diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6ea2889..503e8a4 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->libDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); + VIR_FREE(cfg->lockManagerName); }
@@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, const char *filename) { virConfPtr conf = NULL; + virConfValuePtr p; int ret = -1;
/* Check the file is readable before opening it, otherwise @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, if (libxlGetAutoballoonConf(cfg, conf) < 0) goto cleanup;
+ if ((p = virConfGetValue(conf, "lock_manager"))) { + if (p->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Unexpected type for 'lock_manager' setting")); + goto cleanup; + } + + if (VIR_STRDUP(cfg->lockManagerName, p->str) < 0) + goto cleanup; + } + ret = 0;
cleanup: diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 5ba1a71..0a1c0db 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -38,6 +38,7 @@ # include "virobject.h" # include "virchrdev.h" # include "virhostdev.h" +# include "locking/lock_manager.h"
# define LIBXL_DRIVER_NAME "xenlight" # define LIBXL_VNC_PORT_MIN 5900 @@ -98,6 +99,8 @@ struct _libxlDriverConfig { * memory for new domains from domain0. */ bool autoballoon;
+ char *lockManagerName; + /* Once created, caps are immutable */ virCapsPtr caps;
@@ -144,6 +147,9 @@ struct _libxlDriverPrivate {
/* Immutable pointer, lockless APIs*/ virSysinfoDefPtr hostsysinfo; + + /* Immutable pointer. lockless access */ + virLockManagerPluginPtr lockManager; };
# define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r" diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 77d46d0..70247f5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -34,6 +34,7 @@ #include "virlog.h" #include "virstring.h" #include "virtime.h" +#include "locking/domain_lock.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -217,12 +218,36 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data;
+ VIR_FREE(priv->lockState); virObjectUnref(priv); }
+static int +libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + priv->lockState = virXPathString("string(./lockstate)", ctxt); + + return 0; +} + +static int +libxlDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) +{ + libxlDomainObjPrivatePtr priv = data; + + if (priv->lockState) + virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState); + + return 0; +} + virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = { .alloc = libxlDomainObjPrivateAlloc, .free = libxlDomainObjPrivateFree, + .parse = libxlDomainObjPrivateXMLParse, + .format = libxlDomainObjPrivateXMLFormat, };
@@ -668,6 +693,11 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, VIR_HOSTDEV_SP_PCI, NULL);
+ VIR_FREE(priv->lockState); + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
I see a few calls to ProcessPause, but I'm not seeing any to ProcessResume, so I think you might have missed a few places needing hooks.
Currently I'm not using ProcessResume and instead calling ProcessStart with paused param set to false. ProcessStart is called in libxlDomainStart, which is called by all paths that start a domain, e.g. start, create, restore, migrate, etc. I was under the impression that ProcessResume is not needed if ProcessStart is called with paused=false.
For virtlockd it doesn't matter, but with the sanlock driver there is a tiny difference due to VIR_LOCK_MANAGER_NEW_STARTED being passed to the virLockManagerNew() method in the Start case. Regards, 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 :|
Daniel P. Berrange wrote:
On Tue, Apr 21, 2015 at 03:47:34PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Fri, Apr 17, 2015 at 03:36:22PM -0600, Jim Fehlig wrote:
+ VIR_FREE(priv->lockState); + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
I see a few calls to ProcessPause, but I'm not seeing any to ProcessResume, so I think you might have missed a few places needing hooks.
Currently I'm not using ProcessResume and instead calling ProcessStart with paused param set to false. ProcessStart is called in libxlDomainStart, which is called by all paths that start a domain, e.g. start, create, restore, migrate, etc. I was under the impression that ProcessResume is not needed if ProcessStart is called with paused=false.
For virtlockd it doesn't matter, but with the sanlock driver there is a tiny difference due to VIR_LOCK_MANAGER_NEW_STARTED being passed to the virLockManagerNew() method in the Start case.
Ah, I see. Thanks for the explanation. I'll send a V3 to call ProcessResume right after ProcessStart. I also noticed a small goto label bug that will be fixed in V3. Regards, Jim
participants (2)
-
Daniel P. Berrange -
Jim Fehlig