On Fri, Mar 09, 2012 at 07:11:05PM +0100, Jan Kiszka wrote: > On 2012-03-09 19:02, Daniel P. Berrange wrote: > > On Fri, Mar 09, 2012 at 06:58:06PM +0100, Jan Kiszka wrote: > >> On 2012-03-09 18:53, Daniel P. Berrange wrote: > >>> 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 > >> > >> So, if vm->def->serials[0])->source.type != VIR_DOMAIN_CHR_TYPE_PTY, we > >> should skip this fixup branch? > > > > No, we should do the fix for all types. serial[0] and console[0] should > > be identical, unless the console[0] targetType != TARGET_TYPE_SERIAL > > Well, libvirt is alien to me, and at this point I'm lost in fixups. :) > Do you mean simply dropping the source.type = TYPE_PTY line? Yes, I think that line can just be removed. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list