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@xxxxxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
