[libvirt] [PATCH 0/5] 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. Daniel P. Berrange (5): 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: use virtlogd for character device log files qemu: support use of virtlogd with file based chardevs docs/schemas/domaincommon.rng | 12 + src/conf/domain_conf.c | 28 ++ 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 | 295 ++++++++++++++------- src/qemu/qemu_command.h | 7 +- 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 +- 20 files changed, 350 insertions(+), 103 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
On 02/23/2016 11:41 AM, Daniel P. Berrange wrote:
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(-)
In retrospect, probably could have used ATTRIBUTE_NONNULL on these args previously... ACK John
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/schemas/domaincommon.rng | 12 ++++++++++++ src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 3 files changed, 42 insertions(+) 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 56bd1aa..92c3ce9 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); @@ -20093,6 +20112,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
On 02/23/2016 11:41 AM, Daniel P. Berrange wrote:
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/schemas/domaincommon.rng | 12 ++++++++++++ src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 3 files changed, 42 insertions(+)
Probably should be described in formatdomain.html.in too, right?
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"/>
Although rng requires absFilePath, it's not checked during parse. I think what separates this from others is that it's going to be used for write purposes. I see that virDomainSnapshotDefParse which seemingly can accept an file path as input does check if (def->file && def->file[0] == '/') to ensure absolute file path. I have a secondary concern about where the writing can occur...
+ </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 56bd1aa..92c3ce9 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");
Shouldn't we validate the logfile path at least? Even virLogParseOutputs validates what's in logfile is at least an absFilePath. I looked forward to the next patch and see it's just supplied to qemu. Unlike a number of the absDirPath elements I looked at in domain_conf, this is a "write" path and I would think we want to be ultra conservative... Secondary concern - not that it couldn't be done today w/ log_outputs file name in the libvirtd.conf file... What if someone supplied a name they shouldn't be supplying? That is somewhere they shouldn't be writing... Is there a way to "demand" that the value is to a specific path prefix? or maybe we should just dictate providing a "name" and it's up to the hypervisor to prepend the hypervisor specific path (IOW: the /var/log/libvirt/qemu). That way someone could mount whatever they want at /var/log/libvirt/qemu in order to store inordinate amount of logs (not clogging /var unnecessarily).
+ 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);
Need VIR_FREE(logfile) and VIR_FREE(logappend) John
@@ -20093,6 +20112,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. */
On Fri, Feb 26, 2016 at 10:33:10AM -0500, John Ferlan wrote:
On 02/23/2016 11:41 AM, Daniel P. Berrange wrote:
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/schemas/domaincommon.rng | 12 ++++++++++++ src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 3 files changed, 42 insertions(+)
Probably should be described in formatdomain.html.in too, right?
Yes, it should be.
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"/>
Although rng requires absFilePath, it's not checked during parse. I think what separates this from others is that it's going to be used for write purposes.
I see that virDomainSnapshotDefParse which seemingly can accept an file path as input does check if (def->file && def->file[0] == '/') to ensure absolute file path. I have a secondary concern about where the writing can occur...
@@ -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");
Shouldn't we validate the logfile path at least? Even virLogParseOutputs validates what's in logfile is at least an absFilePath. I looked forward to the next patch and see it's just supplied to qemu. Unlike a number of the absDirPath elements I looked at in domain_conf, this is a "write" path and I would think we want to be ultra conservative...
Secondary concern - not that it couldn't be done today w/ log_outputs file name in the libvirtd.conf file... What if someone supplied a name they shouldn't be supplying? That is somewhere they shouldn't be writing... Is there a way to "demand" that the value is to a specific path prefix? or maybe we should just dictate providing a "name" and it's up to the hypervisor to prepend the hypervisor specific path (IOW: the /var/log/libvirt/qemu). That way someone could mount whatever they want at /var/log/libvirt/qemu in order to store inordinate amount of logs (not clogging /var unnecessarily).
The ability to define XML is considered to be equivalent to having a root shell. We don't try to enforce rules for any of the paths in the XML whether disk filenames, log file names or character device backends. So this isn't causing a problem didn't already have.
@@ -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);
Need VIR_FREE(logfile) and VIR_FREE(logappend)
Ah yes. 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 :|
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 2b953ea..c8f41a3 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 78423e7..dee7208 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 854623d..624639e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1026,6 +1026,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
On 02/23/2016 11:41 AM, Daniel P. Berrange wrote:
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
Seems reasonable - although as a heads up you could have conflicts with Pavel's vram64 series... John
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2b953ea..c8f41a3 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 78423e7..dee7208 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 854623d..624639e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1026,6 +1026,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);
If use of virtlogd is enabled, then use it for backing the character device log files too. --- 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 + src/qemu/qemu_command.c | 234 ++++++++++++++++++++++++-------------- src/qemu/qemu_command.h | 7 +- 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 +- 12 files changed, 179 insertions(+), 96 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 */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dee7208..8378470 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> @@ -149,6 +150,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, @@ -3850,7 +3904,10 @@ qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, /* 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) { @@ -3973,10 +4030,42 @@ 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 (logManager) { + char *fdset, *fdpath; + int flags = 0; + int logfd; + + if (dev->logappend == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + dev->logfile, + flags, + NULL, NULL)) < 0) + goto error; + + virCommandPassFD(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (!(fdset = qemuVirCommandGetFDSet(cmd, logfd))) + goto error; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + VIR_FREE(fdset); + + if (!(fdpath = qemuVirCommandGetDevSet(cmd, logfd))) + goto error; + + virBufferAsprintf(&buf, ",logfile=%s,logappend=on", fdpath); + VIR_FREE(fdpath); + } else { + virBufferAsprintf(&buf, ",logfile=%s", dev->logfile); + if (dev->logappend != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&buf, ",logappend=%s", + virTristateSwitchTypeToString(dev->logappend)); + } } } @@ -4093,7 +4182,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) @@ -4106,10 +4197,11 @@ qemuBuildMonitorCommandLine(virCommandPtr cmd, /* Use -chardev if it's available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) { - virCommandAddArg(cmd, "-chardev"); - if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", + if (!(chrdev = qemuBuildChrChardevStr(logManager, cmd, def, + monitor_chr, "monitor", qemuCaps))) return -1; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, chrdev); VIR_FREE(chrdev); @@ -4248,7 +4340,10 @@ qemuBuildSclpDevStr(virDomainChrDefPtr dev) static int -qemuBuildRNGBackendChrdevStr(virDomainRNGDefPtr rng, +qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainRNGDefPtr rng, virQEMUCapsPtr qemuCaps, char **chr) { @@ -4261,7 +4356,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; } @@ -6567,7 +6663,10 @@ qemuBuildShmemDevStr(virDomainDefPtr def, } char * -qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, +qemuBuildShmemBackendStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { char *devstr = NULL; @@ -6578,13 +6677,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) @@ -6597,7 +6699,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); @@ -6724,59 +6827,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, @@ -7020,6 +7070,7 @@ qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virQEMUDriverPtr driver, + virLogManagerPtr logManager, virDomainDefPtr def, virDomainChrSourceDefPtr monitor_chr, bool monitor_json, @@ -7170,7 +7221,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,13 +7984,14 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &smartcard->data.passthru, smartcard->info.alias, qemuCaps))) { virBufferFreeAndReset(&opt); goto error; } + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -7976,11 +8029,12 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&serial->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &serial->source, serial->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8012,11 +8066,12 @@ 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, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + ¶llel->source, parallel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8045,11 +8100,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, channel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8090,11 +8146,12 @@ qemuBuildCommandLine(virConnectPtr conn, * the newer -chardev interface. */ ; } else { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, channel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } @@ -8124,11 +8181,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, console->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8143,11 +8201,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, console->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8487,13 +8546,14 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; char *devstr; - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &redirdev->source.chr, redirdev->info.alias, qemuCaps))) { goto error; } + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -8713,7 +8773,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 +8919,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..decf463 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, @@ -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 c56f9f1..df422e2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2603,6 +2603,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 c5b9f4e..3780d0c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4987,7 +4987,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 624639e..b5323b4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -345,7 +345,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
On 02/23/2016 11:41 AM, Daniel P. Berrange wrote:
If use of virtlogd is enabled, then use it for backing the character device log files too. --- 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 + src/qemu/qemu_command.c | 234 ++++++++++++++++++++++++--------------
<sigh> One of us will have conflicts depending upon whether my final pile of qemu_command.c cleanup changes gets reviewed...
src/qemu/qemu_command.h | 7 +- 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 +- 12 files changed, 179 insertions(+), 96 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;
So by changing this means you don't envision other flags in the future for virLogHandlerDomainOpenLogFile? IOW: Why isn't this done in the next layer down? Seems we'd be changing what we're doing (not that anything else is using this yet). Should perhaps this part be a separate patch?
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 +}; +
DomainOpenLogFile doesn't have flags now - it just has trunc - so nothing could be added here (at least w/r/t how I understand it).
/* Obtain a file handle suitable for writing to a * log file for a domain */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dee7208..8378470 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> @@ -149,6 +150,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; +} + +
[1] Code motion... Should it be separate?
static int qemuBuildObjectCommandLinePropsInternal(const char *key, const virJSONValue *value, @@ -3850,7 +3904,10 @@ qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, /* 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) { @@ -3973,10 +4030,42 @@ 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 (logManager) { + char *fdset, *fdpath; + int flags = 0; + int logfd; + + if (dev->logappend == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + dev->logfile, + flags, + NULL, NULL)) < 0) + goto error; + + virCommandPassFD(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (!(fdset = qemuVirCommandGetFDSet(cmd, logfd)))
Would this error path need to close logfd or is that done because of the above PassFD path?
+ goto error; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + VIR_FREE(fdset); + + if (!(fdpath = qemuVirCommandGetDevSet(cmd, logfd)))
Similar question about logfd...
+ goto error; + + virBufferAsprintf(&buf, ",logfile=%s,logappend=on", fdpath);
Always on? Doesn't matter what's been provided in logappend in the XML?
+ VIR_FREE(fdpath); + } else { + virBufferAsprintf(&buf, ",logfile=%s", dev->logfile); + if (dev->logappend != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&buf, ",logappend=%s", + virTristateSwitchTypeToString(dev->logappend)); + }
Could be a separate call rather than inlining...
} }
@@ -4093,7 +4182,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) @@ -4106,10 +4197,11 @@ qemuBuildMonitorCommandLine(virCommandPtr cmd, /* Use -chardev if it's available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) {
- virCommandAddArg(cmd, "-chardev"); - if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", + if (!(chrdev = qemuBuildChrChardevStr(logManager, cmd, def, + monitor_chr, "monitor", qemuCaps))) return -1; + virCommandAddArg(cmd, "-chardev");
doh! Although I suppose one could make the argument that all the additions of "-chardev" could be their own patch...
virCommandAddArg(cmd, chrdev); VIR_FREE(chrdev);
@@ -4248,7 +4340,10 @@ qemuBuildSclpDevStr(virDomainChrDefPtr dev)
static int -qemuBuildRNGBackendChrdevStr(virDomainRNGDefPtr rng, +qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainRNGDefPtr rng, virQEMUCapsPtr qemuCaps, char **chr) { @@ -4261,7 +4356,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; } @@ -6567,7 +6663,10 @@ qemuBuildShmemDevStr(virDomainDefPtr def, }
char * -qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, +qemuBuildShmemBackendStr(virLogManagerPtr logManager, + virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { char *devstr = NULL; @@ -6578,13 +6677,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) @@ -6597,7 +6699,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); @@ -6724,59 +6827,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; -} - -
[1] Code motion...
static char * qemuBuildTPMBackendStr(const virDomainDef *def, virCommandPtr cmd, @@ -7020,6 +7070,7 @@ qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virQEMUDriverPtr driver, + virLogManagerPtr logManager, virDomainDefPtr def, virDomainChrSourceDefPtr monitor_chr, bool monitor_json, @@ -7170,7 +7221,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,13 +7984,14 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
- virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &smartcard->data.passthru, smartcard->info.alias, qemuCaps))) { virBufferFreeAndReset(&opt); goto error; } + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -7976,11 +8029,12 @@ qemuBuildCommandLine(virConnectPtr conn,
/* Use -chardev with -device if they are available */ if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&serial->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &serial->source, serial->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -8012,11 +8066,12 @@ 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, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + ¶llel->source, parallel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -8045,11 +8100,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
- virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, channel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -8090,11 +8146,12 @@ qemuBuildCommandLine(virConnectPtr conn, * the newer -chardev interface. */ ; } else { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, channel->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } @@ -8124,11 +8181,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
- virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, console->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -8143,11 +8201,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
- virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, console->info.alias, qemuCaps))) goto error; + virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -8487,13 +8546,14 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; char *devstr;
- virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &redirdev->source.chr, redirdev->info.alias, qemuCaps))) { goto error; }
+ virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, devstr); VIR_FREE(devstr);
@@ -8713,7 +8773,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 +8919,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..decf463 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,
There's a "ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(10);" on this API, that 10 needs to change to 11. Also with Martin's "qemu: Shorten per-domain directory names" you could have to modify 16 & 17 too John
@@ -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 c56f9f1..df422e2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2603,6 +2603,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 c5b9f4e..3780d0c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4987,7 +4987,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 624639e..b5323b4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -345,7 +345,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,
On Fri, Feb 26, 2016 at 10:33:30AM -0500, John Ferlan wrote:
On 02/23/2016 11:41 AM, Daniel P. Berrange wrote:
If use of virtlogd is enabled, then use it for backing the character device log files too. --- 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 + src/qemu/qemu_command.c | 234 ++++++++++++++++++++++++--------------
<sigh> One of us will have conflicts depending upon whether my final pile of qemu_command.c cleanup changes gets reviewed...
src/qemu/qemu_command.h | 7 +- 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 +- 12 files changed, 179 insertions(+), 96 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;
So by changing this means you don't envision other flags in the future for virLogHandlerDomainOpenLogFile?
IOW: Why isn't this done in the next layer down? Seems we'd be changing what we're doing (not that anything else is using this yet).
This flag is defined by the remote protocol. I want to keep the virLogHandler class isolated from the remote protocol, by not exposing the flags to it. Hence I've gone for named arguments in the internal API and convert from the flags to the named arguemnts.
Should perhaps this part be a separate patch?
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 +}; +
DomainOpenLogFile doesn't have flags now - it just has trunc - so nothing could be added here (at least w/r/t how I understand it).
The wire protocol *does* have flags defined currently: struct virLogManagerProtocolDomainOpenLogFileArgs { virLogManagerProtocolNonNullString driver; virLogManagerProtocolDomain dom; virLogManagerProtocolNonNullString path; unsigned int flags; }; this code is just defining the first flag we'll use on the wire.
+/** + * 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; +} + +
[1] Code motion... Should it be separate?
Yeah, that would be best practice.
@@ -3973,10 +4030,42 @@ 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 (logManager) { + char *fdset, *fdpath; + int flags = 0; + int logfd; + + if (dev->logappend == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + dev->logfile, + flags, + NULL, NULL)) < 0) + goto error; + + virCommandPassFD(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (!(fdset = qemuVirCommandGetFDSet(cmd, logfd)))
Would this error path need to close logfd or is that done because of the above PassFD path?
Yep, because we pass VIR_COMMAND_PASS_FD_CLOSE_PARENT, we're guaranteed that virCommandFree will always close the FD for us, no matter what happens.
+ goto error; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + VIR_FREE(fdset); + + if (!(fdpath = qemuVirCommandGetDevSet(cmd, logfd)))
Similar question about logfd...
+ goto error; + + virBufferAsprintf(&buf, ",logfile=%s,logappend=on", fdpath);
Always on? Doesn't matter what's been provided in logappend in the XML?
The append vs truncate decision is taken by virtlogd - we pass that info in the virLogManagerDomainOpenLogFile call flags parameter above. At the QEMU level, it just gets an anonymous pipe file descriptor, which can only ever be used in append mode - you can't call ftruncate() on a pipe.
@@ -4106,10 +4197,11 @@ qemuBuildMonitorCommandLine(virCommandPtr cmd, /* Use -chardev if it's available */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) {
- virCommandAddArg(cmd, "-chardev"); - if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", + if (!(chrdev = qemuBuildChrChardevStr(logManager, cmd, def, + monitor_chr, "monitor", qemuCaps))) return -1; + virCommandAddArg(cmd, "-chardev");
doh! Although I suppose one could make the argument that all the additions of "-chardev" could be their own patch...
Yeah, could be cleaner to separate the chardev move 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 :|
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 | 50 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8378470..9ed1b97 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3934,17 +3934,49 @@ 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")); + if (logManager && virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { + char *fdset, *fdpath; + int flags = 0; + int logfd; + + if (dev->data.file.append == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + dev->data.file.path, + flags, + NULL, NULL)) < 0) goto error; - } - virBufferAsprintf(&buf, ",append=%s", - virTristateSwitchTypeToString(dev->data.file.append)); + virCommandPassFD(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (!(fdset = qemuVirCommandGetFDSet(cmd, logfd))) + goto error; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + VIR_FREE(fdset); + + if (!(fdpath = qemuVirCommandGetDevSet(cmd, logfd))) + goto error; + + virBufferAsprintf(&buf, "file,id=char%s,path=%s,append=on", alias, fdpath); + VIR_FREE(fdpath); + } else { + 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, ",append=%s", + virTristateSwitchTypeToString(dev->data.file.append)); + } } break; -- 2.5.0
On 02/23/2016 11:41 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 | 50 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8378470..9ed1b97 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3934,17 +3934,49 @@ 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")); + if (logManager && virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { + char *fdset, *fdpath; + int flags = 0; + int logfd; + + if (dev->data.file.append == VIR_TRISTATE_SWITCH_OFF) + flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE; + + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + def->uuid, + def->name, + dev->data.file.path, + flags, + NULL, NULL)) < 0) goto error; - }
- virBufferAsprintf(&buf, ",append=%s", - virTristateSwitchTypeToString(dev->data.file.append)); + virCommandPassFD(cmd, logfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + if (!(fdset = qemuVirCommandGetFDSet(cmd, logfd))) + goto error; + + virCommandAddArg(cmd, "-add-fd"); + virCommandAddArg(cmd, fdset); + VIR_FREE(fdset); + + if (!(fdpath = qemuVirCommandGetDevSet(cmd, logfd))) + goto error; + + virBufferAsprintf(&buf, "file,id=char%s,path=%s,append=on", alias, fdpath); + VIR_FREE(fdpath);
hmmmm this all looks very familiar... Seems we could have common API w/ patch 4... I'd obviously would have the same questions ;-) John
+ } else { + 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, ",append=%s", + virTristateSwitchTypeToString(dev->data.file.append)); + } } break;
participants (2)
-
Daniel P. Berrange -
John Ferlan