[libvirt] [PATCH v2 3/3] conf: add 'bochs' video display type
Ján Tomko
jtomko at redhat.com
Fri Jul 12 15:06:05 UTC 2019
On Thu, Jul 11, 2019 at 04:05:33PM -0500, Jonathon Jongsma wrote:
>Update schema and configuration to allow specifying new video type of
>'bochs'. Add implementation and tests for qemu.
>
Usually we split the XML addition from the driver impl,
that is:
* conf: schema, documentation, XML parsing and formating and XML test
case
* qemu: qemu_command.c changes, xml2argvtest
Given that the enum checks would require the conf: patch to touch qemu
anyway and the changes are minimal, I will not ask you to split them,
but at least s/conf/qemu/ in the commit summary.
>Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
>---
> docs/formatdomain.html.in | 5 +--
> docs/schemas/domaincommon.rng | 1 +
> src/conf/domain_conf.c | 2 ++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 6 ++++
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_domain_address.c | 3 ++
> .../video-bochs-display-device.args | 32 +++++++++++++++++++
> .../video-bochs-display-device.xml | 29 +++++++++++++++++
> tests/qemuxml2argvtest.c | 3 ++
> 10 files changed, 81 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/video-bochs-display-device.args
> create mode 100644 tests/qemuxml2argvdata/video-bochs-display-device.xml
>
>diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>index a7a6ec32a5..b7db5bbbea 100644
>--- a/docs/formatdomain.html.in
>+++ b/docs/formatdomain.html.in
>@@ -6990,8 +6990,9 @@ qemu-kvm -net nic,model=? /dev/null
> attribute which takes the value "vga", "cirrus", "vmvga", "xen",
> "vbox", "qxl" (<span class="since">since 0.8.6</span>),
> "virtio" (<span class="since">since 1.3.0</span>),
>- "gop" (<span class="since">since 3.2.0</span>), or
>- "none" (<span class="since">since 4.6.0</span>)
>+ "gop" (<span class="since">since 3.2.0</span>),
>+ "none" (<span class="since">since 4.6.0</span>, or "bochs"
>+ (<span class="since">since 5.5.0</span>)
5.6.0 now
> depending on the hypervisor features available.
> The purpose of the type <code>none</code> is to instruct libvirt not
> to add a default video device in the guest (see the paragraph above).
>diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>index 31db599ab9..763480440c 100644
>--- a/docs/schemas/domaincommon.rng
>+++ b/docs/schemas/domaincommon.rng
>@@ -3584,6 +3584,7 @@
> <value>virtio</value>
> <value>gop</value>
> <value>none</value>
>+ <value>bochs</value>
> </choice>
> </attribute>
> <group>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 3323c9a5b1..6e89569d3b 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -738,6 +738,7 @@ VIR_ENUM_IMPL(virDomainVideo,
> "virtio",
> "gop",
> "none",
>+ "bochs",
> );
>
> VIR_ENUM_IMPL(virDomainVideoVGAConf,
>@@ -15158,6 +15159,7 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
> case VIR_DOMAIN_VIDEO_TYPE_VGA:
> case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
> case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
>+ case VIR_DOMAIN_VIDEO_TYPE_BOCHS:
> if (def->virtType == VIR_DOMAIN_VIRT_VBOX)
> return 8 * 1024;
> else if (def->virtType == VIR_DOMAIN_VIRT_VMWARE)
Even though it's only implemented for the QEMU driver, please give it a
separate case. Future readers might be confused why are we assigning a
different default for virt types that don't even support the device.
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index c1b5fc1337..3b248e8091 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -1385,6 +1385,7 @@ typedef enum {
> VIR_DOMAIN_VIDEO_TYPE_VIRTIO,
> VIR_DOMAIN_VIDEO_TYPE_GOP,
> VIR_DOMAIN_VIDEO_TYPE_NONE,
>+ VIR_DOMAIN_VIDEO_TYPE_BOCHS,
>
> VIR_DOMAIN_VIDEO_TYPE_LAST
> } virDomainVideoType;
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 2f98efd6bf..6b0cd7f36d 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -111,6 +111,7 @@ VIR_ENUM_IMPL(qemuVideo,
> "", /* no need for virtio */
> "" /* don't support gop */,
> "" /* 'none' doesn't make sense here */,
>+ "bochs-display",
> );
>
> VIR_ENUM_DECL(qemuDeviceVideo);
>@@ -128,6 +129,7 @@ VIR_ENUM_IMPL(qemuDeviceVideo,
> "virtio-vga",
> "" /* don't support gop */,
> "" /* 'none' doesn't make sense here */,
>+ "bochs-display",
> );
>
> VIR_ENUM_DECL(qemuDeviceVideoSecondary);
>@@ -145,6 +147,7 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary,
> "virtio-gpu",
> "" /* don't support gop */,
> "" /* 'none' doesn't make sense here */,
>+ "" /* no secondary device for bochs */,
> );
>
> VIR_ENUM_DECL(qemuSoundCodec);
>@@ -4754,6 +4757,9 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
> virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM))) {
> if (video->vram)
> virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
>+ } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS) {
>+ if (video->vram)
>+ virBufferAsprintf(&buf, ",vgamem=%uk", video->vram);
> }
>
> if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 0f1fda2384..00ac5e1307 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -4960,6 +4960,7 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video)
> case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
> case VIR_DOMAIN_VIDEO_TYPE_QXL:
> case VIR_DOMAIN_VIDEO_TYPE_VIRTIO:
>+ case VIR_DOMAIN_VIDEO_TYPE_BOCHS:
> case VIR_DOMAIN_VIDEO_TYPE_LAST:
> break;
> }
>diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>index 4b99e8ca93..972b810211 100644
>--- a/src/qemu/qemu_domain_address.c
>+++ b/src/qemu/qemu_domain_address.c
>@@ -931,6 +931,9 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
> return pciFlags;
>
>+ case VIR_DOMAIN_VIDEO_TYPE_BOCHS:
>+ return pcieFlags;
>+
> case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
> case VIR_DOMAIN_VIDEO_TYPE_GOP:
> case VIR_DOMAIN_VIDEO_TYPE_NONE:
>diff --git a/tests/qemuxml2argvdata/video-bochs-display-device.args b/tests/qemuxml2argvdata/video-bochs-display-device.args
>new file mode 100644
>index 0000000000..f88e9ccb04
>--- /dev/null
>+++ b/tests/qemuxml2argvdata/video-bochs-display-device.args
>@@ -0,0 +1,32 @@
>+LC_ALL=C \
>+PATH=/bin \
>+HOME=/tmp/lib/domain--1-QEMUGuest1 \
>+USER=test \
>+LOGNAME=test \
>+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
>+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
>+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
>+QEMU_AUDIO_DRV=none \
>+/usr/bin/qemu-system-i686 \
>+-name QEMUGuest1 \
>+-S \
>+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
>+-m 1024 \
>+-realtime mlock=off \
>+-smp 1,sockets=1,cores=1,threads=1 \
>+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
>+-display none \
>+-no-user-config \
>+-nodefaults \
>+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
>+server,nowait \
>+-mon chardev=charmonitor,id=monitor,mode=control \
>+-rtc base=utc \
>+-no-shutdown \
>+-no-acpi \
>+-usb \
>+-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
>+id=drive-ide0-0-0,cache=none \
>+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
>+-device bochs-display,id=video0,vgamem=16384k,bus=pci.0,addr=0x2 \
>+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
>diff --git a/tests/qemuxml2argvdata/video-bochs-display-device.xml b/tests/qemuxml2argvdata/video-bochs-display-device.xml
>new file mode 100644
>index 0000000000..3c59bf7c6a
>--- /dev/null
>+++ b/tests/qemuxml2argvdata/video-bochs-display-device.xml
>@@ -0,0 +1,29 @@
>+<domain type='qemu'>
>+ <name>QEMUGuest1</name>
>+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>+ <memory unit='KiB'>1048576</memory>
>+ <currentMemory unit='KiB'>1048576</currentMemory>
>+ <vcpu>1</vcpu>
>+ <os>
>+ <type arch='i686' machine='pc'>hvm</type>
>+ <boot dev='hd'/>
>+ </os>
>+ <clock offset='utc'/>
>+ <on_poweroff>destroy</on_poweroff>
>+ <on_reboot>restart</on_reboot>
>+ <on_crash>destroy</on_crash>
>+ <devices>
>+ <emulator>/usr/bin/qemu-system-i686</emulator>
>+ <disk type='file' device='disk'>
>+ <driver name='qemu' type='qcow2' cache='none'/>
>+ <source file='/var/lib/libvirt/images/QEMUGuest1'/>
>+ <target dev='hda' bus='ide'/>
>+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>+ </disk>
>+ <controller type='ide' index='0'/>
>+ <video>
>+ <model type='bochs' vram='16384' heads='1'/>
>+ </video>
>+ <memballoon model='virtio'/>
>+ </devices>
>+</domain>
>diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>index 0ac128be00..92791f61ac 100644
>--- a/tests/qemuxml2argvtest.c
>+++ b/tests/qemuxml2argvtest.c
>@@ -2028,6 +2028,9 @@ mymain(void)
> QEMU_CAPS_DEVICE_VIRTIO_VGA,
> QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS);
>+ DO_TEST("video-bochs-display-device",
>+ QEMU_CAPS_DEVICE_BOCHS_DISPLAY,
>+ QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
Manually listing the capabilities makes it easy to forget one and it
will not catch the case when a newly introduced capability breaks the
old code. Please use DO_TEST_CAPS_LATEST
Jano
> DO_TEST("video-none-device",
> QEMU_CAPS_VNC);
> DO_TEST_PARSE_ERROR("video-invalid-multiple-devices", NONE);
>--
>2.20.1
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190712/5eecbaed/attachment-0001.sig>
More information about the libvir-list
mailing list