[libvirt] [PATCH 0/3] Support for marking domains as "tainted"
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
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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
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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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 :|
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
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
Eric Blake