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

[Nikolay Shirokovskiy]

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

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>