6:28 a.m.
There are various configurations for virtual domains that we allow,
but do not wish to actively support in production environments.
The support situation is similar to that of binary only or non-GPL
kernel modules, so borrow the kernel's idea of "tainting".
When an undesirable configuration is used on a running VM, set a
suitable taint flag and log a warning. OS distro bug triagers can
see these warnings and decide whether to support users filing bugs
in this scenarios
6:28 a.m.
New subject: [libvirt] [PATCH 1/3] Add field to virDomainObjPtr to track "tainting"
Some configuration setups for guests are allowed, but strongly
discouraged and unsupportable in production systems. Introduce
a concept of 'tainting' to virDomainObjPtr to allow such setups
to be identified. Drivers can then log warnings at suitable
times
* src/conf/domain_conf.c, src/conf/domain_conf.h: Declare taint
flags and add parsing/formatting of domain status XML
---
src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 16 ++++++++++++++
src/libvirt_private.syms | 3 ++
3 files changed, 70 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2a681d9..7af29af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -50,6 +50,13 @@
#define VIR_FROM_THIS VIR_FROM_DOMAIN
+VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
+ "custom-argv",
+ "custom-monitor",
+ "high-privileges",
+ "shell-scripts",
+ "disk-probing");
+
VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST,
"qemu",
"kqemu",
@@ -510,6 +517,20 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
return obj;
}
+
+bool virDomainObjTaint(virDomainObjPtr obj,
+ int taint)
+{
+ int flag = (1 << taint);
+
+ if (obj->taint & flag)
+ return false;
+
+ obj->taint |= flag;
+ return true;
+}
+
+
static void
virDomainGraphicsAuthDefClear(virDomainGraphicsAuthDefPtr def)
{
@@ -6250,6 +6271,8 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps,
xmlNodePtr config;
xmlNodePtr oldnode;
virDomainObjPtr obj;
+ xmlNodePtr *nodes = NULL;
+ int i, n;
if (!(obj = virDomainObjNew(caps)))
return NULL;
@@ -6288,6 +6311,26 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps,
}
obj->pid = (pid_t)val;
+ if ((n = virXPathNodeSet("./taint", ctxt, &nodes)) < 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("failed to parse taint
flags"));
+ goto error;
+ }
+ for (i = 0 ; i < n ; i++) {
+ char *str = virXMLPropString(nodes[i], "flag");
+ if (str) {
+ int flag = virDomainTaintTypeFromString(str);
+ VIR_FREE(str);
+ if (flag < 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown taint flag %s"), str);
+ goto error;
+ }
+ virDomainObjTaint(obj, flag);
+ }
+ }
+ VIR_FREE(nodes);
+
if (caps->privateDataXMLParse &&
((caps->privateDataXMLParse)(ctxt, obj->privateData)) < 0)
goto error;
@@ -6297,6 +6340,7 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps,
error:
/* obj was never shared, so unref should return 0 */
ignore_value(virDomainObjUnref(obj));
+ VIR_FREE(nodes);
return NULL;
}
@@ -8454,11 +8498,18 @@ static char *virDomainObjFormat(virCapsPtr caps,
{
char *config_xml = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ int i;
virBufferVSprintf(&buf, "<domstatus state='%s'
pid='%d'>\n",
virDomainStateTypeToString(obj->state),
obj->pid);
+ for (i = 0 ; i < VIR_DOMAIN_TAINT_LAST ; i++) {
+ if (obj->taint & (1 << i))
+ virBufferVSprintf(&buf, " <taint
flag='%s'/>\n",
+ virDomainTaintTypeToString(i));
+ }
+
if (caps->privateDataXMLFormat &&
((caps->privateDataXMLFormat)(&buf, obj->privateData)) < 0)
goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1dadf98..812f761 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1182,6 +1182,16 @@ struct _virDomainDef {
virDomainXMLNamespace ns;
};
+enum virDomainTaintFlags {
+ VIR_DOMAIN_TAINT_CUSTOM_ARGV, /* Custom ARGV passthrough from XML */
+ VIR_DOMAIN_TAINT_CUSTOM_MONITOR, /* Custom monitor commands issued */
+ VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, /* Running with undesirably high privileges */
+ VIR_DOMAIN_TAINT_SHELL_SCRIPTS, /* Network configuration using opaque shell
scripts */
+ VIR_DOMAIN_TAINT_DISK_PROBING, /* Relying on potentially unsafe disk format
probing */
+
+ VIR_DOMAIN_TAINT_LAST
+};
+
/* Guest VM runtime state */
typedef struct _virDomainObj virDomainObj;
typedef virDomainObj *virDomainObjPtr;
@@ -1204,6 +1214,8 @@ struct _virDomainObj {
void *privateData;
void (*privateDataFreeFunc)(void *);
+
+ int taint;
};
typedef struct _virDomainObjList virDomainObjList;
@@ -1231,6 +1243,8 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms,
virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
const char *name);
+bool virDomainObjTaint(virDomainObjPtr obj,
+ int taint);
void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def);
void virDomainInputDefFree(virDomainInputDefPtr def);
@@ -1429,6 +1443,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
typedef const char* (*virLifecycleToStringFunc)(int type);
typedef int (*virLifecycleFromStringFunc)(const char *type);
+VIR_ENUM_DECL(virDomainTaint)
+
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
VIR_ENUM_DECL(virDomainFeature)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1b22be6..508b044 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -300,6 +300,7 @@ virDomainObjListNumOfDomains;
virDomainObjLock;
virDomainObjRef;
virDomainObjSetDefTransient;
+virDomainObjTaint;
virDomainObjUnlock;
virDomainObjUnref;
virDomainRemoveInactive;
@@ -324,6 +325,8 @@ virDomainSoundModelTypeFromString;
virDomainSoundModelTypeToString;
virDomainStateTypeFromString;
virDomainStateTypeToString;
+virDomainTaintTypeFromString;
+virDomainTaintTypeToString;
virDomainTimerModeTypeFromString;
virDomainTimerModeTypeToString;
virDomainTimerNameTypeFromString;
--
1.7.4.4
9:45 a.m.
New subject: [libvirt] [PATCH 1/3] Add field to virDomainObjPtr to track "tainting"
On 05/04/2011 05:28 AM, Daniel P. Berrange wrote:
+bool virDomainObjTaint(virDomainObjPtr obj,
+ int taint)
+{
+ int flag = (1 << taint);
Undefined behavior if taint is out of bounds (>= 31). Do we need a
sanity check, or to change the 'int taint' parameter to instead be 'enum
virDomainTaintFlags'?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:28 a.m.
New subject: [libvirt] [PATCH 2/3] Log taint warnings in QEMU driver
Wire up logging of VM taintint to the QEMU driver
- If running QEMU as root user/group or without capabilities
being cleared
- If passing custom QEMU command line args
- If issuing custom QEMU monitor commands
- If using a network interface config with an associated
shell script
- If using a disk config relying on format probing
The warnings, per-VM appear in the main libvirtd logs
11:56:17.571: 10832: warning : qemuDomainObjTaint:712 : Domain id=1 name='l2'
uuid=c7a3edbd-edaf-9455-926a-d65c16db1802 is tainted: high-privileges
11:56:17.571: 10832: warning : qemuDomainObjTaint:712 : Domain id=1 name='l2'
uuid=c7a3edbd-edaf-9455-926a-d65c16db1802 is tainted: disk-probing
And in the /var/log/libvirt/qemu/$VMNAME.log file
Domain id=2 is tainted: high-privileges
Domain id=2 is tainted: disk-probing
The taint flags are reset when the VM is stopped.
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Helper APIs
for logging taint warnings
* src/qemu/qemu_driver.c: Log tainting with custom QEMU monitor
commands and disk/net hotplug with unsupported configs
* src/qemu/qemu_process.c: Log tainting at startup based on
unsupported configs
---
src/qemu/qemu_domain.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 16 +++++++++-
src/qemu/qemu_driver.c | 8 ++---
src/qemu/qemu_process.c | 4 ++-
4 files changed, 96 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a947b4e..78839cc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -32,6 +32,7 @@
#include "event.h"
#include "cpu/cpu.h"
#include "ignore-value.h"
+#include "uuid.h"
#include <sys/time.h>
@@ -697,3 +698,77 @@ cleanup:
virCPUDefFree(cpu);
return ret;
}
+
+void qemuDomainObjTaint(virDomainObjPtr obj,
+ int taint,
+ int logFD)
+{
+ if (virDomainObjTaint(obj, taint)) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(obj->def->uuid, uuidstr);
+
+ VIR_WARN("Domain id=%d name='%s' uuid=%s is tainted: %s",
+ obj->def->id,
+ obj->def->name,
+ uuidstr,
+ virDomainTaintTypeToString(taint));
+
+ if (logFD != -1) {
+ char *message;
+ if (virAsprintf(&message,
+ "Domain id=%d is tainted: %s\n",
+ obj->def->id,
+ virDomainTaintTypeToString(taint)) < 0)
+ return;
+ ignore_value(safewrite(logFD, message, strlen(message)));
+ }
+ }
+}
+
+
+void qemuDomainObjCheckTaint(struct qemud_driver *driver,
+ virDomainObjPtr obj,
+ int logFD)
+{
+ int i;
+
+ if (!driver->clearEmulatorCapabilities ||
+ driver->user == 0 ||
+ driver->group == 0)
+ qemuDomainObjTaint(obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD);
+
+ if (obj->def->namespaceData) {
+ qemuDomainCmdlineDefPtr qemucmd = obj->def->namespaceData;
+ if (qemucmd->num_args || qemucmd->num_env)
+ qemuDomainObjTaint(obj, VIR_DOMAIN_TAINT_CUSTOM_ARGV, logFD);
+ }
+
+ for (i = 0 ; i < obj->def->ndisks ; i++)
+ qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logFD);
+
+ for (i = 0 ; i < obj->def->nnets ; i++)
+ qemuDomainObjCheckNetTaint(obj, obj->def->nets[i], logFD);
+}
+
+
+void qemuDomainObjCheckDiskTaint(struct qemud_driver *driver,
+ virDomainObjPtr obj,
+ virDomainDiskDefPtr disk,
+ int logFD)
+{
+ if (!disk->driverType &&
+ driver->allowDiskFormatProbing)
+ qemuDomainObjTaint(obj, VIR_DOMAIN_TAINT_DISK_PROBING, logFD);
+}
+
+
+void qemuDomainObjCheckNetTaint(virDomainObjPtr obj,
+ virDomainNetDefPtr net,
+ int logFD)
+{
+ if ((net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ net->data.ethernet.script != NULL) ||
+ (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE &&
+ net->data.bridge.script != NULL))
+ qemuDomainObjTaint(obj, VIR_DOMAIN_TAINT_SHELL_SCRIPTS, logFD);
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 8258900..dbef4e1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -69,7 +69,6 @@ struct _qemuDomainObjPrivate {
qemuMonitorPtr mon;
virDomainChrSourceDefPtr monConfig;
int monJSON;
- int monitor_warned;
bool gotShutdown;
int nvcpupids;
@@ -113,4 +112,19 @@ char *qemuDomainFormatXML(struct qemud_driver *driver,
virDomainObjPtr vm,
int flags);
+void qemuDomainObjTaint(virDomainObjPtr obj,
+ int taint,
+ int logFD);
+
+void qemuDomainObjCheckTaint(struct qemud_driver *driver,
+ virDomainObjPtr obj,
+ int logFD);
+void qemuDomainObjCheckDiskTaint(struct qemud_driver *driver,
+ virDomainObjPtr obj,
+ virDomainDiskDefPtr disk,
+ int logFD);
+void qemuDomainObjCheckNetTaint(virDomainObjPtr obj,
+ virDomainNetDefPtr net,
+ int logFD);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0919503..13f9362 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3884,6 +3884,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
+ qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1);
ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev, qemuCaps);
if (!ret)
dev->data.disk = NULL;
@@ -3896,6 +3897,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
break;
case VIR_DOMAIN_DEVICE_NET:
+ qemuDomainObjCheckNetTaint(vm, dev->data.net, -1);
ret = qemuDomainAttachNetDevice(dom->conn, driver, vm,
dev->data.net, qemuCaps);
if (!ret)
@@ -7001,11 +7003,7 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char
*cmd,
priv = vm->privateData;
- if (!priv->monitor_warned) {
- VIR_INFO("Qemu monitor command '%s' executed; libvirt results may be
unpredictable!",
- cmd);
- priv->monitor_warned = 1;
- }
+ qemuDomainObjTaint(vm, VIR_DOMAIN_TAINT_CUSTOM_MONITOR, -1);
hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7691cbe..c521dbf 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2229,7 +2229,6 @@ int qemuProcessStart(virConnectPtr conn,
#endif
priv->monJSON = 0;
- priv->monitor_warned = 0;
priv->gotShutdown = false;
if ((ret = virFileDeletePid(driver->stateDir, vm->def->name)) != 0) {
@@ -2316,6 +2315,8 @@ int qemuProcessStart(virConnectPtr conn,
virCommandWriteArgLog(cmd, logfile);
+ qemuDomainObjCheckTaint(driver, vm, logfile);
+
if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
VIR_WARN("Unable to seek to end of logfile: %s",
virStrerror(errno, ebuf, sizeof ebuf));
@@ -2597,6 +2598,7 @@ retry:
qemuProcessReturnPort(driver,
vm->def->graphics[0]->data.spice.tlsPort);
}
+ vm->taint = 0;
vm->pid = -1;
vm->def->id = -1;
vm->state = VIR_DOMAIN_SHUTOFF;
--
1.7.4.4
6:28 a.m.
New subject: [libvirt] [PATCH 3/3] Ensure hotplug / monitor commands log taint warnings
Open the domain log file for hotplug and custom monitor command
usage to write taint warnings.
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
qemuDomainOpenLogWrite/qemuDomainOpenLogRead methods
* src/qemu/qemu_process.c: Remove qemuProcessOpenLogFD
and qemuProcessReadLogFD methods (moved to qemu_domain)
* src/qemu/qemu_process.c: Open log in hotplug and qemu
monitor command functions
---
src/qemu/qemu_domain.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 5 +++
src/qemu/qemu_driver.c | 16 +++++++--
src/qemu/qemu_process.c | 81 ++---------------------------------------------
4 files changed, 101 insertions(+), 81 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78839cc..202340f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -33,8 +33,10 @@
#include "cpu/cpu.h"
#include "ignore-value.h"
#include "uuid.h"
+#include "files.h"
#include <sys/time.h>
+#include <fcntl.h>
#include <libxml/xpathInternals.h>
@@ -699,6 +701,84 @@ cleanup:
return ret;
}
+
+int
+qemuDomainOpenLogWrite(struct qemud_driver *driver,
+ virDomainObjPtr vm, bool append)
+{
+ char *logfile;
+ mode_t logmode;
+ int fd = -1;
+
+ if (virAsprintf(&logfile, "%s/%s.log", driver->logDir,
vm->def->name) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ logmode = O_CREAT | O_WRONLY;
+ /* Only logrotate files in /var/log, so only append if running privileged */
+ if (driver->privileged || append)
+ logmode |= O_APPEND;
+ else
+ logmode |= O_TRUNC;
+
+ if ((fd = open(logfile, logmode, S_IRUSR | S_IWUSR)) < 0) {
+ virReportSystemError(errno,
+ _("failed to create logfile %s"),
+ logfile);
+ VIR_FREE(logfile);
+ return -1;
+ }
+ VIR_FREE(logfile);
+ if (virSetCloseExec(fd) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to set VM logfile close-on-exec
flag"));
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+ }
+ return fd;
+}
+
+
+int
+qemuDomainOpenLogRead(struct qemud_driver *driver,
+ virDomainObjPtr vm, off_t pos)
+{
+ char *logfile;
+ mode_t logmode = O_RDONLY;
+ int fd = -1;
+
+ if (virAsprintf(&logfile, "%s/%s.log", driver->logDir,
vm->def->name) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ if ((fd = open(logfile, logmode)) < 0) {
+ virReportSystemError(errno,
+ _("failed to create logfile %s"),
+ logfile);
+ VIR_FREE(logfile);
+ return -1;
+ }
+ if (virSetCloseExec(fd) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to set VM logfile close-on-exec
flag"));
+ VIR_FORCE_CLOSE(fd);
+ VIR_FREE(logfile);
+ return -1;
+ }
+ if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) {
+ virReportSystemError(pos < 0 ? 0 : errno,
+ _("Unable to seek to %lld in %s"),
+ (long long) pos, logfile);
+ VIR_FORCE_CLOSE(fd);
+ }
+ VIR_FREE(logfile);
+ return fd;
+}
+
+
+
void qemuDomainObjTaint(virDomainObjPtr obj,
int taint,
int logFD)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index dbef4e1..cab7991 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -112,6 +112,11 @@ char *qemuDomainFormatXML(struct qemud_driver *driver,
virDomainObjPtr vm,
int flags);
+int qemuDomainOpenLogWrite(struct qemud_driver *driver,
+ virDomainObjPtr vm, bool append);
+int qemuDomainOpenLogRead(struct qemud_driver *driver,
+ virDomainObjPtr vm, off_t pos);
+
void qemuDomainObjTaint(virDomainObjPtr obj,
int taint,
int logFD);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 13f9362..75e4631 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3881,10 +3881,14 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
{
struct qemud_driver *driver = dom->conn->privateData;
int ret = -1;
+ int logFD = -1;
+
+ if ((logFD = qemuDomainOpenLogWrite(driver, vm, true)) < 0)
+ return -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1);
+ qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, logFD);
ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev, qemuCaps);
if (!ret)
dev->data.disk = NULL;
@@ -3897,7 +3901,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
break;
case VIR_DOMAIN_DEVICE_NET:
- qemuDomainObjCheckNetTaint(vm, dev->data.net, -1);
+ qemuDomainObjCheckNetTaint(vm, dev->data.net, logFD);
ret = qemuDomainAttachNetDevice(dom->conn, driver, vm,
dev->data.net, qemuCaps);
if (!ret)
@@ -3918,6 +3922,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
break;
}
+ VIR_FORCE_CLOSE(logFD);
return ret;
}
@@ -6982,6 +6987,7 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char
*cmd,
int ret = -1;
qemuDomainObjPrivatePtr priv;
bool hmp;
+ int logFD = -1;
virCheckFlags(VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP, -1);
@@ -7001,9 +7007,12 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char
*cmd,
goto cleanup;
}
+ if ((logFD = qemuDomainOpenLogWrite(driver, vm, true)) < 0)
+ goto cleanup;
+
priv = vm->privateData;
- qemuDomainObjTaint(vm, VIR_DOMAIN_TAINT_CUSTOM_MONITOR, -1);
+ qemuDomainObjTaint(vm, VIR_DOMAIN_TAINT_CUSTOM_MONITOR, logFD);
hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP);
@@ -7021,6 +7030,7 @@ cleanup:
if (vm)
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
+ VIR_FORCE_CLOSE(logFD);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c521dbf..187c4c9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -679,81 +679,6 @@ error:
return ret;
}
-static int
-qemuProcessLogFD(struct qemud_driver *driver, const char* name, bool append)
-{
- char *logfile;
- mode_t logmode;
- int fd = -1;
-
- if (virAsprintf(&logfile, "%s/%s.log", driver->logDir, name) < 0)
{
- virReportOOMError();
- return -1;
- }
-
- logmode = O_CREAT | O_WRONLY;
- /* Only logrotate files in /var/log, so only append if running privileged */
- if (driver->privileged || append)
- logmode |= O_APPEND;
- else
- logmode |= O_TRUNC;
-
- if ((fd = open(logfile, logmode, S_IRUSR | S_IWUSR)) < 0) {
- virReportSystemError(errno,
- _("failed to create logfile %s"),
- logfile);
- VIR_FREE(logfile);
- return -1;
- }
- VIR_FREE(logfile);
- if (virSetCloseExec(fd) < 0) {
- virReportSystemError(errno, "%s",
- _("Unable to set VM logfile close-on-exec
flag"));
- VIR_FORCE_CLOSE(fd);
- return -1;
- }
- return fd;
-}
-
-
-static int
-qemuProcessLogReadFD(const char* logDir, const char* name, off_t pos)
-{
- char *logfile;
- mode_t logmode = O_RDONLY;
- int fd = -1;
-
- if (virAsprintf(&logfile, "%s/%s.log", logDir, name) < 0) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to build logfile name %s/%s.log"),
- logDir, name);
- return -1;
- }
-
- if ((fd = open(logfile, logmode)) < 0) {
- virReportSystemError(errno,
- _("failed to create logfile %s"),
- logfile);
- VIR_FREE(logfile);
- return -1;
- }
- if (virSetCloseExec(fd) < 0) {
- virReportSystemError(errno, "%s",
- _("Unable to set VM logfile close-on-exec
flag"));
- VIR_FORCE_CLOSE(fd);
- VIR_FREE(logfile);
- return -1;
- }
- if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) {
- virReportSystemError(pos < 0 ? 0 : errno,
- _("Unable to seek to %lld in %s"),
- (long long) pos, logfile);
- VIR_FORCE_CLOSE(fd);
- }
- VIR_FREE(logfile);
- return fd;
-}
-
typedef int qemuProcessLogHandleOutput(virDomainObjPtr vm,
const char *output,
@@ -1051,7 +976,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver,
virHashTablePtr paths = NULL;
qemuDomainObjPrivatePtr priv;
- if ((logfd = qemuProcessLogReadFD(driver->logDir, vm->def->name, pos)) <
0)
+ if ((logfd = qemuDomainOpenLogRead(driver, vm, pos)) < 0)
return -1;
if (VIR_ALLOC_N(buf, buf_size) < 0) {
@@ -2200,7 +2125,7 @@ int qemuProcessStart(virConnectPtr conn,
}
VIR_DEBUG0("Creating domain log file");
- if ((logfile = qemuProcessLogFD(driver, vm->def->name, false)) < 0)
+ if ((logfile = qemuDomainOpenLogWrite(driver, vm, false)) < 0)
goto cleanup;
VIR_DEBUG0("Determining emulator version");
@@ -2463,7 +2388,7 @@ void qemuProcessStop(struct qemud_driver *driver,
return;
}
- if ((logfile = qemuProcessLogFD(driver, vm->def->name, true)) < 0) {
+ if ((logfile = qemuDomainOpenLogWrite(driver, vm, true)) < 0) {
/* To not break the normal domain shutdown process, skip the
* timestamp log writing if failed on opening log file. */
VIR_WARN("Unable to open logfile: %s",
--
1.7.4.4
7:33 a.m.
On Wed, May 04, 2011 at 12:28:45PM +0100, Daniel P. Berrange wrote:
There are various configurations for virtual domains that we allow,
but do not wish to actively support in production environments.
The support situation is similar to that of binary only or non-GPL
kernel modules, so borrow the kernel's idea of "tainting".
When an undesirable configuration is used on a running VM, set a
suitable taint flag and log a warning. OS distro bug triagers can
see these warnings and decide whether to support users filing bugs
in this scenarios
Looks fine, but post 0.9.1 obviously.
Patch 1 and 2 no problem. For patch 3 I wonder if it's a good idea to
open the logs if we don't have to report a taint violation, which I
would assume is not frequent.
In general the idea of making easier to append a log string to the domain
log file could be used in various places (though I remember arguments
that since it's given to the QEmu process libvirtd should avoid touch
it, on the other hand that's the best place to report per domain
incidents), maybe we could just have a Snprintf like function to append
to it and then only use it if we detect a tainted problem to report.
Could be done as refinement on top of patch 3,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
6:54 a.m.
On Wed, May 04, 2011 at 08:33:09PM +0800, Daniel Veillard wrote:
On Wed, May 04, 2011 at 12:28:45PM +0100, Daniel P. Berrange wrote:
> There are various configurations for virtual domains that we allow,
> but do not wish to actively support in production environments.
> The support situation is similar to that of binary only or non-GPL
> kernel modules, so borrow the kernel's idea of "tainting".
>
> When an undesirable configuration is used on a running VM, set a
> suitable taint flag and log a warning. OS distro bug triagers can
> see these warnings and decide whether to support users filing bugs
> in this scenarios
Looks fine, but post 0.9.1 obviously.
Patch 1 and 2 no problem. For patch 3 I wonder if it's a good idea to
open the logs if we don't have to report a taint violation, which I
would assume is not frequent.
In general the idea of making easier to append a log string to the domain
log file could be used in various places (though I remember arguments
that since it's given to the QEmu process libvirtd should avoid touch
it, on the other hand that's the best place to report per domain
incidents), maybe we could just have a Snprintf like function to append
to it and then only use it if we detect a tainted problem to report.
Could be done as refinement on top of patch 3,
I re-wrote patch 3 completely to work along these lines and posted a
new series.
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 :|
9:03 a.m.
On Wed, May 04, 2011 at 12:28:45PM +0100, Daniel P. Berrange wrote:
There are various configurations for virtual domains that we allow,
but do not wish to actively support in production environments.
The support situation is similar to that of binary only or non-GPL
kernel modules, so borrow the kernel's idea of "tainting".
When an undesirable configuration is used on a running VM, set a
suitable taint flag and log a warning. OS distro bug triagers can
see these warnings and decide whether to support users filing bugs
in this scenarios
ACK to the concept.
Dave
4950
days inactive
4951
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
Eric Blake