[libvirt] [PATCH 2/2] qemu: implement debugcon-isa chardev

[Nikolay Shirokovskiy]

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 at 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