[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Copy console definition from serial



On Fri, Mar 09, 2012 at 06:48:42PM +0100, Jan Kiszka wrote:
> On 2011-11-16 14:14, Michal Privoznik wrote:
> > Now, when we support multiple consoles per domain,
> > the vm->def->console[0] can still remain an alias
> > for vm->def->serial[0]; However, we need to copy
> > it's source definition as well otherwise we'll regress
> > on virDomainOpenConsole.
> 
> ...
> 
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 2882ef8..e0b1824 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -1163,11 +1163,22 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm,
> >  
> >      for (i = 0 ; i < vm->def->nconsoles ; i++) {
> >          virDomainChrDefPtr chr = vm->def->consoles[i];
> > -        if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY &&
> > -            chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) {
> > -            if ((ret = qemuProcessExtractTTYPath(output, &offset,
> > -                                                 &chr->source.data.file.path)) != 0)
> > +        /* For historical reasons, console[0] can be just an alias
> > +         * for serial[0]; That's why we need to update it as well */
> > +        if (i == 0 && vm->def->nserials &&
> > +            chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
> > +            chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
> > +            if ((ret = virDomainChrSourceDefCopy(&chr->source,
> > +                                                 &((vm->def->serials[0])->source))) != 0)
> >                  return ret;
> > +            chr->source.type = VIR_DOMAIN_CHR_TYPE_PTY;
> 
> This unconditional setting of TYPE_PTY breaks serial on stdio (we use
> this to easily fold guest into host logs). Can you explain why the
> copied source.type of serial[0] is not always correct? Or are we already
> in the wrong branch for a
> 
>     <serial type='stdio'></serial>
> 
> configuration?

Yeah I think this is a bug. The first serial element should match the
first console exactly, with targetType==serial. We shouldn't be
forcing it to type=pty


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]