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

Re: [libvirt] [PATCH 1/7] Allow multiple consoles per virtual guest



On Tue, Nov 01, 2011 at 04:03:27PM -0600, Eric Blake wrote:
> On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
> >From: "Daniel P. Berrange"<berrange redhat com>
> >
> >While Xen only has a single paravirt console, UML, and
> >QEMU both support multiple paravirt consoles. The LXC
> >driver can also be trivially made to support multiple
> >consoles. This patch extends the XML to allow multiple
> ><console>  elements in the XML. It also makes the UML
> >and QEMU drivers support this config.
> >
> 
> See my earlier question about documenting this in docs/formatdomain.html.in.
> 
> >+++ b/src/qemu/qemu_process.c
> >@@ -1112,8 +1112,8 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm,
> >                                paths, chardevfmt)<  0)
> >          return -1;
> >
> >-    if (vm->def->console&&
> >-        qemuProcessLookupPTYs(&vm->def->console, 1, paths, chardevfmt)<  0)
> >+    if (qemuProcessLookupPTYs(vm->def->consoles, vm->def->nconsoles,
> >+                              paths, chardevfmt)<  0)
> 
> Does qemuProcessLookupPTYs behave if vm->def->nconsoles is 0 (we
> previously skipped calling it if there were no consoles, so you may
> need to skip calling it here in the same situation).

We already call qemuProcessLookupPTYs unconditionally for nserials,
nparallels and nchannels & I've double checked its safe :-)

> >+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args
> >@@ -1,6 +1,7 @@
> >  LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> >-pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
> >-unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \
> >+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
> >+socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
> >+chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
> 
> Is this conversion from '-monitor' to '-device -mon' intentional?  I
> think its okay, but wonder if it should have been split into a
> separate patch, since it has nothing to do with consoles.
> 
> >+++ b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args
> >@@ -1,6 +1,7 @@
> >  LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> >-pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
> >-unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \
> >+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \
> >+socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \
> >+chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
> >  virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \
> >  /dev/HostVG/QEMUGuest1 -chardev pty,id=charconsole0 -device virtconsole,\
> >  chardev=charconsole0,id=console0 -usb -device virtio-balloon-pci,id=balloon0,\
> >diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> >index a7ae925..2d158cf 100644
> >--- a/tests/qemuxml2argvtest.c
> >+++ b/tests/qemuxml2argvtest.c
> >@@ -471,11 +471,13 @@ mymain(void)
> >      DO_TEST("channel-guestfwd", false,
> >              QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
> >      DO_TEST("channel-virtio", false,
> >-            QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
> >+            QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
> 
> Ah, I see - you altered the flags being passed, which is why the
> .args files had to change.  It makes sense that you need
> QEMU_CAPS_CHARDEV for console-virtio-many to work, and you just made
> the other tests consistent; still, my observation above about
> splitting just this part of the tests change into a separate commit
> might help.

Yes, this test case was using a bogus combination of capabilities flags
that would never exist in a real QEMU  binary, so giving test output
that was bogus and caused problems with many console support

Regards,
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]