On 03/03/2014 10:21 AM, Martin Kletzander wrote:
When domain is started with setting that cannot be done, i.e. those
that require cgroups, there is no error reported and it succeeds
without any message whatsoever.
When setting with API, virsh, an error is reported, but only due to
the fact that no cgroups are mounted (priv->cgroup == NULL).
Given the above it seems reasonable to reject such unsupported
settings.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Notes:
Few questions came to my mind while writing the commit message:
1) Would helper function (macro) be preferred so the code looks
cleaner?
What macro do you have in mind?
2) Do we want to allow reading of some of these settings through an
API? This would however require our cgroup handling to be
reworked.
3) Would new error type (for session-unsupported settings) be any
good or it doesn't make sense to create one just for these added
messages plus few older ones (just guessing the amount)?
Looks like your use of VIR_ERR_CONFIG_UNSUPPORTED was reasonable.
+ if (!cfg->privileged) {
+ /* If we have no cgroups than we can have no tunings that
+ * require them */
+
+ if (def->mem.hard_limit || def->mem.soft_limit ||
+ def->mem.min_guarantee || def->mem.swap_hard_limit) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Memory tuning is not available in session
mode"));
+ goto error;
+ }
+
Or maybe we should think about some day adding a daemon that accepts RPC
commands for manipulating cgroups on behalf of a session client. But
that would be a later and bigger patch.
Seems reasonable to me, but it might be nice to also get Dan's opinion
on this one.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org