(I've cc'd a few people who might have opinions on possible
command-line compatibility breakage.)
On Wed, 10 Jan 2024 at 14:38, Bohdan Kostiv <bogdan.kos...@gmail.com> wrote:
>
> Hello,
>
> I have faced an issue in using serial ports when I need to skip a couple of
> ports in the CLI.
>
> For example the ARM machine netduinoplus2 supports up to 7 UARTS.
> Following case works (the first UART is used to send data in the firmware):
> qemu-system-arm -machine netduinoplus2 -nographic -serial mon:stdio -kernel
> path-to-fw/firmware.elf
> But this one doesn't (the third UART is used to send data in the firmware):
> qemu-system-arm -machine netduinoplus2 -nographic -serial none -serial none
> -serial mon:stdio -kernel path-to-fw/firmware.elf
Putting the patch inline for more convenient discussion:
> Subject: [PATCH] Fixed '-serial none' usage breaks following '-serial ...'
> usage
>
> Signed-off-by: Bohdan Kostiv <bohdan.kos...@tii.ae>
> ---
> system/vl.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/system/vl.c b/system/vl.c
> index 2bcd9efb9a..b8744475cd 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1442,8 +1442,11 @@ static int serial_parse(const char *devname)
> int index = num_serial_hds;
> char label[32];
>
> - if (strcmp(devname, "none") == 0)
> + if (strcmp(devname, "none") == 0) {
> + num_serial_hds++;
> return 0;
> + }
> +
> snprintf(label, sizeof(label), "serial%d", index);
> serial_hds = g_renew(Chardev *, serial_hds, index + 1);
>
> --
> 2.39.3 (Apple Git-145)
I agree that it's the right thing to do -- '-serial none
-serial foo' ought to set serial_hds(0) as 'none' and
serial_hds(1) as 'foo'.
My only concern here is that this is a very very
longstanding bug -- as far as I can see it was
introduced in commit 998bbd74b9d81 in 2009. So I am
a little worried that maybe some existing command lines
accidentally rely on the current behaviour.
I think the current behaviour is:
* "-serial none -serial something" is the same as
"-serial something"
* "-serial none" on its own disables the default serial
device (the docs say it will "disable all serial ports"
but I don't think that is correct...)
which amounts to "the only effectively useful use of
'-serial none' is to disable the default serial device"
and if we apply this patch:
* "-serial none -serial something" has the sensible behaviour
of "first serial port not connected/present, second serial
port exists" (which of those you get depends on the machine
model)
* "-serial none" on its own has no behaviour change
So I think the only affected users would be anybody who
accidentally had an extra "-serial none" in their command
line that was previously being overridden by a later
"-serial" option. That doesn't seem very likely to me,
so I think I'd be in favour of making this change and
having something in the release notes about it.
Does anybody on the CC list have a different opinion /
think I've mis-analysed what the current code is doing ?
thanks
-- PMM
