[libvirt] [PATCH v2] qemu: Add support for SGA
This patch creates new attribute 'sga' for <serial> element. Serial Graphics Adapter allows users to see BIOS messages from the very first moment domain boots up. Therefore, users can choose boot medium, set PXE, etc. However, to be able to use this, one need SGABIOS, which is accessible here: http://code.google.com/p/sgabios/ --- diff to v1: -move from <video> to <serial> as Dan suggested: https://www.redhat.com/archives/libvir-list/2011-July/msg00134.html docs/formatdomain.html.in | 10 ++++-- docs/schemas/domain.rng | 20 +++++++++--- src/conf/domain_conf.c | 34 ++++++++++++++++++- src/conf/domain_conf.h | 10 ++++++ src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 +++ .../qemuxml2argv-serial-pty-chardev.args | 3 +- .../qemuxml2argv-serial-pty-chardev.xml | 2 +- tests/qemuxml2argvtest.c | 3 +- 10 files changed, 78 insertions(+), 13 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fe8d74c..1e9eee5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2040,7 +2040,7 @@ qemu-kvm -net nic,model=? /dev/null <source path='/dev/pts/2'/> <target port='0'/> </parallel> - <serial type='pty'> + <serial type='pty' sga='on'> <source path='/dev/pts/3'/> <target port='0'/> </serial> @@ -2105,7 +2105,7 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <serial type='pty'> + <serial type='pty' sga='on'> <source path='/dev/pts/3'/> <target port='0'/> </serial> @@ -2115,7 +2115,11 @@ qemu-kvm -net nic,model=? /dev/null <p> <code>target</code> can have a <code>port</code> attribute, which specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. + usually 0, 1 or 2 serial ports. The <code>sga</code> attribute enables + or disables Serial Graphics Adapter. Accepted values are <code>on</code> + and <code>off</code>. To be able to use this feature, you need to install + <a href="http://code.google.com/p/sgabios/">SGABios</a>. SGA + <span class="since">Since 0.9.4</span> </p> <h6><a name="elementCharConsole">Console</a></h6> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index fb1497b..e9539d5 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1618,11 +1618,21 @@ --> <define name="qemucdev"> <ref name="qemucdevSrcType"/> - <optional> - <attribute name="tty"> - <ref name="absFilePath"/> - </attribute> - </optional> + <interleave> + <optional> + <attribute name="tty"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <attribute name="sga"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> + </interleave> <interleave> <ref name="qemucdevSrcDef"/> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 109a947..cb7fe23 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -258,6 +258,11 @@ VIR_ENUM_IMPL(virDomainChrSpicevmc, VIR_DOMAIN_CHR_SPICEVMC_LAST, "vdagent", "smartcard") +VIR_ENUM_IMPL(virDomainChrSGA, VIR_DOMAIN_CHR_SGA_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainSmartcard, VIR_DOMAIN_SMARTCARD_TYPE_LAST, "host", "host-certificates", @@ -3488,6 +3493,10 @@ virDomainChrDefNew(void) { * <target port="1"/> * </serial> * + * <serial type="pty" sga="on"> + * <target port="0"/> + * </serial> + * * <serial type="dev"> * <source path="/dev/ttyS0"/> * <target port="1"/> @@ -3522,6 +3531,7 @@ virDomainChrDefParseXML(virCapsPtr caps, int flags) { xmlNodePtr cur; char *type = NULL; + char *sga = NULL; const char *nodeName; virDomainChrDefPtr def; int remaining; @@ -3530,6 +3540,7 @@ virDomainChrDefParseXML(virCapsPtr caps, return NULL; type = virXMLPropString(node, "type"); + sga = virXMLPropString(node, "sga"); if (type == NULL) { def->source.type = VIR_DOMAIN_CHR_TYPE_PTY; } else if ((def->source.type = virDomainChrTypeFromString(type)) < 0) { @@ -3539,6 +3550,13 @@ virDomainChrDefParseXML(virCapsPtr caps, goto error; } + if (sga && (def->source.sga=virDomainChrSGATypeFromString(sga)) <= 0) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown value for sga: %s"), + sga); + goto error; + } + nodeName = (const char *) node->name; if ((def->deviceType = virDomainChrDeviceTypeFromString(nodeName)) < 0) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3580,6 +3598,7 @@ virDomainChrDefParseXML(virCapsPtr caps, cleanup: VIR_FREE(type); + VIR_FREE(sga); return def; @@ -8739,9 +8758,11 @@ static int virDomainChrSourceDefFormat(virBufferPtr buf, virDomainChrSourceDefPtr def, bool tty_compat, + int deviceType, int flags) { const char *type = virDomainChrTypeToString(def->type); + const char *sga = virDomainChrSGATypeToString(def->sga); if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -8755,6 +8776,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " tty='%s'", def->data.file.path); } + if (deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && def->sga) { + if (!sga) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected sga value %d"), def->sga); + return -1; + } + virBufferAsprintf(buf, " sga='%s'", sga); + } virBufferAddLit(buf, ">\n"); switch (def->type) { @@ -8857,7 +8886,8 @@ virDomainChrDefFormat(virBufferPtr buf, def->source.type == VIR_DOMAIN_CHR_TYPE_PTY && !(flags & VIR_DOMAIN_XML_INACTIVE) && def->source.data.file.path); - if (virDomainChrSourceDefFormat(buf, &def->source, tty_compat, flags) < 0) + if (virDomainChrSourceDefFormat(buf, &def->source, tty_compat, + def->deviceType, flags) < 0) return -1; /* Format <target> block */ @@ -8962,7 +8992,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: if (virDomainChrSourceDefFormat(buf, &def->data.passthru, false, - flags) < 0) + -1, flags) < 0) return -1; break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 258289a..ab0632c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -457,11 +457,20 @@ enum virDomainChrSpicevmcName { VIR_DOMAIN_CHR_SPICEVMC_LAST, }; +enum virDomainChrSGA { + VIR_DOMAIN_CHR_SGA_DEFAULT = 0, + VIR_DOMAIN_CHR_SGA_ON, + VIR_DOMAIN_CHR_SGA_OFF, + + VIR_DOMAIN_CHR_SGA_LAST +}; + /* The host side information for a character device. */ typedef struct _virDomainChrSourceDef virDomainChrSourceDef; typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; struct _virDomainChrSourceDef { int type; /* virDomainChrType */ + int sga; /* Serial Graphics Adapter */ union { /* no <source> for null, vc, stdio */ struct { @@ -1570,6 +1579,7 @@ VIR_ENUM_DECL(virDomainSmartcard) VIR_ENUM_DECL(virDomainChr) VIR_ENUM_DECL(virDomainChrTcpProtocol) VIR_ENUM_DECL(virDomainChrSpicevmc) +VIR_ENUM_DECL(virDomainChrSGA) VIR_ENUM_DECL(virDomainSoundModel) VIR_ENUM_DECL(virDomainMemballoonModel) VIR_ENUM_DECL(virDomainSmbiosMode) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ad62a07..2dfe375 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -122,6 +122,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "pci-multifunction", /* 60 */ "virtio-blk-pci.ioeventfd", + "sga", ); struct qemu_feature_flags { @@ -1210,6 +1211,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) qemuCapsSet(flags, QEMU_CAPS_DEVICE_QXL_VGA); if (strstr(str, "virtio-blk-pci.ioeventfd")) qemuCapsSet(flags, QEMU_CAPS_VIRTIO_IOEVENTFD); + if (strstr(str, "name \"sga\"")) + qemuCapsSet(flags, QEMU_CAPS_SGA); return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0b9c8be..d251262 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -97,6 +97,7 @@ enum qemuCapsFlags { QEMU_CAPS_DEVICE_QXL_VGA = 59, /* Is the primary and vga campatible qxl device named qxl-vga? */ QEMU_CAPS_PCI_MULTIFUNCTION = 60, /* -device multifunction=on|off */ QEMU_CAPS_VIRTIO_IOEVENTFD = 61, /* IOeventFD feature: virtio-{net|blk}-pci.ioeventfd=on/off */ + QEMU_CAPS_SGA = 62, /* Serial Graphics Adapter */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6e4480e..29936a6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3884,6 +3884,11 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-device"); virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s", serial->info.alias, serial->info.alias); + + if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + serial->source.sga == VIR_DOMAIN_CHR_SGA_ON && + qemuCapsGet(qemuCaps, QEMU_CAPS_SGA)) + virCommandAddArgList(cmd, "-device", "sga", NULL); } else { virCommandAddArg(cmd, "-serial"); if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args index 4a6202e..369379f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args @@ -2,5 +2,6 @@ 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 -chardev socket,\ id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,\ id=monitor,mode=readline -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev \ -pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -usb \ +pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 \ +-device sga -usb \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.xml index 57d1b74..31eb7d1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.xml @@ -20,7 +20,7 @@ <address type='drive' controller='0' bus='0' unit='0'/> </disk> <controller type='ide' index='0'/> - <serial type='pty'> + <serial type='pty' sga='on'> <target port='0'/> </serial> <console type='pty'> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ec1f4b5..b4562d1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -425,7 +425,8 @@ mymain(void) DO_TEST("serial-vc-chardev", false, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("serial-pty-chardev", false, - QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_SGA); DO_TEST("serial-dev-chardev", false, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("serial-file-chardev", false, -- 1.7.5.rc3
On 07/05/2011 04:29 AM, Michal Privoznik wrote:
This patch creates new attribute 'sga' for <serial> element. Serial Graphics Adapter allows users to see BIOS messages from the very first moment domain boots up. Therefore, users can choose boot medium, set PXE, etc.
However, to be able to use this, one need SGABIOS, which is accessible here: http://code.google.com/p/sgabios/ --- diff to v1: -move from <video> to <serial> as Dan suggested: https://www.redhat.com/archives/libvir-list/2011-July/msg00134.html
I'll let Dan comment on whether this better fits what he envisioned, but I have some code comments:
+++ b/docs/schemas/domain.rng @@ -1618,11 +1618,21 @@ --> <define name="qemucdev"> <ref name="qemucdevSrcType"/> - <optional> - <attribute name="tty"> - <ref name="absFilePath"/> - </attribute> - </optional> + <interleave> + <optional> + <attribute name="tty"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <attribute name="sga"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute>
Aren't attributes already interleaved in RNG notation? That is, I thought that <interleave> was only necessary for sub-elements. The easiest way to test would be (temporarily) listing 'sga=' prior to 'tty=' and seeing if you still pass the rng validation tests (at that point, it is the rng test that is important; if you fail the tests for round-trip from xml -> parsed -> xml, that's okay, since our parser outputs in fixed order - the real goal of this exercise is to prove that we are liberal in what we accept).
+++ b/src/qemu/qemu_command.c @@ -3884,6 +3884,11 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-device"); virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s", serial->info.alias, serial->info.alias); + + if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + serial->source.sga == VIR_DOMAIN_CHR_SGA_ON && + qemuCapsGet(qemuCaps, QEMU_CAPS_SGA)) + virCommandAddArgList(cmd, "-device", "sga", NULL);
This should issue an error if we lack QEMU_CAPS_SGA but requested VIR_DOMAIN_CHR_SGA_ON, rather than silently ignoring the request. Meanwhile, it is okay to have an explicit VIR_DOMAIN_CHR_SGA_OFF even if we lack the qemu feature. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Jul 05, 2011 at 12:29:43PM +0200, Michal Privoznik wrote:
This patch creates new attribute 'sga' for <serial> element. Serial Graphics Adapter allows users to see BIOS messages from the very first moment domain boots up. Therefore, users can choose boot medium, set PXE, etc.
However, to be able to use this, one need SGABIOS, which is accessible here: http://code.google.com/p/sgabios/ --- diff to v1: -move from <video> to <serial> as Dan suggested: https://www.redhat.com/archives/libvir-list/2011-July/msg00134.html
docs/formatdomain.html.in | 10 ++++-- docs/schemas/domain.rng | 20 +++++++++--- src/conf/domain_conf.c | 34 ++++++++++++++++++- src/conf/domain_conf.h | 10 ++++++ src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 +++ .../qemuxml2argv-serial-pty-chardev.args | 3 +- .../qemuxml2argv-serial-pty-chardev.xml | 2 +- tests/qemuxml2argvtest.c | 3 +- 10 files changed, 78 insertions(+), 13 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fe8d74c..1e9eee5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2040,7 +2040,7 @@ qemu-kvm -net nic,model=? /dev/null <source path='/dev/pts/2'/> <target port='0'/> </parallel> - <serial type='pty'> + <serial type='pty' sga='on'> <source path='/dev/pts/3'/> <target port='0'/> </serial> @@ -2105,7 +2105,7 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <serial type='pty'> + <serial type='pty' sga='on'> <source path='/dev/pts/3'/> <target port='0'/> </serial> @@ -2115,7 +2115,11 @@ qemu-kvm -net nic,model=? /dev/null <p> <code>target</code> can have a <code>port</code> attribute, which specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. + usually 0, 1 or 2 serial ports. The <code>sga</code> attribute enables + or disables Serial Graphics Adapter. Accepted values are <code>on</code> + and <code>off</code>. To be able to use this feature, you need to install + <a href="http://code.google.com/p/sgabios/">SGABios</a>. SGA + <span class="since">Since 0.9.4</span>
This is describing a private implementation detail of QEMU. The libvirt XML should be documented in a more general fashion.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6e4480e..29936a6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3884,6 +3884,11 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-device"); virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s", serial->info.alias, serial->info.alias); + + if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + serial->source.sga == VIR_DOMAIN_CHR_SGA_ON && + qemuCapsGet(qemuCaps, QEMU_CAPS_SGA)) + virCommandAddArgList(cmd, "-device", "sga", NULL); } else { virCommandAddArg(cmd, "-serial"); if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL)))
Looking at this makes me see a flaw in my suggestion to add an attribute to <serial>. The SGA option ROM isn't associated with a particular serial port, nor can you add it multiple times for each serial port. It just uses the "primary" serial port. So I think we might be better off putting it in the same place as the other BIOS options. It is regretable that we have <bootmenu enable='no'/>, when it really should have been <bios bootmenu=yes|no>, to which we could have added other attributes :-( What do people think about adding a new element for this <bios useserial='yes|no'/> ??? Also, agree with Eric that whatever we use in XML, we should raise an error if it is requested in the XML, but not supported in QEMU 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 :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik