[RFC v2 0/1] qemu: add index for isa-serial device using target.port

“Divya posted 1 patch 2 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211212035746.282546-1-divya.garg@nutanix.com
src/conf/domain_conf.c                        | 66 ++++++++++++++++---
src/conf/domain_conf.h                        |  1 +
src/qemu/qemu_command.c                       | 21 ++++--
...g-console-compat-2-live+console-virtio.xml |  4 +-
.../qemuhotplug-console-compat-2-live.xml     |  4 +-
tests/qemuxml2argvdata/bios.args              |  2 +-
.../qemuxml2argvdata/console-compat-auto.args |  2 +-
.../console-compat-chardev.args               |  2 +-
tests/qemuxml2argvdata/console-compat.args    |  2 +-
.../qemuxml2argvdata/console-virtio-many.args |  2 +-
tests/qemuxml2argvdata/controller-order.args  |  2 +-
.../name-escape.x86_64-2.11.0.args            |  4 +-
.../q35-virt-manager-basic.args               |  2 +-
.../serial-dev-chardev-iobase.args            |  2 +-
.../qemuxml2argvdata/serial-dev-chardev.args  |  2 +-
.../qemuxml2argvdata/serial-file-chardev.args |  2 +-
tests/qemuxml2argvdata/serial-file-log.args   |  2 +-
.../qemuxml2argvdata/serial-many-chardev.args |  4 +-
.../qemuxml2argvdata/serial-pty-chardev.args  |  2 +-
tests/qemuxml2argvdata/serial-spiceport.args  |  2 +-
.../qemuxml2argvdata/serial-tcp-chardev.args  |  2 +-
.../serial-tcp-telnet-chardev.args            |  2 +-
.../serial-tcp-tlsx509-chardev-notls.args     |  4 +-
.../serial-tcp-tlsx509-chardev-notls.xml      |  2 +-
.../serial-tcp-tlsx509-chardev-verify.args    |  4 +-
.../serial-tcp-tlsx509-chardev-verify.xml     |  2 +-
.../serial-tcp-tlsx509-chardev.args           |  4 +-
.../serial-tcp-tlsx509-chardev.xml            |  2 +-
.../serial-tcp-tlsx509-secret-chardev.args    |  4 +-
.../serial-tcp-tlsx509-secret-chardev.xml     |  2 +-
.../qemuxml2argvdata/serial-udp-chardev.args  |  4 +-
.../qemuxml2argvdata/serial-unix-chardev.args |  4 +-
.../serial-unix-chardev.x86_64-latest.args    |  4 +-
tests/qemuxml2argvdata/serial-vc-chardev.args |  2 +-
tests/qemuxml2argvdata/user-aliases.args      |  4 +-
.../virtio-9p-createmode.x86_64-latest.args   |  2 +-
.../virtio-9p-multidevs.x86_64-latest.args    |  2 +-
.../x86_64-pc-graphics.x86_64-latest.args     |  2 +-
.../x86_64-pc-headless.x86_64-latest.args     |  2 +-
.../x86_64-q35-graphics.x86_64-latest.args    |  2 +-
.../x86_64-q35-headless.x86_64-latest.args    |  2 +-
.../serial-tcp-tlsx509-chardev.xml            |  2 +-
42 files changed, 126 insertions(+), 64 deletions(-)
[RFC v2 0/1] qemu: add index for isa-serial device using target.port
Posted by “Divya 2 years, 4 months ago
From: Divya Garg<divya.garg@nutanix.com>

Issue
-----
The port being provided in the xml file of the domain is not getting reflected
in the generated qemu command

For instance, on adding the serial device:

<serial>
<target type='serial' port='3'/>
</serial>

Generated qemu command will look like :
/usr/libexec/qemu-kvm ...\
    -device isa-serial,chardev=charserial0,id=serial0

Actually it should be :
/usr/libexec/qemu-kvm ...\
    -device isa-serial,chardev=charserial0,id=serial0,index=3

So that qemu understands which port needs to be allocated for the device.

Patch
-----
Out already for the correction :
https://listman.redhat.com/archives/libvir-list/2018-April/msg02302.html

This patch was not followed up. According to me there can be multiple reasons.
I have mentioned them below:
---------------------------------------------------------------------------
Index : specifies the index number of a connector port. If not specified, the
index is automatically incremented. This logic exists both on qemu as well as
libvirt.
https://github.com/qemu/qemu/blob/master/hw/char/serial-isa.c#L62

Issue 1:

