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

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 > >