Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On 04/01/22 7:27 pm, Daniel P. Berrangé wrote: On Tue, Jan 04, 2022 at 11:47:39AM +, John Levon wrote: On Sat, Dec 11, 2021 at 07:57:47PM -0800, “Divya wrote: From: Divya Garg VM XML accepts target.port but this does not get passed while building the qemu command line for this VM. Apologies, I failed to notice this had been sent out to the list; Re-posting my comments from an internal thread, so all can see: I don't think serial ports that are not isa-serial should clash with serial ports like this. In fact, from what I can see, as "port" is not used for any other type, we should just completely ignore non-isa-serial ports here. Agreed, conceptually the 'port' is only required to be unique within the scope of the 'type' setting. Sure Daniel. Let me update it and send the patch for the review. Thanks Regards Divya Regards, Daniel
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On Tue, Jan 04, 2022 at 11:47:39AM +, John Levon wrote: > On Sat, Dec 11, 2021 at 07:57:47PM -0800, “Divya wrote: > > > From: Divya Garg > > > > VM XML accepts target.port but this does not get passed while building the > > qemu > > command line for this VM. > > Apologies, I failed to notice this had been sent out to the list; Re-posting > my comments from an internal thread, so all can see: > > I don't think serial ports that are not isa-serial should clash with serial > ports like this. > > In fact, from what I can see, as "port" is not used for any other type, we > should just completely ignore non-isa-serial ports here. Agreed, conceptually the 'port' is only required to be unique within the scope of the 'type' setting. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On 04/01/22 5:56 pm, Ani Sinha wrote: On Tue, 4 Jan 2022, Divya Garg wrote: On 04/01/22 5:47 pm, Ani Sinha wrote: On Mon, 3 Jan 2022, Divya Garg wrote: Thankyou Ani for the review. I will be taking up the comments in next patchset along with other comments. On 03/01/22 1:44 pm, Ani Sinha wrote: On Mon, 3 Jan 2022, Divya Garg wrote: Hi all ! Looking forward for the review comments on the patch. For single patches, no need to use numbering. I have also added the cover letter with the patch. Hence added the numbering here. btw, cover letters are not essential for single patches: https://urldefense.proofpoint.com/v2/url?u=https-3A__libvirt.org_submitting-2Dpatches.html=DwIBAg=s883GpUCOChKOHiocYtGcg=2QGHz-fTCVWImEBKe1ZcSe5t6UfasnhvdzD5DcixwOE=neaBi0ZuC9uGNNnNZGkXsPK_JEReYD2oA8csE4KE4LUYwtOWFxn6ipCd7_mPXNll=u-g3In7hJ90OdEzNmUj41bzzvmAIvOmLJSBpcAB2kQc= Thankyou so much Ani. I added the cover letter because i was not sure if the checks I am applying are needed or not. Hence wanted to explain all my cases that I am handling through the cover letter. yes that's perfectly fine. Thanks Ani for your time.
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On Tue, 4 Jan 2022, Divya Garg wrote: > On 04/01/22 5:47 pm, Ani Sinha wrote: > > > > On Mon, 3 Jan 2022, Divya Garg wrote: > > > > > Thankyou Ani for the review. I will be taking up the comments > > > in next patchset along with other comments. > > > > > > On 03/01/22 1:44 pm, Ani Sinha wrote: > > > > > > > On Mon, 3 Jan 2022, Divya Garg wrote: > > > > > > > > > Hi all ! > > > > > > > > > > Looking forward for the review comments on the patch. > > > > > > > > > For single patches, no need to use numbering. > > > I have also added the cover letter with the patch. Hence added the > > > numbering > > > here. > > btw, cover letters are not essential for single patches: > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__libvirt.org_submitting-2Dpatches.html=DwIBAg=s883GpUCOChKOHiocYtGcg=2QGHz-fTCVWImEBKe1ZcSe5t6UfasnhvdzD5DcixwOE=neaBi0ZuC9uGNNnNZGkXsPK_JEReYD2oA8csE4KE4LUYwtOWFxn6ipCd7_mPXNll=u-g3In7hJ90OdEzNmUj41bzzvmAIvOmLJSBpcAB2kQc= > > Thankyou so much Ani. I added the cover letter because i was not sure if the > checks I am applying are needed or not. Hence wanted to explain all my cases > that I am handling through the cover letter. yes that's perfectly fine.
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On 04/01/22 5:47 pm, Ani Sinha wrote: On Mon, 3 Jan 2022, Divya Garg wrote: Thankyou Ani for the review. I will be taking up the comments in next patchset along with other comments. On 03/01/22 1:44 pm, Ani Sinha wrote: On Mon, 3 Jan 2022, Divya Garg wrote: Hi all ! Looking forward for the review comments on the patch. For single patches, no need to use numbering. I have also added the cover letter with the patch. Hence added the numbering here. btw, cover letters are not essential for single patches: https://urldefense.proofpoint.com/v2/url?u=https-3A__libvirt.org_submitting-2Dpatches.html=DwIBAg=s883GpUCOChKOHiocYtGcg=2QGHz-fTCVWImEBKe1ZcSe5t6UfasnhvdzD5DcixwOE=neaBi0ZuC9uGNNnNZGkXsPK_JEReYD2oA8csE4KE4LUYwtOWFxn6ipCd7_mPXNll=u-g3In7hJ90OdEzNmUj41bzzvmAIvOmLJSBpcAB2kQc= Thankyou so much Ani. I added the cover letter because i was not sure if the checks I am applying are needed or not. Hence wanted to explain all my cases that I am handling through the cover letter.
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On 04/01/22 5:17 pm, John Levon wrote:
On Sat, Dec 11, 2021 at 07:57:47PM -0800, “Divya wrote:
From: Divya Garg
VM XML accepts target.port but this does not get passed while building the qemu
command line for this VM.
Apologies, I failed to notice this had been sent out to the list; Re-posting
my comments from an internal thread, so all can see:
I don't think serial ports that are not isa-serial should clash with serial
ports like this.
In fact, from what I can see, as "port" is not used for any other type, we
should just completely ignore non-isa-serial ports here.
So you can simplify this all to, I think:
for (i = 0; i < isa_serial_count; i++) {
if (type ! = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL):
continue
if (def->serials[i]->port != -1):
// check for clash / too high value
// else mark port used
}
for (i = 0; i < isa_serial_count; i++) {
if (type != VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL):
continue
if (def->serials[i]->port == -1):
// allocate lowest free port
// error if none available
}
regards
john
Thankyou John ! I will be taking up the changes in my next patch.
This patch fixes this bug. In addition, this introduces additional checks in
the port allocation logic for isa-serial devices to :
* Check availability of requested ports
* 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.
Signed-off-by: Divya Garg
---
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(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 107d2a4f5d..e8b19828d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
}
+static int
+virDomainChrIsaSerialDefPostParse(virDomainDef *def)
+{
+size_t i, j;
+size_t isa_serial_count = 0;
+int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
+bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {false};
+
+for (i = 0; i < def->nserials; i++) {
+if (def->serials[i]->targetType ==
+VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
+if (isa_serial_count >= VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Maximum supported number of ISA serial ports is
%d."),
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On Sat, Dec 11, 2021 at 07:57:47PM -0800, “Divya wrote:
> From: Divya Garg
>
> VM XML accepts target.port but this does not get passed while building the
> qemu
> command line for this VM.
Apologies, I failed to notice this had been sent out to the list; Re-posting
my comments from an internal thread, so all can see:
I don't think serial ports that are not isa-serial should clash with serial
ports like this.
In fact, from what I can see, as "port" is not used for any other type, we
should just completely ignore non-isa-serial ports here.
So you can simplify this all to, I think:
for (i = 0; i < isa_serial_count; i++) {
if (type ! = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL):
continue
if (def->serials[i]->port != -1):
// check for clash / too high value
// else mark port used
}
for (i = 0; i < isa_serial_count; i++) {
if (type != VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL):
continue
if (def->serials[i]->port == -1):
// allocate lowest free port
// error if none available
}
regards
john
>
> This patch fixes this bug. In addition, this introduces additional checks in
> the port allocation logic for isa-serial devices to :
> * Check availability of requested ports
> * 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.
>
> Signed-off-by: Divya Garg
> ---
> 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(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 107d2a4f5d..e8b19828d4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
> }
>
>
> +static int
> +virDomainChrIsaSerialDefPostParse(virDomainDef *def)
> +{
> +size_t i, j;
> +size_t isa_serial_count = 0;
> +int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
> +bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {false};
> +
> +for (i = 0; i < def->nserials; i++) {
> +if (def->serials[i]->targetType ==
> +VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
> +if (isa_serial_count >= VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Maximum supported number of ISA serial
> ports is %d."), VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS);
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On Mon, 3 Jan 2022, Divya Garg wrote: > Thankyou Ani for the review. I will be taking up the comments > in next patchset along with other comments. > > On 03/01/22 1:44 pm, Ani Sinha wrote: > > > > > On Mon, 3 Jan 2022, Divya Garg wrote: > > > > > Hi all ! > > > > > > Looking forward for the review comments on the patch. > > > > > For single patches, no need to use numbering. > I have also added the cover letter with the patch. Hence added the numbering > here. btw, cover letters are not essential for single patches: https://libvirt.org/submitting-patches.html
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On Mon, 3 Jan 2022, Divya Garg wrote:
> Thankyou Ani for the review. I will be taking up the comments
> in next patchset along with other comments.
>
> On 03/01/22 1:44 pm, Ani Sinha wrote:
>
> >
> > On Mon, 3 Jan 2022, Divya Garg wrote:
> >
> > > Hi all !
> > >
> > > Looking forward for the review comments on the patch.
> > >
> > For single patches, no need to use numbering.
> I have also added the cover letter with the patch. Hence added the numbering
> here.
OK I did not see the cover letter.
> >
> > > Thanks
> > > Regards
> > > Divya Garg
> > >
> > >
> > > On 12/12/21 9:27 am, “Divya wrote:
> > > > From: Divya Garg
> > > >
> > > > VM XML accepts target.port but this does not get passed while building
> > > > the
> > > > qemu
> > > > command line for this VM.
> > > >
> > > > This patch fixes this bug. In addition,
> > I think we should split up this patch into two, one that fixes the
> > existing issue and the other that introduces additinal checks. It would be
> > easier to review it that way.
> Yeah sounds reasonable. Will do it in the next patch set.
> >
> > this introduces additional checks in
> > > > the port allocation logic for isa-serial devices to :
> > > > * Check availability of requested ports
> > > > * 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.
> > > >
> > > > Signed-off-by: Divya Garg
> > > > ---
> > > >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(-)
> > > >
> > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > > index 107d2a4f5d..e8b19828d4 100644
> > > > --- a/src/conf/domain_conf.c
> > > > +++ b/src/conf/domain_conf.c
> > > > @@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef
> > > > *dev,
> > > >}
> > > > +static int
> > > > +virDomainChrIsaSerialDefPostParse(virDomainDef *def)
> > > > +{
> > > > +size_t i, j;
> > > > +size_t isa_serial_count = 0;
> > > > +int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
> > > > +bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] =
> > > >
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
Thankyou Ani for the review. I will be taking up the comments
in next patchset along with other comments.
On 03/01/22 1:44 pm, Ani Sinha wrote:
On Mon, 3 Jan 2022, Divya Garg wrote:
Hi all !
Looking forward for the review comments on the patch.
For single patches, no need to use numbering.
I have also added the cover letter with the patch. Hence added the
numbering here.
Thanks
Regards
Divya Garg
On 12/12/21 9:27 am, “Divya wrote:
From: Divya Garg
VM XML accepts target.port but this does not get passed while building the
qemu
command line for this VM.
This patch fixes this bug. In addition,
I think we should split up this patch into two, one that fixes the
existing issue and the other that introduces additinal checks. It would be
easier to review it that way.
Yeah sounds reasonable. Will do it in the next patch set.
this introduces additional checks in
the port allocation logic for isa-serial devices to :
* Check availability of requested ports
* 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.
Signed-off-by: Divya Garg
---
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(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 107d2a4f5d..e8b19828d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef
*dev,
}
+static int
+virDomainChrIsaSerialDefPostParse(virDomainDef *def)
+{
+size_t i, j;
+size_t isa_serial_count = 0;
+int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
+bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {false};
+
+for (i = 0; i < def->nserials; i++) {
+if (def->serials[i]->targetType ==
+VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
+if (isa_serial_count >= VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Maximum supported number of ISA serial
ports is %d."), VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS);
Use '%d' , please see
https://urldefense.proofpoint.com/v2/url?u=https-3A__libvirt.org_coding-2Dstyle.html-23error-2Dmessage-2Dformat=DwIDaQ=s883GpUCOChKOHiocYtGcg=2QGHz-fTCVWImEBKe1ZcSe5t6UfasnhvdzD5DcixwOE=KNQ_Qw3y8qxj42KUdx9VcbmzQrwtyRwq2RiDkhMkNFypGybsocys4iwqrICwnIDn=n1ip3VFBqnojUNQYyHUmorNlMSHxKHm-rHK9BfGQ2lQ=
Got it ! Thanks.
+return -1;
+
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
On Mon, 3 Jan 2022, Divya Garg wrote:
> Hi all !
>
> Looking forward for the review comments on the patch.
>
For single patches, no need to use numbering.
> Thanks
> Regards
> Divya Garg
>
>
> On 12/12/21 9:27 am, “Divya wrote:
> > From: Divya Garg
> >
> > VM XML accepts target.port but this does not get passed while building the
> > qemu
> > command line for this VM.
> >
> > This patch fixes this bug. In addition,
I think we should split up this patch into two, one that fixes the
existing issue and the other that introduces additinal checks. It would be
easier to review it that way.
this introduces additional checks in
> > the port allocation logic for isa-serial devices to :
> > * Check availability of requested ports
> > * 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.
> >
> > Signed-off-by: Divya Garg
> > ---
> > 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(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 107d2a4f5d..e8b19828d4 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef
> > *dev,
> > }
> > +static int
> > +virDomainChrIsaSerialDefPostParse(virDomainDef *def)
> > +{
> > +size_t i, j;
> > +size_t isa_serial_count = 0;
> > +int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
> > +bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {false};
> > +
> > +for (i = 0; i < def->nserials; i++) {
> > +if (def->serials[i]->targetType ==
> > +VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
> > +if (isa_serial_count >= VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("Maximum supported number of ISA serial
> > ports is %d."), VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS);
Use '%d' , please see
https://libvirt.org/coding-style.html#error-message-format
> > +return -1;
> > +}
> > +isa_device_index_arr[isa_serial_count++] = i;
> > +}
> > +/* Check for the user defined ports and mark them used if in the
> > range
> > + * of isa-serial ports, .i.e, from 0 to
> > + *
Re: [RFC v2 1/1] qemu: add index for isa-serial device using target.port
Hi all !
Looking forward for the review comments on the patch.
Thanks
Regards
Divya Garg
On 12/12/21 9:27 am, “Divya wrote:
From: Divya Garg
VM XML accepts target.port but this does not get passed while building the qemu
command line for this VM.
This patch fixes this bug. In addition, this introduces additional checks in
the port allocation logic for isa-serial devices to :
* Check availability of requested ports
* 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.
Signed-off-by: Divya Garg
---
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(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 107d2a4f5d..e8b19828d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
}
+static int
+virDomainChrIsaSerialDefPostParse(virDomainDef *def)
+{
+size_t i, j;
+size_t isa_serial_count = 0;
+int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
+bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {false};
+
+for (i = 0; i < def->nserials; i++) {
+if (def->serials[i]->targetType ==
+VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
+if (isa_serial_count >= VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Maximum supported number of ISA serial ports is
%d."), VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS);
+return -1;
+}
+isa_device_index_arr[isa_serial_count++] = i;
+}
+/* Check for the user defined ports and mark them used if in the range
+ * of isa-serial ports, .i.e, from 0 to
+ * VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS.
+ */
+if (def->serials[i]->target.port != -1 &&
+def->serials[i]->target.port < VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS)
{
+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 < isa_serial_count; i++) {
+int isa_index = isa_device_index_arr[i];
+if
[RFC v2 1/1] qemu: add index for isa-serial device using target.port
From: Divya Garg
VM XML accepts target.port but this does not get passed while building the qemu
command line for this VM.
This patch fixes this bug. In addition, this introduces additional checks in
the port allocation logic for isa-serial devices to :
* Check availability of requested ports
* 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.
Signed-off-by: Divya Garg
---
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(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 107d2a4f5d..e8b19828d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5319,6 +5319,61 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
}
+static int
+virDomainChrIsaSerialDefPostParse(virDomainDef *def)
+{
+size_t i, j;
+size_t isa_serial_count = 0;
+int isa_device_index_arr[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {0};
+bool used_serial_port[VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS] = {false};
+
+for (i = 0; i < def->nserials; i++) {
+if (def->serials[i]->targetType ==
+VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
+if (isa_serial_count >= VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Maximum supported number of ISA serial ports
is %d."), VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS);
+return -1;
+}
+isa_device_index_arr[isa_serial_count++] = i;
+}
+/* Check for the user defined ports and mark them used if in the range
+ * of isa-serial ports, .i.e, from 0 to
+ * VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS.
+ */
+if (def->serials[i]->target.port != -1 &&
+def->serials[i]->target.port < VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS)
{
+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 < isa_serial_count; i++) {
+int isa_index = isa_device_index_arr[i];
+if (def->serials[isa_index]->target.port != -1)
+continue;
+for (j = 0; j < VIR_MAX_AVAILABLE_ISA_SERIAL_PORTS; j++) {
+if (!used_serial_port[j]) {
+