If we want two isa-serial devices and for the first one if we mention the port
to be 3, then for the next device it automatically assigns the port number 4,
which will throw the following error :
error: internal error: process exited while connecting to monitor:
2021-11-12T11:05:31.169987Z qemu-kvm: -device
isa-serial,chardev=charserial2,id=serial2,index=5: Max. supported number of ISA
serial ports is 4.

But we are left with 3 ports (0,1,2) which are unused. So ideally we should
have used them.

Issue 2:

It is possible that two devices get the same port address which might lead to a
lot of ambiguity. Example: we want two devices and for the second one we
provide the index 0. Then from default logic the first device will be allotted
port 0 and the second device will overwrite it and get port 0.

Solution :
----------
Port allocation logic

1. Precedence should be given to serial devices as we only have the first 4
ports for them.
    1.1. Check the command line/xml file, scan for all the devices mentioned
    and then start with the isa-serial devices for port allocation.
2.Maintain a buffer for marking the allocated ports.
3.While assigning a port to the device
    3.1. If no port is provided by the user : provide the next available port.
    3.2. Else:
        3.2.1. If the port is already allocated : throw the error.
        3.2.2. Else allocate the port.
    3.3. If out of ports : throw error -> qemu also throws the error.

Libvirt manages the port numbers with the auto increment logic but never
updates the index value using the port number. Hence using the above mentioned
port allocation logic to assign the port for isa-serial devices and fill the
index value using the same.

Divya Garg(1):
  qemu: add index for isa-serial device using target.port

 src/conf/domain_conf.c                        | 66 ++++++++++++++++---
 src/conf/domain_conf.h                        |  1 +
 src/qemu/qemu_command.c                       | 21 ++++--
 ...g-console-compat-2-live+console-virtio.xml |  4 +-
 .../qemuhotplug-console-compat-2-live.xml     |  4 +-
 tests/qemuxml2argvdata/bios.args              |  2 +-
 .../qemuxml2argvdata/console-compat-auto.args |  2 +-
 .../console-compat-chardev.args               |  2 +-
 tests/qemuxml2argvdata/console-compat.args    |  2 +-
 .../qemuxml2argvdata/console-virtio-many.args |  2 +-
 tests/qemuxml2argvdata/controller-order.args  |  2 +-
 .../name-escape.x86_64-2.11.0.args            |  4 +-
 .../q35-virt-manager-basic.args               |  2 +-
 .../serial-dev-chardev-iobase.args            |  2 +-
 .../qemuxml2argvdata/serial-dev-chardev.args  |  2 +-
 .../qemuxml2argvdata/serial-file-chardev.args |  2 +-
 tests/qemuxml2argvdata/serial-file-log.args   |  2 +-
 .../qemuxml2argvdata/serial-many-chardev.args |  4 +-
 .../qemuxml2argvdata/serial-pty-chardev.args  |  2 +-
 tests/qemuxml2argvdata/serial-spiceport.args  |  2 +-
 .../qemuxml2argvdata/serial-tcp-chardev.args  |  2 +-
 .../serial-tcp-telnet-chardev.args            |  2 +-
 .../serial-tcp-tlsx509-chardev-notls.args     |  4 +-
 .../serial-tcp-tlsx509-chardev-notls.xml      |  2 +-
 .../serial-tcp-tlsx509-chardev-verify.args    |  4 +-
 .../serial-tcp-tlsx509-chardev-verify.xml     |  2 +-
 .../serial-tcp-tlsx509-chardev.args           |  4 +-
 .../serial-tcp-tlsx509-chardev.xml            |  2 +-
 .../serial-tcp-tlsx509-secret-chardev.args    |  4 +-
 .../serial-tcp-tlsx509-secret-chardev.xml     |  2 +-
 .../qemuxml2argvdata/serial-udp-chardev.args  |  4 +-
 .../qemuxml2argvdata/serial-unix-chardev.args |  4 +-
 .../serial-unix-chardev.x86_64-latest.args    |  4 +-
 tests/qemuxml2argvdata/serial-vc-chardev.args |  2 +-
 tests/qemuxml2argvdata/user-aliases.args      |  4 +-
 .../virtio-9p-createmode.x86_64-latest.args   |  2 +-
 .../virtio-9p-multidevs.x86_64-latest.args    |  2 +-
 .../x86_64-pc-graphics.x86_64-latest.args     |  2 +-
 .../x86_64-pc-headless.x86_64-latest.args     |  2 +-
 .../x86_64-q35-graphics.x86_64-latest.args    |  2 +-
 .../x86_64-q35-headless.x86_64-latest.args    |  2 +-
 .../serial-tcp-tlsx509-chardev.xml            |  2 +-
 42 files changed, 126 insertions(+), 64 deletions(-)

-- 
2.27.0