[RFC 0/1] qemu: update index for serial device using taget.port

divya posted 1 patch 2 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211124120605.719078-1-divya.garg@nutanix.com
src/conf/domain_conf.c                        | 70 +++++++++++++++++--
src/qemu/qemu_command.c                       | 20 ++++--
tests/qemuhotplugtest.c                       |  1 -
...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 +-
tests/qemuxml2argvdata/name-escape.xml        |  1 +
.../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     |  8 ++-
.../serial-tcp-tlsx509-chardev-notls.xml      | 18 ++++-
.../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 +-
43 files changed, 149 insertions(+), 65 deletions(-)
[RFC 0/1] qemu: update index for serial device using taget.port
Posted by divya 2 years, 5 months ago
Issue
-----
The port being provided in the xml file of the domain is not being used for the
creation of qemu command.

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

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 are multiple reasons

Reasons for not following up
----------------------------
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 is we mention the port
to be 3, then for the next device it not automatically assign 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(bitmap) 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 check:
        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 throws the error.

Libvirt manages the port numbers with the similar logic(auto increment) along
with the above mentioned bug. Hence need to add the above patch along with the
Port allocation logic.

root (1):
  update index for serial device using taget.port

 src/conf/domain_conf.c                        | 70 +++++++++++++++++--
 src/qemu/qemu_command.c                       | 20 ++++--
 tests/qemuhotplugtest.c                       |  1 -
 ...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 +-
 tests/qemuxml2argvdata/name-escape.xml        |  1 +
 .../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     |  8 ++-
 .../serial-tcp-tlsx509-chardev-notls.xml      | 18 ++++-
 .../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 +-
 43 files changed, 149 insertions(+), 65 deletions(-)

-- 
2.27.0