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

On Thu, Jan 24, 2019 at 05:29:00PM +0300, Nikolay Shirokovskiy wrote: > Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; > --- > 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.