[libvirt] [PATCH v2 0/8] Use virtlogd for chardevs & support logging
This series of patches does two things * Converts the type=file chardev over to use virtlogd (assuming use of virtlogd is enabled in qemu.conf) * Adds a <log file="..."/> element to all chardev sources, allowing data to be captured to a logfile Both of these are important features wanted by openstack. The first thing fixes a long standing security issue that a guest OS can trivially exhuast host disk space by outputing lots of data to its serial port. The second thing allows OpenStack to record boot up messages for a guest's serial console, while still allowing interactive serial console ussage. ie the serial port will be configured with type=tcp, and the new <log> element used to record the data. Both these thing required changes in QEMU, which have been merged for the forthcoming QEMU 2.6 release. Changed in v2: * Split patch 4 up into 4 separate patches * Fix memory leaks * Use common function for building log manager cli args * Add missing XML docs Daniel P. Berrange (8): logging: allow inode/offset params to be NULL conf: allow use of a logfile with chardev backends qemu: add support for logging chardev output to a file qemu: don't append -chardev arg until after value is formatted qemu: move functions for handling FD passing logging: support truncation of logfiles when opening qemu: use virtlogd for character device log files qemu: support use of virtlogd with file based chardevs docs/formatdomain.html.in | 14 + docs/schemas/domaincommon.rng | 12 + src/conf/domain_conf.c | 30 +++ src/conf/domain_conf.h | 2 + src/logging/log_daemon_dispatch.c | 3 +- src/logging/log_handler.c | 6 +- src/logging/log_handler.h | 2 +- src/logging/log_manager.c | 6 +- src/logging/log_manager.h | 2 + src/logging/log_protocol.x | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 282 ++++++++++++++------- src/qemu/qemu_command.h | 9 +- src/qemu/qemu_domain.c | 6 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 4 +- .../qemuxml2argv-serial-file-log.args | 25 ++ .../qemuxml2argv-serial-file-log.xml | 39 +++ tests/qemuxml2argvtest.c | 4 +- 21 files changed, 353 insertions(+), 105 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.xml -- 2.5.0
Not all callers of virLogManagerDomainOpenLogFile will care about getting the current inode/offset, so we should allow those parameters to be NULL Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/logging/log_manager.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c index f5b4b7d..7c37ba1 100644 --- a/src/logging/log_manager.c +++ b/src/logging/log_manager.c @@ -196,8 +196,10 @@ virLogManagerDomainOpenLogFile(virLogManagerPtr mgr, goto cleanup; } - *inode = ret.pos.inode; - *offset = ret.pos.offset; + if (inode) + *inode = ret.pos.inode; + if (offset) + *offset = ret.pos.offset; rv = fdout[0]; cleanup: -- 2.5.0
Extend the chardev source XML so that there is a new optional <log/> element, which is applicable to all character device backend types. For example, to log output of a TCP backed serial port <serial type='tcp'> <source mode='connect' host='127.0.0.1' service='9999'/> <protocol type='raw'/> <log file='/var/log/libvirt/qemu/demo-serial0.log' append='on'/> <target port='0'/> </serial> Not all hypervisors will support use of logfiles. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 14 ++++++++++++++ docs/schemas/domaincommon.rng | 12 ++++++++++++ src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 4 files changed, 58 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5016772..cdf5291 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5274,6 +5274,20 @@ qemu-kvm -net nic,model=? /dev/null </p> <p> + Regardless of the<code>type</code>, character devices can + have an optional log file associated with them. This is + expressed via a <code>log</code> sub-element, with a + <code>file</code> attribute. There can also be a <code>append</code> + attribute which takes teh same values described above. + <span class="since">1.3.3</span>. + </p> + + <pre> + ... + <log file="/var/log/libvirt/qemu/guestname-serial0.log" append="off"/> + ...</pre> + + <p> Each character device element has an optional sub-element <code><address></code> which can tie the device to a diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67af93a..b49d9eb 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3276,6 +3276,18 @@ </optional> </element> </optional> + <optional> + <element name="log"> + <attribute name="file"> + <ref name="absFilePath"/> + </attribute> + <optional> + <attribute name="append"> + <ref name="virOnOff"/> + </attribute> + </optional> + </element> + </optional> </define> <!-- The description for a console diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 79758d4..ae241e8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1768,6 +1768,8 @@ virDomainChrSourceDefClear(virDomainChrSourceDefPtr def) VIR_FREE(def->data.spiceport.channel); break; } + + VIR_FREE(def->logfile); } /* Deep copies the contents of src into dest. Return -1 and report @@ -9396,6 +9398,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *connectHost = NULL; char *connectService = NULL; char *path = NULL; + char *logfile = NULL; + char *logappend = NULL; char *mode = NULL; char *protocol = NULL; char *channel = NULL; @@ -9483,6 +9487,11 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, } ctxt->node = saved_node; } + } else if (xmlStrEqual(cur->name, BAD_CAST "log")) { + if (!logfile) + logfile = virXMLPropString(cur, "file"); + if (!logappend) + logappend = virXMLPropString(cur, "append"); } else if (xmlStrEqual(cur->name, BAD_CAST "protocol")) { if (!protocol) protocol = virXMLPropString(cur, "type"); @@ -9641,6 +9650,16 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; } + def->logfile = logfile; + logfile = NULL; + + if (logappend != NULL && + (def->logappend = virTristateSwitchTypeFromString(logappend)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid append attribute value '%s'"), logappend); + goto error; + } + cleanup: VIR_FREE(mode); VIR_FREE(protocol); @@ -9651,6 +9670,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(path); VIR_FREE(channel); VIR_FREE(append); + VIR_FREE(logappend); + VIR_FREE(logfile); return remaining; @@ -20087,6 +20108,15 @@ virDomainChrSourceDefFormat(virBufferPtr buf, } + if (def->logfile) { + virBufferEscapeString(buf, "<log file='%s'", def->logfile); + if (def->logappend != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, " append='%s'", + virTristateSwitchTypeToString(def->logappend)); + } + virBufferAddLit(buf, "/>\n"); + } + return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1de3be3..2b2f75b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1204,6 +1204,8 @@ struct _virDomainChrSourceDef { char *channel; } spiceport; } data; + char *logfile; + int logappend; }; /* A complete character device, both host and domain views. */ -- 2.5.0
Honour the <log file='...'/> element in chardevs to output data to a file. This requires QEMU >= 2.6 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 19 +++++++++++ .../qemuxml2argv-serial-file-log.args | 25 ++++++++++++++ .../qemuxml2argv-serial-file-log.xml | 39 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 6 files changed, 88 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1018d6c..9dcd546 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -315,6 +315,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vserport-change-event", /* 210 */ "virtio-balloon-pci.deflate-on-oom", "mptsas1068", + "chardev-logfile", ); @@ -2627,6 +2628,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP }, { "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP }, { "chardev", "append", QEMU_CAPS_CHARDEV_FILE_APPEND }, + { "chardev", "logfile", QEMU_CAPS_CHARDEV_LOGFILE }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b73c529..ea2e723 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -344,6 +344,7 @@ typedef enum { QEMU_CAPS_VIRTIO_BALLOON_AUTODEFLATE, /* virtio-balloon-{device,pci,ccw}. * deflate-on-oom */ QEMU_CAPS_SCSI_MPTSAS1068, /* -device mptsas1068 */ + QEMU_CAPS_CHARDEV_LOGFILE, /* -chardev logfile=xxxx */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 854e51c..22cfc47 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3967,6 +3967,19 @@ qemuBuildChrChardevStr(const virDomainChrSourceDef *dev, goto error; } + if (dev->logfile) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_LOGFILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logfile not supported in this QEMU binary")); + goto error; + } + virBufferAsprintf(&buf, ",logfile=%s", dev->logfile); + if (dev->logappend != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&buf, ",logappend=%s", + virTristateSwitchTypeToString(dev->logappend)); + } + } + if (virBufferCheckError(&buf) < 0) goto error; @@ -3984,6 +3997,12 @@ qemuBuildChrArgStr(const virDomainChrSourceDef *dev, { virBuffer buf = VIR_BUFFER_INITIALIZER; + if (dev->logfile) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logfile not supported in this QEMU binary")); + goto error; + } + if (prefix) virBufferAdd(&buf, prefix, strlen(prefix)); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.args new file mode 100644 index 0000000..de313e5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-chardev file,id=charserial0,path=/tmp/serial.log,\ +logfile=/var/lib/libvirt/qemu/demo-serial.log,logappend=off \ +-device isa-serial,chardev=charserial0,id=serial0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.xml new file mode 100644 index 0000000..adb3b78 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='file'> + <source path='/tmp/serial.log'/> + <log file='/var/lib/libvirt/qemu/demo-serial.log' append='off'/> + <target port='0'/> + </serial> + <console type='file'> + <source path='/tmp/serial.log'/> + <target type='serial' port='0'/> + </console> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ff12077..ff93329 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1035,6 +1035,8 @@ mymain(void) DO_TEST("serial-pty", NONE); DO_TEST("serial-dev", NONE); DO_TEST("serial-file", NONE); + DO_TEST("serial-file-log", QEMU_CAPS_CHARDEV, QEMU_CAPS_CHARDEV_FILE_APPEND, + QEMU_CAPS_CHARDEV_LOGFILE); DO_TEST("serial-unix", NONE); DO_TEST("serial-tcp", NONE); DO_TEST("serial-udp", NONE); -- 2.5.0
The act of formatting a chardev backend value may need to append command line arguments for passing FDs. If we append the -chardev arg before formatting the value, then the resulting arguments will end up intersposed Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 22cfc47..c7abd0e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4106,10 +4106,10 @@ qemuBuildMonitorCommandLine(virCommandPtr cmd, /* Use -chardev if it's available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) { - virCommandAddArg(cmd, "-chardev"); if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", qemuCaps))) return -1; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, chrdev); VIR_FREE(chrdev); @@ -7932,13 +7932,13 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, smartcard->info.alias, qemuCaps))) { virBufferFreeAndReset(&opt); goto error; } + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -7976,11 +7976,11 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&serial->source, serial->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8012,11 +8012,11 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(¶llel->source, parallel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8045,11 +8045,11 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&channel->source, channel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8090,11 +8090,11 @@ qemuBuildCommandLine(virConnectPtr conn, * the newer -chardev interface. */ ; } else { - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&channel->source, channel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } @@ -8124,11 +8124,11 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&console->source, console->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8143,11 +8143,11 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&console->source, console->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8487,13 +8487,13 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; char *devstr; - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, redirdev->info.alias, qemuCaps))) { goto error; } + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); -- 2.5.0
On 02/29/2016 08:33 AM, Daniel P. Berrange wrote:
The act of formatting a chardev backend value may need to append command line arguments for passing FDs. If we append the -chardev arg before formatting the value, then the resulting arguments will end up intersposed
I assume you meant interspersed (at least that's what my brain read)!
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
The functions for handling FD passing when building command line arguments need to be used by many different bits of code, so need to be at the start of the source file Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 106 ++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c7abd0e..490e991 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -149,6 +149,59 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST, "preferred", "interleave"); +/** + * qemuVirCommandGetFDSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU -add-fd command line option + * for the given file descriptor. The file descriptor must previously + * have been 'transferred' in a virCommandPassFD() call. + * This function for example returns "set=10,fd=20". + */ +static char * +qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + + return result; +} + + +/** + * qemuVirCommandGetDevSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU path= parameter where a file + * descriptor is accessed via a file descriptor set, for example + * /dev/fdset/10. The file descriptor must previously have been + * 'transferred' in a virCommandPassFD() call. + */ +static char * +qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + return result; +} + + static int qemuBuildObjectCommandLinePropsInternal(const char *key, const virJSONValue *value, @@ -6724,59 +6777,6 @@ qemuBuildTPMDevStr(const virDomainDef *def, } -/** - * qemuVirCommandGetFDSet: - * @cmd: the command to modify - * @fd: fd to reassign to the child - * - * Get the parameters for the QEMU -add-fd command line option - * for the given file descriptor. The file descriptor must previously - * have been 'transferred' in a virCommandPassFD() call. - * This function for example returns "set=10,fd=20". - */ -static char * -qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) -{ - char *result = NULL; - int idx = virCommandPassFDGetFDIndex(cmd, fd); - - if (idx >= 0) { - ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd)); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("file descriptor %d has not been transferred"), fd); - } - - return result; -} - - -/** - * qemuVirCommandGetDevSet: - * @cmd: the command to modify - * @fd: fd to reassign to the child - * - * Get the parameters for the QEMU path= parameter where a file - * descriptor is accessed via a file descriptor set, for example - * /dev/fdset/10. The file descriptor must previously have been - * 'transferred' in a virCommandPassFD() call. - */ -static char * -qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) -{ - char *result = NULL; - int idx = virCommandPassFDGetFDIndex(cmd, fd); - - if (idx >= 0) { - ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx)); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("file descriptor %d has not been transferred"), fd); - } - return result; -} - - static char * qemuBuildTPMBackendStr(const virDomainDef *def, virCommandPtr cmd, -- 2.5.0
The virtlogd daemon currently opens all files for append, but in some cases the user may wish to discard existing data. Define a new flag to indicate that logfiles should be truncated when opening. --- src/logging/log_daemon_dispatch.c | 3 ++- src/logging/log_handler.c | 6 ++---- src/logging/log_handler.h | 2 +- src/logging/log_manager.h | 2 ++ src/logging/log_protocol.x | 4 ++++ 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/logging/log_daemon_dispatch.c b/src/logging/log_daemon_dispatch.c index a5fa7f0..b00cee2 100644 --- a/src/logging/log_daemon_dispatch.c +++ b/src/logging/log_daemon_dispatch.c @@ -50,13 +50,14 @@ virLogManagerProtocolDispatchDomainOpenLogFile(virNetServerPtr server ATTRIBUTE_ int rv = -1; off_t offset; ino_t inode; + bool trunc = args->flags & VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; if ((fd = virLogHandlerDomainOpenLogFile(virLogDaemonGetHandler(logDaemon), args->driver, (unsigned char *)args->dom.uuid, args->dom.name, args->path, - args->flags, + trunc, &inode, &offset)) < 0) goto cleanup; diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 92cff50..4c08223 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -357,7 +357,7 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, const unsigned char *domuuid, const char *domname, const char *path, - unsigned int flags, + bool trunc, ino_t *inode, off_t *offset) { @@ -365,8 +365,6 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, virLogHandlerLogFilePtr file = NULL; int pipefd[2] = { -1, -1 }; - virCheckFlags(0, -1); - virObjectLock(handler); handler->inhibitor(true, handler->opaque); @@ -400,7 +398,7 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, if ((file->file = virRotatingFileWriterNew(path, DEFAULT_FILE_SIZE, DEFAULT_MAX_BACKUP, - false, + trunc, DEFAULT_MODE)) == NULL) goto error; diff --git a/src/logging/log_handler.h b/src/logging/log_handler.h index e61f32d..54a9cd9 100644 --- a/src/logging/log_handler.h +++ b/src/logging/log_handler.h @@ -48,7 +48,7 @@ int virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, const unsigned char *domuuid, const char *domname, const char *path, - unsigned int flags, + bool trunc, ino_t *inode, off_t *offset); diff --git a/src/logging/log_manager.h b/src/logging/log_manager.h index d3b9d29..7deaba7 100644 --- a/src/logging/log_manager.h +++ b/src/logging/log_manager.h @@ -26,6 +26,8 @@ # include "internal.h" +# include "logging/log_protocol.h" + typedef struct _virLogManager virLogManager; typedef virLogManager *virLogManagerPtr; diff --git a/src/logging/log_protocol.x b/src/logging/log_protocol.x index b0ac31b..0363c75 100644 --- a/src/logging/log_protocol.x +++ b/src/logging/log_protocol.x @@ -30,6 +30,10 @@ struct virLogManagerProtocolLogFilePosition { }; typedef struct virLogManagerProtocolLogFilePosition virLogManagerProtocolLogFilePosition; +enum virLogManagerProtocolDomainOpenLogFileFlags { + VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE = 1 +}; + /* Obtain a file handle suitable for writing to a * log file for a domain */ -- 2.5.0
On 02/29/2016 08:33 AM, Daniel P. Berrange wrote:
The virtlogd daemon currently opens all files for append, but in some cases the user may wish to discard existing data. Define a new flag to indicate that logfiles should be truncated when opening. --- src/logging/log_daemon_dispatch.c | 3 ++- src/logging/log_handler.c | 6 ++---- src/logging/log_handler.h | 2 +- src/logging/log_manager.h | 2 ++ src/logging/log_protocol.x | 4 ++++ 5 files changed, 11 insertions(+), 6 deletions(-)
[...]
diff --git a/src/logging/log_protocol.x b/src/logging/log_protocol.x index b0ac31b..0363c75 100644 --- a/src/logging/log_protocol.x +++ b/src/logging/log_protocol.x @@ -30,6 +30,10 @@ struct virLogManagerProtocolLogFilePosition { }; typedef struct virLogManagerProtocolLogFilePosition virLogManagerProtocolLogFilePosition;
+enum virLogManagerProtocolDomainOpenLogFileFlags { + VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE = 1 +}; +
Use "= (1 << 0)" (just to be painfully obvious for the next adjustment) Since the trunc setting is "flags & VIR_LOG_*_TRUNCATE" On an unrelated note (while looking at other possible uses of similar constructs - virLockSpaceProtocolAcquireResourceFlags so far is OK, but if the "next" value isn't 4....
On Fri, Mar 04, 2016 at 09:33:42AM -0500, John Ferlan wrote:
On 02/29/2016 08:33 AM, Daniel P. Berrange wrote:
The virtlogd daemon currently opens all files for append, but in some cases the user may wish to discard existing data. Define a new flag to indicate that logfiles should be truncated when opening. --- src/logging/log_daemon_dispatch.c | 3 ++- src/logging/log_handler.c | 6 ++---- src/logging/log_handler.h | 2 +- src/logging/log_manager.h | 2 ++ src/logging/log_protocol.x | 4 ++++ 5 files changed, 11 insertions(+), 6 deletions(-)
[...]
diff --git a/src/logging/log_protocol.x b/src/logging/log_protocol.x index b0ac31b..0363c75 100644 --- a/src/logging/log_protocol.x +++ b/src/logging/log_protocol.x @@ -30,6 +30,10 @@ struct virLogManagerProtocolLogFilePosition { }; typedef struct virLogManagerProtocolLogFilePosition virLogManagerProtocolLogFilePosition;
+enum virLogManagerProtocolDomainOpenLogFileFlags { + VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE = 1 +}; +
Use "= (1 << 0)" (just to be painfully obvious for the next adjustment)
Since the trunc setting is "flags & VIR_LOG_*_TRUNCATE"
Sadly you can't use (1 << 0) in .x files 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 :|
If use of virtlogd is enabled, then use it for backing the character device log files too. This avoids the possibility of a guest denial of service by writing too much data to the log file. --- src/qemu/qemu_command.c | 128 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_command.h | 9 +++- src/qemu/qemu_domain.c | 6 +++ src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 4 +- tests/qemuxml2argvtest.c | 2 +- 7 files changed, 124 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 490e991..e04e55e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -60,6 +60,7 @@ #if defined(__linux__) # include <linux/capability.h> #endif +#include "logging/log_manager.h" #include <sys/stat.h> #include <fcntl.h> @@ -3900,10 +3901,62 @@ qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, return NULL; } +static int +qemuBuildChrChardevFileStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virBufferPtr buf, + const char *filearg, const char *fileval, + const char *appendarg, int appendval) +{ + if (logManager) { + char *fdset, *fdpath; + int flags = 0; + int logfd; + + if (appendval == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + fileval, + flags, + NULL, NULL)) < 0) + return -1; + + virCommandPassFD(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (!(fdset = qemuVirCommandGetFDSet(cmd, logfd))) + return -1; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + VIR_FREE(fdset); + + if (!(fdpath = qemuVirCommandGetDevSet(cmd, logfd))) + return -1; + + virBufferAsprintf(buf, ",%s=%s,%s=on", filearg, fdpath, appendarg); + VIR_FREE(fdpath); + } else { + virBufferAsprintf(buf, ",%s=%s", filearg, fileval); + if (appendval != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, ",%s=%s", appendarg, + virTristateSwitchTypeToString(appendval)); + } + } + + return 0; +} + /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static char * -qemuBuildChrChardevStr(const virDomainChrSourceDef *dev, +qemuBuildChrChardevStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + const virDomainChrSourceDef *dev, const char *alias, virQEMUCapsPtr qemuCaps) { @@ -4026,11 +4079,10 @@ qemuBuildChrChardevStr(const virDomainChrSourceDef *dev, _("logfile not supported in this QEMU binary")); goto error; } - virBufferAsprintf(&buf, ",logfile=%s", dev->logfile); - if (dev->logappend != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&buf, ",logappend=%s", - virTristateSwitchTypeToString(dev->logappend)); - } + if (qemuBuildChrChardevFileStr(logManager, cmd, def, &buf, + "logfile", dev->logfile, + "logappend", dev->logappend) < 0) + goto error; } if (virBufferCheckError(&buf) < 0) @@ -4146,7 +4198,9 @@ qemuBuildChrArgStr(const virDomainChrSourceDef *dev, static int -qemuBuildMonitorCommandLine(virCommandPtr cmd, +qemuBuildMonitorCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, virQEMUCapsPtr qemuCaps, const virDomainChrSourceDef *monitor_chr, bool monitor_json) @@ -4159,7 +4213,8 @@ qemuBuildMonitorCommandLine(virCommandPtr cmd, /* Use -chardev if it's available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) { - if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", + if (!(chrdev = qemuBuildChrChardevStr(logManager, cmd, def, + monitor_chr, "monitor", qemuCaps))) return -1; virCommandAddArg(cmd, "-chardev"); @@ -4301,7 +4356,10 @@ qemuBuildSclpDevStr(virDomainChrDefPtr dev) static int -qemuBuildRNGBackendChrdevStr(virDomainRNGDefPtr rng, +qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainRNGDefPtr rng, virQEMUCapsPtr qemuCaps, char **chr) { @@ -4314,7 +4372,8 @@ qemuBuildRNGBackendChrdevStr(virDomainRNGDefPtr rng, return 0; case VIR_DOMAIN_RNG_BACKEND_EGD: - if (!(*chr = qemuBuildChrChardevStr(rng->source.chardev, + if (!(*chr = qemuBuildChrChardevStr(logManager, cmd, def, + rng->source.chardev, rng->info.alias, qemuCaps))) return -1; } @@ -6620,7 +6679,10 @@ qemuBuildShmemDevStr(virDomainDefPtr def, } char * -qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, +qemuBuildShmemBackendStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { char *devstr = NULL; @@ -6631,13 +6693,16 @@ qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, shmem->name) < 0) return NULL; - devstr = qemuBuildChrChardevStr(&shmem->server.chr, shmem->info.alias, qemuCaps); + devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &shmem->server.chr, + shmem->info.alias, qemuCaps); return devstr; } static int -qemuBuildShmemCommandLine(virCommandPtr cmd, +qemuBuildShmemCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) @@ -6650,7 +6715,8 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, VIR_FREE(devstr); if (shmem->server.enabled) { - if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps))) + if (!(devstr = qemuBuildShmemBackendStr(logManager, cmd, def, + shmem, qemuCaps))) return -1; virCommandAddArgList(cmd, "-chardev", devstr, NULL); @@ -7020,6 +7086,7 @@ qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virQEMUDriverPtr driver, + virLogManagerPtr logManager, virDomainDefPtr def, virDomainChrSourceDefPtr monitor_chr, bool monitor_json, @@ -7170,7 +7237,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSgaCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildMonitorCommandLine(cmd, qemuCaps, monitor_chr, + if (qemuBuildMonitorCommandLine(logManager, cmd, def, + qemuCaps, monitor_chr, monitor_json) < 0) goto error; @@ -7932,7 +8000,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &smartcard->data.passthru, smartcard->info.alias, qemuCaps))) { virBufferFreeAndReset(&opt); @@ -7976,7 +8045,8 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - if (!(devstr = qemuBuildChrChardevStr(&serial->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &serial->source, serial->info.alias, qemuCaps))) goto error; @@ -8012,7 +8082,8 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (!(devstr = qemuBuildChrChardevStr(¶llel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + ¶llel->source, parallel->info.alias, qemuCaps))) goto error; @@ -8045,7 +8116,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (!(devstr = qemuBuildChrChardevStr(&channel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, channel->info.alias, qemuCaps))) goto error; @@ -8090,7 +8162,8 @@ qemuBuildCommandLine(virConnectPtr conn, * the newer -chardev interface. */ ; } else { - if (!(devstr = qemuBuildChrChardevStr(&channel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, channel->info.alias, qemuCaps))) goto error; @@ -8124,7 +8197,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (!(devstr = qemuBuildChrChardevStr(&console->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, console->info.alias, qemuCaps))) goto error; @@ -8143,7 +8217,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (!(devstr = qemuBuildChrChardevStr(&console->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, console->info.alias, qemuCaps))) goto error; @@ -8487,7 +8562,8 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; char *devstr; - if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &redirdev->source.chr, redirdev->info.alias, qemuCaps))) { goto error; @@ -8713,7 +8789,8 @@ qemuBuildCommandLine(virConnectPtr conn, } /* possibly add character device for backend */ - if (qemuBuildRNGBackendChrdevStr(rng, qemuCaps, &tmp) < 0) + if (qemuBuildRNGBackendChrdevStr(logManager, cmd, def, + rng, qemuCaps, &tmp) < 0) goto error; if (tmp) { @@ -8858,7 +8935,8 @@ qemuBuildCommandLine(virConnectPtr conn, } for (i = 0; i < def->nshmems; i++) { - if (qemuBuildShmemCommandLine(cmd, def, def->shmems[i], qemuCaps)) + if (qemuBuildShmemCommandLine(logManager, cmd, + def, def->shmems[i], qemuCaps)) goto error; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index fb684d0..160425e 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -32,6 +32,7 @@ # include "qemu_domain.h" # include "qemu_domain_address.h" # include "qemu_capabilities.h" +# include "logging/log_manager.h" /* Config type for XML import/export conversions */ # define QEMU_CONFIG_FORMAT_ARGV "qemu-argv" @@ -59,6 +60,7 @@ char *qemuBuildObjectCommandlineFromJSON(const char *type, virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virQEMUDriverPtr driver, + virLogManagerPtr logManager, virDomainDefPtr def, virDomainChrSourceDefPtr monitor_chr, bool monitor_json, @@ -72,7 +74,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virBitmapPtr nodeset, size_t *nnicindexes, int **nicindexes) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(10); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11); /* Generate '-device' string for chardev device */ int @@ -180,7 +182,10 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, char *qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps); -char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, +char *qemuBuildShmemBackendStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3fc1513..3d045cc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2609,6 +2609,12 @@ void qemuDomainLogContextRef(qemuDomainLogContextPtr ctxt) } +virLogManagerPtr qemuDomainLogContextGetManager(qemuDomainLogContextPtr ctxt) +{ + return ctxt->manager; +} + + void qemuDomainLogContextFree(qemuDomainLogContextPtr ctxt) { bool lastRef; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 8359b1a..f6ce19c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -35,6 +35,7 @@ # include "qemu_capabilities.h" # include "virchrdev.h" # include "virobject.h" +# include "logging/log_manager.h" # define QEMU_DOMAIN_FORMAT_LIVE_FLAGS \ (VIR_DOMAIN_XML_SECURE | \ @@ -377,6 +378,8 @@ void qemuDomainLogContextMarkPosition(qemuDomainLogContextPtr ctxt); void qemuDomainLogContextRef(qemuDomainLogContextPtr ctxt); void qemuDomainLogContextFree(qemuDomainLogContextPtr ctxt); +virLogManagerPtr qemuDomainLogContextGetManager(qemuDomainLogContextPtr ctxt); + const char *qemuFindQemuImgBinary(virQEMUDriverPtr driver); int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 45ff3c0..1afafd6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7159,7 +7159,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, } } - if (!(cmd = qemuBuildCommandLine(conn, driver, def, + if (!(cmd = qemuBuildCommandLine(conn, driver, NULL, def, &monConfig, monitor_json, qemuCaps, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e760182..03b7418 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4993,7 +4993,9 @@ qemuProcessLaunch(virConnectPtr conn, } VIR_DEBUG("Building emulator command line"); - if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, + if (!(cmd = qemuBuildCommandLine(conn, driver, + qemuDomainLogContextGetManager(logCtxt), + vm->def, priv->monConfig, priv->monJSON, priv->qemuCaps, incoming ? incoming->launchURI : NULL, snapshot, vmop, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ff93329..4c760ee 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -347,7 +347,7 @@ static int testCompareXMLToArgvFiles(const char *xml, testFailed = true; if (!testFailed && - !(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, + !(cmd = qemuBuildCommandLine(conn, &driver, NULL, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateURI, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, -- 2.5.0
Currently the file based character devices let QEMU write directly to a file on disk. This allows a malicious QEMU to inflict a denial of service by consuming all free space. Switch QEMU to use a pipe to virtlogd, which will enforce file rollover. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e04e55e..462a019 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3983,18 +3983,19 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, break; case VIR_DOMAIN_CHR_TYPE_FILE: - virBufferAsprintf(&buf, "file,id=char%s,path=%s", alias, - dev->data.file.path); - if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("append not supported in this QEMU binary")); - goto error; - } + virBufferAsprintf(&buf, "file,id=char%s", alias); - virBufferAsprintf(&buf, ",append=%s", - virTristateSwitchTypeToString(dev->data.file.append)); + if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("append not supported in this QEMU binary")); + goto error; } + if (qemuBuildChrChardevFileStr(virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND) ? + logManager : NULL, cmd, def, &buf, + "file", dev->data.file.path, + "append", dev->data.file.append) < 0) + goto error; break; case VIR_DOMAIN_CHR_TYPE_PIPE: -- 2.5.0
On 02/29/2016 08:33 AM, Daniel P. Berrange wrote:
Currently the file based character devices let QEMU write directly to a file on disk. This allows a malicious QEMU to inflict a denial of service by consuming all free space.
Switch QEMU to use a pipe to virtlogd, which will enforce file rollover.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e04e55e..462a019 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3983,18 +3983,19 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, break;
case VIR_DOMAIN_CHR_TYPE_FILE: - virBufferAsprintf(&buf, "file,id=char%s,path=%s", alias, ^^^^
(see below)
- dev->data.file.path); - if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("append not supported in this QEMU binary")); - goto error; - } + virBufferAsprintf(&buf, "file,id=char%s", alias);
- virBufferAsprintf(&buf, ",append=%s", - virTristateSwitchTypeToString(dev->data.file.append)); + if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("append not supported in this QEMU binary")); + goto error; } + if (qemuBuildChrChardevFileStr(virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND) ? + logManager : NULL, cmd, def, &buf, + "file", dev->data.file.path,
s/"file"/"path" ?
+ "append", dev->data.file.append) < 0) + goto error; break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
On 02/29/2016 08:33 AM, Daniel P. Berrange wrote:
This series of patches does two things
* Converts the type=file chardev over to use virtlogd (assuming use of virtlogd is enabled in qemu.conf)
* Adds a <log file="..."/> element to all chardev sources, allowing data to be captured to a logfile
Both of these are important features wanted by openstack. The first thing fixes a long standing security issue that a guest OS can trivially exhuast host disk space by outputing lots of data to its serial port.
The second thing allows OpenStack to record boot up messages for a guest's serial console, while still allowing interactive serial console ussage. ie the serial port will be configured with type=tcp, and the new <log> element used to record the data.
Both these thing required changes in QEMU, which have been merged for the forthcoming QEMU 2.6 release.
Changed in v2:
* Split patch 4 up into 4 separate patches * Fix memory leaks * Use common function for building log manager cli args * Add missing XML docs
Daniel P. Berrange (8): logging: allow inode/offset params to be NULL conf: allow use of a logfile with chardev backends qemu: add support for logging chardev output to a file qemu: don't append -chardev arg until after value is formatted qemu: move functions for handling FD passing logging: support truncation of logfiles when opening qemu: use virtlogd for character device log files qemu: support use of virtlogd with file based chardevs
docs/formatdomain.html.in | 14 + docs/schemas/domaincommon.rng | 12 + src/conf/domain_conf.c | 30 +++ src/conf/domain_conf.h | 2 + src/logging/log_daemon_dispatch.c | 3 +- src/logging/log_handler.c | 6 +- src/logging/log_handler.h | 2 +- src/logging/log_manager.c | 6 +- src/logging/log_manager.h | 2 + src/logging/log_protocol.x | 4 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 282 ++++++++++++++------- src/qemu/qemu_command.h | 9 +- src/qemu/qemu_domain.c | 6 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 4 +- .../qemuxml2argv-serial-file-log.args | 25 ++ .../qemuxml2argv-serial-file-log.xml | 39 +++ tests/qemuxml2argvtest.c | 4 +- 21 files changed, 353 insertions(+), 105 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-log.xml
As you know patch 3 (qemu_capabilities.{c|h} & 7 (qemu_command.c) will require handling merge conflicts... ACK series modulo a couple of nits pointed out. John
participants (2)
-
Daniel P. Berrange -
John Ferlan