[libvirt] [PATCH 0/2] add debugcon-isa chardev guest interface
It corresponds to QEMU's isa-debugcon device. It can be used to output debug info by firmware at least. I do not introduce capability to test for the device because it was supported long before current minimal 1.5.0 version. Nikolay Shirokovskiy (2): conf: add debugcon-isa chardev guest interface qemu: implement debugcon-isa chardev docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 6 +++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 13 +++++++ src/qemu/qemu_domain.c | 20 +++++++++++ tests/qemuxml2argvdata/channel-debugcon-isa.args | 29 ++++++++++++++++ tests/qemuxml2argvdata/channel-debugcon-isa.xml | 34 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/channel-debugcon-isa.xml | 41 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 12 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.args create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.xml create mode 100644 tests/qemuxml2xmloutdata/channel-debugcon-isa.xml -- 1.8.3.1
This interface can be used for example by firmware to print debug messages. Here is domain xml example: <channel type='file'> <source path='/var/log/libvirt/qemu/VM.firmware.log'/> <target type='debugcon-isa'/> <address type='isa' iobase='0x402'/> </channel> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 6 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 3 ++ tests/qemuxml2argvdata/channel-debugcon-isa.xml | 34 +++++++++++++++++++ tests/qemuxml2xmloutdata/channel-debugcon-isa.xml | 41 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.xml create mode 100644 tests/qemuxml2xmloutdata/channel-debugcon-isa.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f07bb7..b760a9a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7305,6 +7305,12 @@ qemu-kvm -net nic,model=? /dev/null <channel type='spicevmc'> <target type='virtio' name='com.redhat.spice.0'/> </channel> + + <channel type='file'> + <source path='/var/log/libvirt/qemu/VM.firmware.log'/> + <target type='debugcon-isa'/> + <address type='isa' iobase='0x402'/> + </channel> </devices> ...</pre> @@ -7369,6 +7375,11 @@ qemu-kvm -net nic,model=? /dev/null optional <code>address</code> element can tie the channel to a particular <code>type='virtio-serial'</code> controller. <span class="since">Since 0.8.8</span></dd> + <dt><code>debugcon-isa</code></dt> + <dd> + Debug console on isa bus. Address on bus is given by optional + <code>address</code> element. + <span class="since">Since 5.1.0</span></dd> </dl> <h5><a id="elementsCharHostInterface">Host interface</a></h5> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index aa50eac..595f073 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4150,6 +4150,13 @@ </optional> </element> </define> + <define name="debugconIsaTarget"> + <element name="target"> + <attribute name="type"> + <value>debugcon-isa</value> + </attribute> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -4159,6 +4166,7 @@ <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> <ref name="xenTarget"/> + <ref name="debugconIsaTarget"/> </choice> <optional> <ref name="alias"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f3d541b..af41e4e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -458,7 +458,8 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget, "none", "guestfwd", "virtio", - "xen") + "xen", + "debugcon-isa") VIR_ENUM_IMPL(virDomainChrConsoleTarget, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, @@ -17230,6 +17231,9 @@ virDomainChrEquals(virDomainChrDefPtr src, sizeof(*src->target.addr)) == 0; break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: + break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST: /* shouldn't happen */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7776a3a..7319494 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1106,6 +1106,7 @@ typedef enum { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN, + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST } virDomainChrChannelTargetType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2fb7d32..e96b1a0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10787,6 +10787,9 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, goto cleanup; break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: + break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST: diff --git a/tests/qemuxml2argvdata/channel-debugcon-isa.xml b/tests/qemuxml2argvdata/channel-debugcon-isa.xml new file mode 100644 index 0000000..d2d4904 --- /dev/null +++ b/tests/qemuxml2argvdata/channel-debugcon-isa.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>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-system-i686</emulator> + <disk type='block' device='disk'> + <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'/> + <channel type='pipe'> + <source path='/tmp/debugcon-isa'/> + <target type='debugcon-isa'/> + <address type='isa' iobase='0x402'/> + </channel> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/channel-debugcon-isa.xml b/tests/qemuxml2xmloutdata/channel-debugcon-isa.xml new file mode 100644 index 0000000..bff97f5 --- /dev/null +++ b/tests/qemuxml2xmloutdata/channel-debugcon-isa.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static' cpuset='1-4,8-20,525'>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-system-i686</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'> + <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'/> + <channel type='pipe'> + <source path='/tmp/debugcon-isa'/> + <target type='debugcon-isa'/> + <address type='isa' iobase='0x402'/> + </channel> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 82e2c0e..dde2332 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -464,6 +464,7 @@ mymain(void) DO_TEST("channel-guestfwd", NONE); DO_TEST("channel-virtio", NONE); DO_TEST("channel-virtio-state", NONE); + DO_TEST("channel-debugcon-isa", NONE); DO_TEST_FULL("channel-unix-source-path", WHEN_INACTIVE, GIC_NONE, NONE); -- 1.8.3.1
On Thu, Jan 24, 2019 at 05:28:59PM +0300, Nikolay Shirokovskiy wrote:
This interface can be used for example by firmware to print debug messages. Here is domain xml example:
<channel type='file'> <source path='/var/log/libvirt/qemu/VM.firmware.log'/> <target type='debugcon-isa'/>
IMO 'debugcon' would be enough. Jano
<address type='isa' iobase='0x402'/> </channel>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 6 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 3 ++ tests/qemuxml2argvdata/channel-debugcon-isa.xml | 34 +++++++++++++++++++ tests/qemuxml2xmloutdata/channel-debugcon-isa.xml | 41 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.xml create mode 100644 tests/qemuxml2xmloutdata/channel-debugcon-isa.xml
On 25.01.2019 16:37, Ján Tomko wrote:
On Thu, Jan 24, 2019 at 05:28:59PM +0300, Nikolay Shirokovskiy wrote:
This interface can be used for example by firmware to print debug messages. Here is domain xml example:
<channel type='file'> <source path='/var/log/libvirt/qemu/VM.firmware.log'/> <target type='debugcon-isa'/>
IMO 'debugcon' would be enough.
I followed serial chardev xml. It uses pci-serial, usb-serial etc for target type. I think idea is to make it possible to specify type when address is not specified (if it is then one can use address type to distinguish). Of course we have only isa-debugcon device yet. However I agree that for channel configuration having enumeration like spicevm, guesfw, debugcon-isa looks like mixing types and subtypes. Nikolay
Jano
<address type='isa' iobase='0x402'/> </channel>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 11 ++++++ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 6 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 3 ++ tests/qemuxml2argvdata/channel-debugcon-isa.xml | 34 +++++++++++++++++++ tests/qemuxml2xmloutdata/channel-debugcon-isa.xml | 41 +++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.xml create mode 100644 tests/qemuxml2xmloutdata/channel-debugcon-isa.xml
On Fri, 2019-01-25 at 14:06 +0000, Nikolay Shirokovskiy wrote:
On 25.01.2019 16:37, Ján Tomko wrote:
On Thu, Jan 24, 2019 at 05:28:59PM +0300, Nikolay Shirokovskiy wrote:
This interface can be used for example by firmware to print debug messages. Here is domain xml example:
<channel type='file'> <source path='/var/log/libvirt/qemu/VM.firmware.log'/> <target type='debugcon-isa'/>
IMO 'debugcon' would be enough.
I followed serial chardev xml. It uses pci-serial, usb-serial etc for target type. I think idea is to make it possible to specify type when address is not specified (if it is then one can use address type to distinguish). Of course we have only isa-debugcon device yet. However I agree that for channel configuration having enumeration like spicevm, guesfw, debugcon-isa looks like mixing types and subtypes.
Should this be a <channel> at all? We generally use <serial> for native emulated devices such as isa-serial, which isa-debugcon seems very closely related to... Perhaps the best way to hook this up would be <serial type='pty'> <target type='isa-serial'> <model name='isa-debugcon'/> </target> </serial> CC'ing Pavel as we looked into this mess together the last time around, so he might have an opinion on the subject :) -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jan 28, 2019 at 11:24:38AM +0100, Andrea Bolognani wrote:
On Fri, 2019-01-25 at 14:06 +0000, Nikolay Shirokovskiy wrote:
On 25.01.2019 16:37, Ján Tomko wrote:
On Thu, Jan 24, 2019 at 05:28:59PM +0300, Nikolay Shirokovskiy wrote:
This interface can be used for example by firmware to print debug messages. Here is domain xml example:
<channel type='file'> <source path='/var/log/libvirt/qemu/VM.firmware.log'/> <target type='debugcon-isa'/>
IMO 'debugcon' would be enough.
I followed serial chardev xml. It uses pci-serial, usb-serial etc for target type. I think idea is to make it possible to specify type when address is not specified (if it is then one can use address type to distinguish). Of course we have only isa-debugcon device yet. However I agree that for channel configuration having enumeration like spicevm, guesfw, debugcon-isa looks like mixing types and subtypes.
Should this be a <channel> at all?
We generally use <serial> for native emulated devices such as isa-serial, which isa-debugcon seems very closely related to...
<channel> is aiming to expose application specific guest data channels. <serial> is aiming to expose general purpose guest data channels. To me isa-debugcon is an application specific data channels, so <channel> feels a better fit.
Perhaps the best way to hook this up would be
<serial type='pty'> <target type='isa-serial'>
IMHO using 'isa-serial' would be misleading for applications which know about <serial> ports today and may well cause unexpected side effects, especially wrt to the <console> mapping of the first <serial> port.
<model name='isa-debugcon'/> </target> </serial>
CC'ing Pavel as we looked into this mess together the last time around, so he might have an opinion on the subject :)
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 Mon, 2019-01-28 at 10:36 +0000, Daniel P. Berrangé wrote:
On Mon, Jan 28, 2019 at 11:24:38AM +0100, Andrea Bolognani wrote:
Should this be a <channel> at all?
We generally use <serial> for native emulated devices such as isa-serial, which isa-debugcon seems very closely related to...
<channel> is aiming to expose application specific guest data channels.
<serial> is aiming to expose general purpose guest data channels.
To me isa-debugcon is an application specific data channels, so <channel> feels a better fit.
Maybe I have misunderstood what isa-debugcon is, but it looks more of a general purpose channel in the vein of isa-serial rather than something application specific like the qemu-guest-agent or SPICE channels for example.
Perhaps the best way to hook this up would be
<serial type='pty'> <target type='isa-serial'>
IMHO using 'isa-serial' would be misleading for applications which know about <serial> ports today and may well cause unexpected side effects, especially wrt to the <console> mapping of the first <serial> port.
Good point about the <console>/<serial> mapping, which we should make sure is handled correctly. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jan 28, 2019 at 12:19:36PM +0100, Andrea Bolognani wrote:
On Mon, 2019-01-28 at 10:36 +0000, Daniel P. Berrangé wrote:
On Mon, Jan 28, 2019 at 11:24:38AM +0100, Andrea Bolognani wrote:
Should this be a <channel> at all?
We generally use <serial> for native emulated devices such as isa-serial, which isa-debugcon seems very closely related to...
<channel> is aiming to expose application specific guest data channels.
<serial> is aiming to expose general purpose guest data channels.
To me isa-debugcon is an application specific data channels, so <channel> feels a better fit.
Maybe I have misunderstood what isa-debugcon is, but it looks more of a general purpose channel in the vein of isa-serial rather than something application specific like the qemu-guest-agent or SPICE channels for example.
My understanding is that isa-debugcon is a special device just for usage by SeaBIOS to emit debug messages. 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 Mon, Jan 28, 2019 at 11:26:57AM +0000, Daniel P. Berrangé wrote:
On Mon, Jan 28, 2019 at 12:19:36PM +0100, Andrea Bolognani wrote:
On Mon, 2019-01-28 at 10:36 +0000, Daniel P. Berrangé wrote:
On Mon, Jan 28, 2019 at 11:24:38AM +0100, Andrea Bolognani wrote:
Should this be a <channel> at all?
We generally use <serial> for native emulated devices such as isa-serial, which isa-debugcon seems very closely related to...
<channel> is aiming to expose application specific guest data channels.
<serial> is aiming to expose general purpose guest data channels.
To me isa-debugcon is an application specific data channels, so <channel> feels a better fit.
Maybe I have misunderstood what isa-debugcon is, but it looks more of a general purpose channel in the vein of isa-serial rather than something application specific like the qemu-guest-agent or SPICE channels for example.
My understanding is that isa-debugcon is a special device just for usage by SeaBIOS to emit debug messages.
Well, it's a primitive, output only device with a single, configurable I/O port. Every byte written to the io port is send to the chardev. It's used in practice for firmware debug output. seabios and OVMF support this (coreboot too IIRC). And even though the port is configurable pretty much everybody agreed on using iobase=0x402. I'd classify this as serial device: <serial><target type='debugcon'/></serial> And, yes, you surely don't want use that as <console>. cheers, Gerd
On Mon, Jan 28, 2019 at 11:26:57AM +0000, Daniel P. Berrangé wrote:
On Mon, Jan 28, 2019 at 12:19:36PM +0100, Andrea Bolognani wrote:
On Mon, 2019-01-28 at 10:36 +0000, Daniel P. Berrangé wrote:
On Mon, Jan 28, 2019 at 11:24:38AM +0100, Andrea Bolognani wrote:
Should this be a <channel> at all?
We generally use <serial> for native emulated devices such as isa-serial, which isa-debugcon seems very closely related to...
<channel> is aiming to expose application specific guest data channels.
<serial> is aiming to expose general purpose guest data channels.
To me isa-debugcon is an application specific data channels, so <channel> feels a better fit.
Maybe I have misunderstood what isa-debugcon is, but it looks more of a general purpose channel in the vein of isa-serial rather than something application specific like the qemu-guest-agent or SPICE channels for example.
My understanding is that isa-debugcon is a special device just for usage by SeaBIOS to emit debug messages.
I'm not sure if it's that simple, if you look at the QEMU commit <c9f398e53fedb88df243e32eb9bc50fda4ec44d0> that introduced isa-debugcon there are also changes notes and apparently there are other non-ISA variants of the debugcon which are not implemented in QEMU but they exists. From what I understand it's a special device to gather debug messages from various firmwares, not only SeaBIOS, but also OVMF for example. According to our documentation: ======================================================================== Due to hystorical reasons, the serial and console elements have partially overlapping scopes. In general, both elements are used to configure one or more serial consoles to be used for interacting with the guest. The main difference between the two is that serial is used for emulated, usually native, serial consoles, whereas console is used for paravirtualized ones. Both emulated and paravirtualized serial consoles have advantages and disadvantages: - emulated serial consoles are usually initialized much earlier than paravirtualized ones, so they can be used to control the bootloader and display both firmware and early boot messages; - on several platforms, there can only be a single emulated serial console per guest but paravirtualized consoles don't suffer from the same limitation. ======================================================================== Based on that and the fact that this is a debug console for firmwares I'm more inclined to model it as <serial>. I would suggest something like this: <serial type='...'> <target type='isa-serial'> <model type='debugcon'/> or <model type='isa-debugcon'/> </serial> As Dan mentioned first 'isa-serial' is duplicated as <channel> no matter the model of the 'isa-serial' target. In order to use 'isa-serial' target we would have add some code to make sure that we duplicate the <serial> device only if the 'mode' is not set or is set to 'isa-serial'. Or if we would go with the channel: <channel type='...'> <target type='debugcon'> </channel> In case of channel we don't have to model the BUS into the type since we can pick the address type based on architecture unless the isa-debugcon works also on other architectures and not only on x86. Pavel
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 :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 10 ++++++++ src/qemu/qemu_domain.c | 20 ++++++++++++++++ tests/qemuxml2argvdata/channel-debugcon-isa.args | 29 ++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 62 insertions(+) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.args diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3d6306..37cb43a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -203,6 +203,8 @@ virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; virDomainCapsFeatureTypeToString; +virDomainChrChannelTargetTypeFromString; +virDomainChrChannelTargetTypeToString; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e96b1a0..7c28944 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9485,6 +9485,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, channel->source, @@ -10788,6 +10789,15 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: + if (chr->info.addr.isa.iobase) { + if (virAsprintf(deviceStr, "isa-debugcon,iobase=0x%x,chardev=char%s", + chr->info.addr.isa.iobase, chr->info.alias) < 0) + goto cleanup; + } else { + if (virAsprintf(deviceStr, "isa-debugcon,chardev=char%s", + chr->info.alias) < 0) + goto cleanup; + } break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 32a43f2..5f53784 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4451,7 +4451,19 @@ qemuDomainChrTargetDefValidate(const virDomainChrDef *chr) case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: + if (chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA && + chr->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + chr->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target type 'debugcon-isa' requires " + " address type 'isa'")); + return -1; + } + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: /* Nothing to do */ break; @@ -4516,6 +4528,14 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, } } + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + dev->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA && + !ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("debugcon-isa is not supported")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/channel-debugcon-isa.args b/tests/qemuxml2argvdata/channel-debugcon-isa.args new file mode 100644 index 0000000..6b57e60 --- /dev/null +++ b/tests/qemuxml2argvdata/channel-debugcon-isa.args @@ -0,0 +1,29 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-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,\ +bootindex=1 \ +-chardev pipe,id=charchannel0,path=/tmp/debugcon-isa \ +-device isa-debugcon,iobase=0x402,chardev=charchannel0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ba6fd4d..ddcbe41 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1499,6 +1499,7 @@ mymain(void) DO_TEST("channel-virtio-auto", NONE); DO_TEST("channel-virtio-autoassign", NONE); DO_TEST("channel-virtio-autoadd", NONE); + DO_TEST("channel-debugcon-isa", NONE); DO_TEST("console-virtio", NONE); DO_TEST("console-virtio-many", QEMU_CAPS_DEVICE_ISA_SERIAL); -- 1.8.3.1
On Thu, Jan 24, 2019 at 05:29:00PM +0300, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 10 ++++++++ src/qemu/qemu_domain.c | 20 ++++++++++++++++ tests/qemuxml2argvdata/channel-debugcon-isa.args | 29 ++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 62 insertions(+) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3d6306..37cb43a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -203,6 +203,8 @@ virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; virDomainCapsFeatureTypeToString; +virDomainChrChannelTargetTypeFromString; +virDomainChrChannelTargetTypeToString; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e96b1a0..7c28944 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9485,6 +9485,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, channel->source, @@ -10788,6 +10789,15 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: + if (chr->info.addr.isa.iobase) { + if (virAsprintf(deviceStr, "isa-debugcon,iobase=0x%x,chardev=char%s", + chr->info.addr.isa.iobase, chr->info.alias) < 0) + goto cleanup;
For the panic device, we format the iobase unconditionally. Doing the same here would prevent ambiguity. The default address could also be assigned during postParse, but we don't seem to do that for the panic device - not sure whether there was a good reason not to do it. Jano
On 25.01.2019 16:43, Ján Tomko wrote:
On Thu, Jan 24, 2019 at 05:29:00PM +0300, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 10 ++++++++ src/qemu/qemu_domain.c | 20 ++++++++++++++++ tests/qemuxml2argvdata/channel-debugcon-isa.args | 29 ++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 62 insertions(+) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3d6306..37cb43a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -203,6 +203,8 @@ virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; virDomainCapsFeatureTypeToString; +virDomainChrChannelTargetTypeFromString; +virDomainChrChannelTargetTypeToString; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e96b1a0..7c28944 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9485,6 +9485,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, channel->source, @@ -10788,6 +10789,15 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: + if (chr->info.addr.isa.iobase) { + if (virAsprintf(deviceStr, "isa-debugcon,iobase=0x%x,chardev=char%s", + chr->info.addr.isa.iobase, chr->info.alias) < 0) + goto cleanup;
For the panic device, we format the iobase unconditionally.
Nope. We have similar construction there switching on def->panics[i]->info.type.
Doing the same here would prevent ambiguity.
The default address could also be assigned during postParse, but we don't seem to do that for the panic device - not sure whether there was a good reason not to do it.
I agree that we can have issues with default addresses if qemu decides to change default value. For example during migration we won't be able to detect ABI incompatability. Yet we have same issue for panic device. Nikolay
On 25.01.2019 17:11, Nikolay Shirokovskiy wrote:
On 25.01.2019 16:43, Ján Tomko wrote:
On Thu, Jan 24, 2019 at 05:29:00PM +0300, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 10 ++++++++ src/qemu/qemu_domain.c | 20 ++++++++++++++++ tests/qemuxml2argvdata/channel-debugcon-isa.args | 29 ++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 62 insertions(+) create mode 100644 tests/qemuxml2argvdata/channel-debugcon-isa.args
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3d6306..37cb43a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -203,6 +203,8 @@ virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; virDomainCapsFeatureTypeToString; +virDomainChrChannelTargetTypeFromString; +virDomainChrChannelTargetTypeToString; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e96b1a0..7c28944 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9485,6 +9485,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, channel->source, @@ -10788,6 +10789,15 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_DEBUGCON_ISA: + if (chr->info.addr.isa.iobase) { + if (virAsprintf(deviceStr, "isa-debugcon,iobase=0x%x,chardev=char%s", + chr->info.addr.isa.iobase, chr->info.alias) < 0) + goto cleanup;
For the panic device, we format the iobase unconditionally.
Nope. We have similar construction there switching on def->panics[i]->info.type.
I did not notice the difference :) I check iobase instead of info.type. Well, iobase is optional... Nikolay
Doing the same here would prevent ambiguity.
The default address could also be assigned during postParse, but we don't seem to do that for the panic device - not sure whether there was a good reason not to do it.
I agree that we can have issues with default addresses if qemu decides to change default value. For example during migration we won't be able to detect ABI incompatability. Yet we have same issue for panic device.
Nikolay
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (6)
-
Andrea Bolognani -
Daniel P. Berrangé -
Gerd Hoffmann -
Ján Tomko -
Nikolay Shirokovskiy -
Pavel Hrdina