src/qemu/libvirtd_qemu.aug | 3 ++ src/qemu/qemu.conf.in | 9 ++++ src/qemu/qemu_conf.c | 4 ++ src/qemu/qemu_conf.h | 2 + src/qemu/qemu_domain.c | 13 +++++ src/qemu/test_libvirtd_qemu.aug.in | 1 + tests/qemuxml2argvdata/serial-append.xml | 53 +++++++++++++++++++ .../qemuxml2xmloutdata/serial-append.off.xml | 53 +++++++++++++++++++ tests/qemuxml2xmloutdata/serial-append.on.xml | 53 +++++++++++++++++++ tests/qemuxml2xmloutdata/serial-append.xml | 53 +++++++++++++++++++ tests/qemuxml2xmltest.c | 10 ++++ 11 files changed, 254 insertions(+) create mode 100644 tests/qemuxml2argvdata/serial-append.xml create mode 100644 tests/qemuxml2xmloutdata/serial-append.off.xml create mode 100644 tests/qemuxml2xmloutdata/serial-append.on.xml create mode 100644 tests/qemuxml2xmloutdata/serial-append.xml
Serial log file contains lots of useful information for debugging
configuration problems. It makes sense to preserve the log in between
restarts, so that one can later figure out what was going on. Before
that, we could do that on a per-domain basis, now we can configure it
once for all domains.
Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
---
src/qemu/libvirtd_qemu.aug | 3 ++
src/qemu/qemu.conf.in | 9 ++++
src/qemu/qemu_conf.c | 4 ++
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_domain.c | 13 +++++
src/qemu/test_libvirtd_qemu.aug.in | 1 +
tests/qemuxml2argvdata/serial-append.xml | 53 +++++++++++++++++++
.../qemuxml2xmloutdata/serial-append.off.xml | 53 +++++++++++++++++++
tests/qemuxml2xmloutdata/serial-append.on.xml | 53 +++++++++++++++++++
tests/qemuxml2xmloutdata/serial-append.xml | 53 +++++++++++++++++++
tests/qemuxml2xmltest.c | 10 ++++
11 files changed, 254 insertions(+)
create mode 100644 tests/qemuxml2argvdata/serial-append.xml
create mode 100644 tests/qemuxml2xmloutdata/serial-append.off.xml
create mode 100644 tests/qemuxml2xmloutdata/serial-append.on.xml
create mode 100644 tests/qemuxml2xmloutdata/serial-append.xml
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index ed097ea3d9..7f3eec7cfd 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -147,6 +147,8 @@ module Libvirtd_qemu =
let capability_filters_entry = str_array_entry "capability_filters"
+ let serial_file_append_entry = str_entry "serial_file_append"
+
(* Each entry in the config is one of the following ... *)
let entry = default_tls_entry
| vnc_entry
@@ -171,6 +173,7 @@ module Libvirtd_qemu =
| swtpm_entry
| capability_filters_entry
| obsolete_entry
+ | serial_file_append_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index 3895d42514..9c9a535e8d 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -968,3 +968,12 @@
# "full" - both QEMU and its helper processes are placed into separate
# scheduling group
#sched_core = "none"
+
+# Default append mode for writing to serial file. QEMU will set the chosen
+# value everytime it processes the config, unless some value is already there.
+#
+# Possible options are:
+# "default" - leave as-is for QEMU to decide.
+# "on" - set append value to "on", meaning file won't be truncated on restart
+# "off" - set append value to "off", file will be cleared on restart
+#serial_file_append = "default"
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ae5bbcd138..bfd93a168f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -288,6 +288,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
return NULL;
cfg->deprecationBehavior = g_strdup("none");
+ cfg->serialFileAppend = g_strdup("default");
return g_steal_pointer(&cfg);
}
@@ -376,6 +377,7 @@ static void virQEMUDriverConfigDispose(void *obj)
g_strfreev(cfg->capabilityfilters);
g_free(cfg->deprecationBehavior);
+ g_free(cfg->serialFileAppend);
}
@@ -903,6 +905,8 @@ virQEMUDriverConfigLoadDebugEntry(virQEMUDriverConfig *cfg,
return -1;
if (virConfGetValueString(conf, "deprecation_behavior", &cfg->deprecationBehavior) < 0)
return -1;
+ if (virConfGetValueString(conf, "serial_file_append", &cfg->serialFileAppend) < 0)
+ return -1;
return 0;
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8cf2dd2ec5..316200f38d 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -229,6 +229,8 @@ struct _virQEMUDriverConfig {
char *deprecationBehavior;
virQEMUSchedCore schedCore;
+
+ char *serialFileAppend;
};
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virQEMUDriverConfig, virObjectUnref);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8ae458ae45..0d1fe1ffcc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -37,6 +37,7 @@
#include "qemu_validate.h"
#include "qemu_namespace.h"
#include "viralloc.h"
+#include "virenum.h"
#include "virlog.h"
#include "virerror.h"
#include "viridentity.h"
@@ -5357,6 +5358,8 @@ qemuDomainChrDefPostParse(virDomainChrDef *chr,
virQEMUDriver *driver,
unsigned int parseFlags)
{
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+
/* Historically, isa-serial and the default matched, so in order to
* maintain backwards compatibility we map them here. The actual default
* will be picked below based on the architecture and machine type. */
@@ -5428,6 +5431,16 @@ qemuDomainChrDefPostParse(virDomainChrDef *chr,
}
}
+
+ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+ chr->source &&
+ chr->source->type == VIR_DOMAIN_CHR_TYPE_FILE &&
+ chr->source->data.file.append == VIR_TRISTATE_SWITCH_ABSENT) {
+
+ chr->source->data.file.append =
+ virTristateSwitchTypeFromString(cfg->serialFileAppend);
+ }
+
return 0;
}
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 1dbd692921..d1ca9c8d3d 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -117,3 +117,4 @@ module Test_libvirtd_qemu =
}
{ "deprecation_behavior" = "none" }
{ "sched_core" = "none" }
+{ "serial_file_append" = "default" }
diff --git a/tests/qemuxml2argvdata/serial-append.xml b/tests/qemuxml2argvdata/serial-append.xml
new file mode 100644
index 0000000000..662a711dd0
--- /dev/null
+++ b/tests/qemuxml2argvdata/serial-append.xml
@@ -0,0 +1,53 @@
+<domain type='qemu'>
+ <name>machine</name>
+ <uuid>2187c512-ff97-47d7-b67c-c02d3bdc219d</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0' model='piix3-uhci'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='file'>
+ <source path='/tmp/serial.file'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='0'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <serial type='unix'>
+ <source mode='connect' path='/tmp/serial.sock'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='1'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <console type='file'>
+ <source path='/tmp/serial.file'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/serial-append.off.xml b/tests/qemuxml2xmloutdata/serial-append.off.xml
new file mode 100644
index 0000000000..83fd37a0d1
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/serial-append.off.xml
@@ -0,0 +1,53 @@
+<domain type='qemu'>
+ <name>machine</name>
+ <uuid>2187c512-ff97-47d7-b67c-c02d3bdc219d</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0' model='piix3-uhci'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='file'>
+ <source path='/tmp/serial.file' append='off'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='0'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <serial type='unix'>
+ <source mode='connect' path='/tmp/serial.sock'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='1'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <console type='file'>
+ <source path='/tmp/serial.file' append='off'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/serial-append.on.xml b/tests/qemuxml2xmloutdata/serial-append.on.xml
new file mode 100644
index 0000000000..e54959a725
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/serial-append.on.xml
@@ -0,0 +1,53 @@
+<domain type='qemu'>
+ <name>machine</name>
+ <uuid>2187c512-ff97-47d7-b67c-c02d3bdc219d</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0' model='piix3-uhci'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='file'>
+ <source path='/tmp/serial.file' append='on'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='0'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <serial type='unix'>
+ <source mode='connect' path='/tmp/serial.sock'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='1'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <console type='file'>
+ <source path='/tmp/serial.file' append='on'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/serial-append.xml b/tests/qemuxml2xmloutdata/serial-append.xml
new file mode 100644
index 0000000000..662a711dd0
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/serial-append.xml
@@ -0,0 +1,53 @@
+<domain type='qemu'>
+ <name>machine</name>
+ <uuid>2187c512-ff97-47d7-b67c-c02d3bdc219d</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' 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-system-x86_64</emulator>
+ <controller type='usb' index='0' model='piix3-uhci'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='file'>
+ <source path='/tmp/serial.file'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='0'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <serial type='unix'>
+ <source mode='connect' path='/tmp/serial.sock'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='isa-serial' port='1'>
+ <model name='isa-serial'/>
+ </target>
+ </serial>
+ <console type='file'>
+ <source path='/tmp/serial.file'>
+ <seclabel model='dac' relabel='no'/>
+ </source>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e13da8bd2c..e4ccee0cf7 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -915,6 +915,16 @@ mymain(void)
QEMU_CAPS_DEVICE_VIRTIO_RNG,
QEMU_CAPS_OBJECT_RNG_EGD);
+ DO_TEST_NOCAPS("serial-append");
+
+ g_free(cfg->serialFileAppend);
+ cfg->serialFileAppend = g_strdup("on");
+ DO_TEST_FULL("serial-append", ".on", WHEN_BOTH, ARG_END);
+
+ g_free(cfg->serialFileAppend);
+ cfg->serialFileAppend = g_strdup("off");
+ DO_TEST_FULL("serial-append", ".off", WHEN_BOTH, ARG_END);
+
DO_TEST_NOCAPS("cpu-numa1");
DO_TEST_NOCAPS("cpu-numa2");
DO_TEST_NOCAPS("cpu-numa-no-memory-element");
--
2.39.0On Tue, Jan 10, 2023 at 02:42:34PM +0600, Oleg Vasilev wrote: > Serial log file contains lots of useful information for debugging > configuration problems. It makes sense to preserve the log in between > restarts, so that one can later figure out what was going on. Before > that, we could do that on a per-domain basis, now we can configure it > once for all domains. IMHO this is the job of the mgmt app that is using libvirt. We've got a few places in qemu.conf that interact with the guest config, but I largely consider them to be historical mistakes that shouldn't be added to. Adding this setting specifically for serial devs is opening up a can of worms IMHO. eg what if you have multiple serial ports and only want one of them in append mode. There are many other devices with chardev backends which log to files, but this setting only touches serial ports. Ultimately append/truncate is a policy decision for the mgmt app to make, not a libvirt host level tunable. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Jan 10, 2023 at 09:03:40AM +0000, Daniel P. Berrangé wrote: >On Tue, Jan 10, 2023 at 02:42:34PM +0600, Oleg Vasilev wrote: >> Serial log file contains lots of useful information for debugging >> configuration problems. It makes sense to preserve the log in between >> restarts, so that one can later figure out what was going on. Before >> that, we could do that on a per-domain basis, now we can configure it >> once for all domains. > >IMHO this is the job of the mgmt app that is using libvirt. > >We've got a few places in qemu.conf that interact with the >guest config, but I largely consider them to be historical >mistakes that shouldn't be added to. > >Adding this setting specifically for serial devs is opening >up a can of worms IMHO. eg what if you have multiple serial >ports and only want one of them in append mode. There are >many other devices with chardev backends which log to files, >but this setting only touches serial ports. > >Ultimately append/truncate is a policy decision for the >mgmt app to make, not a libvirt host level tunable. > I agree with that since it allows for more configurability and it is concentrated in one place, the mgmt app. Defaulting to a value is something that should not be difficult to implement in any controlling application that uses libvirt, so I don't really see the benefit for other mgmt apps either. >With regards, >Daniel >-- >|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| >|: https://libvirt.org -o- https://fstop138.berrange.com :| >|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| >
On 10.01.2023 15:09, Martin Kletzander wrote: > On Tue, Jan 10, 2023 at 09:03:40AM +0000, Daniel P. Berrangé wrote: >> On Tue, Jan 10, 2023 at 02:42:34PM +0600, Oleg Vasilev wrote: >>> Serial log file contains lots of useful information for debugging >>> configuration problems. It makes sense to preserve the log in between >>> restarts, so that one can later figure out what was going on. Before >>> that, we could do that on a per-domain basis, now we can configure it >>> once for all domains. >> >> IMHO this is the job of the mgmt app that is using libvirt. >> >> We've got a few places in qemu.conf that interact with the >> guest config, but I largely consider them to be historical >> mistakes that shouldn't be added to. >> >> Adding this setting specifically for serial devs is opening >> up a can of worms IMHO. eg what if you have multiple serial >> ports and only want one of them in append mode. There are >> many other devices with chardev backends which log to files, >> but this setting only touches serial ports. Hi, What would be other cases with chardev file backends worth considering? >> >> Ultimately append/truncate is a policy decision for the >> mgmt app to make, not a libvirt host level tunable. >> > > I agree with that since it allows for more configurability and it is > concentrated in one place, the mgmt app. Defaulting to a value is > something that should not be difficult to implement in any controlling > application that uses libvirt, so I don't really see the benefit for > other mgmt apps either. The issue here is that we have a 5 different management apps, and we would have to introduce this change to all of them. These are in different technology stacks, different languages, so it would be more complex to duplicate the implementation of this feature. Another use case for this, similar to the one I had during my previous experience in Huawei. I was given multiple libvirt's XMLs to debug issues with boot. No management app was involved, except initial creation through virt-install by third party. It would be beneficial to automatically retain serial logs to ease the debugging process. Best wishes, Oleg > >> With regards, >> Daniel >> -- >> |: https://berrange.com -o- >> https://www.flickr.com/photos/dberrange :| >> |: https://libvirt.org -o- >> https://fstop138.berrange.com :| >> |: https://entangle-photo.org -o- >> https://www.instagram.com/dberrange :| >>
On Tue, Jan 10, 2023 at 04:20:59PM +0600, Oleg Vasilev wrote: > > > On 10.01.2023 15:09, Martin Kletzander wrote: > > On Tue, Jan 10, 2023 at 09:03:40AM +0000, Daniel P. Berrangé wrote: > > > On Tue, Jan 10, 2023 at 02:42:34PM +0600, Oleg Vasilev wrote: > > > > Serial log file contains lots of useful information for debugging > > > > configuration problems. It makes sense to preserve the log in between > > > > restarts, so that one can later figure out what was going on. Before > > > > that, we could do that on a per-domain basis, now we can configure it > > > > once for all domains. > > > > > > IMHO this is the job of the mgmt app that is using libvirt. > > > > > > We've got a few places in qemu.conf that interact with the > > > guest config, but I largely consider them to be historical > > > mistakes that shouldn't be added to. > > > > > > Adding this setting specifically for serial devs is opening > > > up a can of worms IMHO. eg what if you have multiple serial > > > ports and only want one of them in append mode. There are > > > many other devices with chardev backends which log to files, > > > but this setting only touches serial ports. > > What would be other cases with chardev file backends worth considering? I don't want to consider adding even more cases to the qemu.conf. > > > Ultimately append/truncate is a policy decision for the > > > mgmt app to make, not a libvirt host level tunable. > > > > > > > I agree with that since it allows for more configurability and it is > > concentrated in one place, the mgmt app. Defaulting to a value is > > something that should not be difficult to implement in any controlling > > application that uses libvirt, so I don't really see the benefit for > > other mgmt apps either. > > The issue here is that we have a 5 different management apps, and we would > have to introduce this change to all of them. These are in different > technology stacks, different languages, so it would be more complex to > duplicate the implementation of this feature. If you have 5 different mgmt apps then I feel a simple boolean flag for the <serial> console configuration when provisioning a guest should be negligible in the big picture. I don't think the answer is to push policy logic from the apps into libvirt via qemu.conf. > Another use case for this, similar to the one I had during my previous > experience in Huawei. I was given multiple libvirt's XMLs to debug issues > with boot. No management app was involved, except initial creation through > virt-install by third party. It would be beneficial to automatically retain > serial logs to ease the debugging process. If debugging a VM without a mgmt app being involved, it is trivial to use 'virsh edit' add the flag to the XML. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 10.01.2023 16:35, Daniel P. Berrangé wrote: > On Tue, Jan 10, 2023 at 04:20:59PM +0600, Oleg Vasilev wrote: >> >> >> On 10.01.2023 15:09, Martin Kletzander wrote: >>> On Tue, Jan 10, 2023 at 09:03:40AM +0000, Daniel P. Berrangé wrote: >>>> On Tue, Jan 10, 2023 at 02:42:34PM +0600, Oleg Vasilev wrote: >>>>> Serial log file contains lots of useful information for debugging >>>>> configuration problems. It makes sense to preserve the log in between >>>>> restarts, so that one can later figure out what was going on. Before >>>>> that, we could do that on a per-domain basis, now we can configure it >>>>> once for all domains. >>>> >>>> IMHO this is the job of the mgmt app that is using libvirt. >>>> >>>> We've got a few places in qemu.conf that interact with the >>>> guest config, but I largely consider them to be historical >>>> mistakes that shouldn't be added to. >>>> >>>> Adding this setting specifically for serial devs is opening >>>> up a can of worms IMHO. eg what if you have multiple serial >>>> ports and only want one of them in append mode. There are >>>> many other devices with chardev backends which log to files, >>>> but this setting only touches serial ports. >> >> What would be other cases with chardev file backends worth considering? > > I don't want to consider adding even more cases to the qemu.conf. > >>>> Ultimately append/truncate is a policy decision for the >>>> mgmt app to make, not a libvirt host level tunable. >>>> >>> >>> I agree with that since it allows for more configurability and it is >>> concentrated in one place, the mgmt app. Defaulting to a value is >>> something that should not be difficult to implement in any controlling >>> application that uses libvirt, so I don't really see the benefit for >>> other mgmt apps either. >> >> The issue here is that we have a 5 different management apps, and we would >> have to introduce this change to all of them. These are in different >> technology stacks, different languages, so it would be more complex to >> duplicate the implementation of this feature. > > If you have 5 different mgmt apps then I feel a simple boolean > flag for the <serial> console configuration when provisioning > a guest should be negligible in the big picture. You are right, that's not the biggest thing, but it still would take time to develop across different teams. > I don't think > the answer is to push policy logic from the apps into > libvirt via qemu.conf. Why not? I remember the concern of trying to make user's expectation for importing a configuration into any libvirt installation and expecting the same guest-facing behavior, but: - This is not observable from guest - The idea of having some way to configure policy on meta-domain level seems valuable to me, as this would make libvirt more wholesome without the need of even existence of the management app. This way could provide a way for interoperability of different setups, meaning people don't have to learn the specifics of every mgmt app configuration. They just need to know how to flip a flag in qemu.conf (or some other place for this meta-domain policy configuration we could devise) > >> Another use case for this, similar to the one I had during my previous >> experience in Huawei. I was given multiple libvirt's XMLs to debug issues >> with boot. No management app was involved, except initial creation through >> virt-install by third party. It would be beneficial to automatically retain >> serial logs to ease the debugging process. > > If debugging a VM without a mgmt app being involved, it is trivial > to use 'virsh edit' add the flag to the XML. That's true when you have a single VM. When there are multiple, it becomes tedious. Another case is when support takes over the machine from the customer, flips the flag to 'on', restarts the VM and can expect the logs to be preserved. > > With regards, > Daniel
© 2016 - 2024 Red Hat, Inc.