[RFC v3 1/2] Add the port allocation logic for isa-serial devices.

Ani Sinha ani at anisinha.ca
Tue Jan 11 10:39:06 UTC 2022


Please add SOB.

On Mon, 10 Jan 2022, Divya Garg wrote:

> This commit takes care of following cases:
> -> Check availability of requested ports.
>   ->The total number of requested ports should not be more than
>     VIR_MAX_ISA_SERIAL_PORTS.
>   ->The ports requested should be less than VIR_MAX_ISA_SERIAL_PORTS.
>   ->VIR_MAX_ISA_SERIAL_PORTS should correspond to MAX_ISA_SERIAL_PORTS
>     specified in qemu code commit def337ffda34d331404bd7f1a42726b71500df22.
> -> Prevent duplicate device assignments to the same port.
> -> In case no ports are provided in the XML, this patch scans the list of unused
>    isa-serial indices to automatically assign available ports for this VM.
> ---
>  src/conf/domain_conf.c                        | 61 ++++++++++++++++---
>  src/conf/domain_conf.h                        |  6 ++
>  ...g-console-compat-2-live+console-virtio.xml |  4 +-
>  .../qemuhotplug-console-compat-2-live.xml     |  4 +-
>  .../serial-tcp-tlsx509-chardev-notls.xml      |  2 +-
>  .../serial-tcp-tlsx509-chardev-verify.xml     |  2 +-
>  .../serial-tcp-tlsx509-chardev.xml            |  2 +-
>  .../serial-tcp-tlsx509-secret-chardev.xml     |  2 +-
>  .../serial-tcp-tlsx509-chardev.xml            |  2 +-
>  9 files changed, 68 insertions(+), 17 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5691b8d2d5..e468e98045 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5330,6 +5330,56 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
>  }
>
>
> +static int
> +virDomainChrIsaSerialDefPostParse(virDomainDef *def)
> +{
> +    size_t i, j;
> +    size_t isa_serial_count = 0;
> +    bool used_serial_port[VIR_MAX_ISA_SERIAL_PORTS] = {false};
> +
> +    /* Perform all the required checks. */
> +    for (i = 0; i < def->nserials; i++) {
> +
> +        if (def->serials[i]->targetType !=
> +            VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL)
> +            continue;
> +
> +        if (isa_serial_count++ >= VIR_MAX_ISA_SERIAL_PORTS ||
> +            def->serials[i]->target.port >= VIR_MAX_ISA_SERIAL_PORTS) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Maximum supported number of ISA serial ports is '%d'."), VIR_MAX_ISA_SERIAL_PORTS);
> +            return -1;
> +        }
> +
> +        if (def->serials[i]->target.port != -1) {
> +            if (used_serial_port[def->serials[i]->target.port]) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("target port '%d' already allocated."), def->serials[i]->target.port);
> +                return -1;
> +            }
> +            used_serial_port[def->serials[i]->target.port] = true;
> +        }
> +    }
> +
> +    /* Assign the ports to the devices. */
> +    for (i = 0; i < def->nserials; i++) {
> +
> +        if (def->serials[i]->targetType !=
> +            VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL ||
> +            def->serials[i]->target.port != -1)
> +            continue;
> +
> +        for (j = 0; j < VIR_MAX_ISA_SERIAL_PORTS; j++) {
> +            if (!used_serial_port[j]) {
> +                def->serials[i]->target.port = j;
> +                used_serial_port[j] = true;
> +                break;
> +            }
> +        }
> +    }
> +    return 0;
> +}
> +
>  static void
>  virDomainChrDefPostParse(virDomainChrDef *chr,
>                           const virDomainDef *def)
> @@ -6197,6 +6247,9 @@ virDomainDefPostParse(virDomainDef *def,
>              goto cleanup;
>      }
>
> +    if (virDomainChrIsaSerialDefPostParse(def) < 0)
> +            return -1;
> +
>      /* iterate the devices */
>      ret = virDomainDeviceInfoIterateFlags(def,
>                                            virDomainDefPostParseDeviceIterator,
> @@ -19929,14 +19982,6 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
>          if (!chr)
>              return NULL;
>
> -        if (chr->target.port == -1) {
> -            int maxport = -1;
> -            for (j = 0; j < i; j++) {
> -                if (def->serials[j]->target.port > maxport)
> -                    maxport = def->serials[j]->target.port;
> -            }
> -            chr->target.port = maxport + 1;
> -        }
>          def->serials[def->nserials++] = chr;
>      }
>      VIR_FREE(nodes);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 144ba4dd12..a8f41dc8c2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1187,6 +1187,12 @@ typedef enum {
>      VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST
>  } virDomainChrConsoleTargetType;
>
> +/*
> + * The value of VIR_MAX_ISA_SERIAL_PORTS corresponds to MAX_ISA_SERIAL_PORTS
> + * set in qemu code base.
> + */
> +#define VIR_MAX_ISA_SERIAL_PORTS 4
> +
>  typedef enum {
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NONE = 0,
>      VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL,
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml
> index 5d688e7748..295d75b768 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml
> @@ -74,7 +74,7 @@
>        <alias name='serial0'/>
>      </serial>
>      <serial type='pty'>
> -      <target type='isa-serial' port='0'>
> +      <target type='isa-serial' port='1'>
>          <model name='isa-serial'/>
>        </target>
>        <alias name='serial1'/>
> @@ -82,7 +82,7 @@
>      <serial type='tcp'>
>        <source mode='bind' host='0.0.0.0' service='2445'/>
>        <protocol type='raw'/>
> -      <target type='isa-serial' port='1'>
> +      <target type='isa-serial' port='2'>
>          <model name='isa-serial'/>
>        </target>
>        <alias name='serial2'/>
> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml
> index b916f30412..850ebddf52 100644
> --- a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live.xml
> @@ -74,7 +74,7 @@
>        <alias name='serial0'/>
>      </serial>
>      <serial type='pty'>
> -      <target type='isa-serial' port='0'>
> +      <target type='isa-serial' port='1'>
>          <model name='isa-serial'/>
>        </target>
>        <alias name='serial1'/>
> @@ -82,7 +82,7 @@
>      <serial type='tcp'>
>        <source mode='bind' host='0.0.0.0' service='2445'/>
>        <protocol type='raw'/>
> -      <target type='isa-serial' port='1'>
> +      <target type='isa-serial' port='2'>
>          <model name='isa-serial'/>
>        </target>
>        <alias name='serial2'/>
> diff --git a/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-notls.xml b/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-notls.xml
> index 9b0b8b3e73..522f0184d3 100644
> --- a/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-notls.xml
> +++ b/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-notls.xml
> @@ -37,7 +37,7 @@
>      <serial type='tcp'>
>        <source mode='connect' host='127.0.0.1' service='5555' tls='no'/>
>        <protocol type='raw'/>
> -      <target type='isa-serial' port='0'>
> +      <target type='isa-serial' port='1'>
>          <model name='isa-serial'/>
>        </target>
>      </serial>
> diff --git a/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-verify.xml b/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-verify.xml
> index 75c6c7f5f4..d41f5ee03e 100644
> --- a/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-verify.xml
> +++ b/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev-verify.xml
> @@ -29,7 +29,7 @@
>      <serial type='tcp'>
>        <source mode='connect' host='127.0.0.1' service='5555'/>
>        <protocol type='raw'/>
> -      <target port='0'/>
> +      <target port='1'/>
>      </serial>
>      <console type='udp'>
>        <source mode='bind' host='127.0.0.1' service='1111'/>
> diff --git a/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev.xml b/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev.xml
> index 75c6c7f5f4..d41f5ee03e 100644
> --- a/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev.xml
> +++ b/tests/qemuxml2argvdata/serial-tcp-tlsx509-chardev.xml
> @@ -29,7 +29,7 @@
>      <serial type='tcp'>
>        <source mode='connect' host='127.0.0.1' service='5555'/>
>        <protocol type='raw'/>
> -      <target port='0'/>
> +      <target port='1'/>
>      </serial>
>      <console type='udp'>
>        <source mode='bind' host='127.0.0.1' service='1111'/>
> diff --git a/tests/qemuxml2argvdata/serial-tcp-tlsx509-secret-chardev.xml b/tests/qemuxml2argvdata/serial-tcp-tlsx509-secret-chardev.xml
> index 670f282b84..2bf954bc9f 100644
> --- a/tests/qemuxml2argvdata/serial-tcp-tlsx509-secret-chardev.xml
> +++ b/tests/qemuxml2argvdata/serial-tcp-tlsx509-secret-chardev.xml
> @@ -34,7 +34,7 @@
>      <serial type='tcp'>
>        <source mode='connect' host='127.0.0.1' service='5555'/>
>        <protocol type='raw'/>
> -      <target port='0'/>
> +      <target port='1'/>
>      </serial>
>      <console type='udp'>
>        <source mode='bind' host='127.0.0.1' service='1111'/>
> diff --git a/tests/qemuxml2xmloutdata/serial-tcp-tlsx509-chardev.xml b/tests/qemuxml2xmloutdata/serial-tcp-tlsx509-chardev.xml
> index 7fde19f283..de11465233 100644
> --- a/tests/qemuxml2xmloutdata/serial-tcp-tlsx509-chardev.xml
> +++ b/tests/qemuxml2xmloutdata/serial-tcp-tlsx509-chardev.xml
> @@ -37,7 +37,7 @@
>      <serial type='tcp'>
>        <source mode='connect' host='127.0.0.1' service='5555'/>
>        <protocol type='raw'/>
> -      <target type='isa-serial' port='0'>
> +      <target type='isa-serial' port='1'>
>          <model name='isa-serial'/>
>        </target>
>      </serial>
> --
> 2.25.1
>
>




More information about the libvir-list mailing list