| Message ID | 20240122163607.459769-2-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | system: Fix handling of '-serial none -serial something' | expand |
On Mon, Jan 22, 2024 at 04:36:06PM +0000, Peter Maydell wrote: > Currently if the user passes multiple -serial options on the command > line, we mostly treat those as applying to the different serial > devices in order, so that for example > -serial stdio -serial file:filename > will connect the first serial port to stdio and the second to the > named file. > > The exception to this is the '-serial none' serial device type. This > means "don't allocate this serial device", but a bug means that > following -serial options are not correctly handled, so that > -serial none -serial stdio > has the unexpected effect that stdio is connected to the first serial > port, not the second. > > This is a very long-standing bug that dates back at least as far as > commit 998bbd74b9d81 from 2009. > > Make the 'none' serial type move forward in the indexing of serial > devices like all the other serial types, so that any subsequent > -serial options are correctly handled. > > Note that if your commandline mistakenly had a '-serial none' that > was being overridden by a following '-serial something' option, you > should delete the unnecessary '-serial none'. This will give you the > same behaviour as before, on QEMU versions both with and without this > bug fix. > > Cc: qemu-stable@nongnu.org > Reported-by: Bohdan Kostiv <bohdan.kostiv@tii.ae> > Fixes: 998bbd74b9d81 ("default devices: core code & serial lines") > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > See the discussion of Bohdan's patch on the mailing list for > further context: > https://lore.kernel.org/qemu-devel/CAA3Sv1LQ8yDUNLCB5WqLVZjsHffrU0uSbL_YYJW_m+Db2PhEeQ@mail.gmail.com/ > --- > system/vl.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel
On 1/23/24 02:36, Peter Maydell wrote: > Currently if the user passes multiple -serial options on the command > line, we mostly treat those as applying to the different serial > devices in order, so that for example > -serial stdio -serialfile:filename > will connect the first serial port to stdio and the second to the > named file. > > The exception to this is the '-serial none' serial device type. This > means "don't allocate this serial device", but a bug means that > following -serial options are not correctly handled, so that > -serial none -serial stdio > has the unexpected effect that stdio is connected to the first serial > port, not the second. > > This is a very long-standing bug that dates back at least as far as > commit 998bbd74b9d81 from 2009. > > Make the 'none' serial type move forward in the indexing of serial > devices like all the other serial types, so that any subsequent > -serial options are correctly handled. > > Note that if your commandline mistakenly had a '-serial none' that > was being overridden by a following '-serial something' option, you > should delete the unnecessary '-serial none'. This will give you the > same behaviour as before, on QEMU versions both with and without this > bug fix. > > Cc:qemu-stable@nongnu.org > Reported-by: Bohdan Kostiv<bohdan.kostiv@tii.ae> > Fixes: 998bbd74b9d81 ("default devices: core code & serial lines") > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > See the discussion of Bohdan's patch on the mailing list for > further context: > https://lore.kernel.org/qemu-devel/CAA3Sv1LQ8yDUNLCB5WqLVZjsHffrU0uSbL_YYJW_m+Db2PhEeQ@mail.gmail.com/ > --- > system/vl.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/system/vl.c b/system/vl.c index 788d88ea03a..aa7953bdd7d 100644 --- a/system/vl.c +++ b/system/vl.c @@ -1439,18 +1439,22 @@ static void qemu_create_default_devices(void) static int serial_parse(const char *devname) { int index = num_serial_hds; - char label[32]; - if (strcmp(devname, "none") == 0) - return 0; - snprintf(label, sizeof(label), "serial%d", index); serial_hds = g_renew(Chardev *, serial_hds, index + 1); - serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL); - if (!serial_hds[index]) { - error_report("could not connect serial device" - " to character backend '%s'", devname); - return -1; + if (strcmp(devname, "none") == 0) { + /* Don't allocate a serial device for this index */ + serial_hds[index] = NULL; + } else { + char label[32]; + snprintf(label, sizeof(label), "serial%d", index); + + serial_hds[index] = qemu_chr_new_mux_mon(label, devname, NULL); + if (!serial_hds[index]) { + error_report("could not connect serial device" + " to character backend '%s'", devname); + return -1; + } } num_serial_hds++; return 0;
Currently if the user passes multiple -serial options on the command line, we mostly treat those as applying to the different serial devices in order, so that for example -serial stdio -serial file:filename will connect the first serial port to stdio and the second to the named file. The exception to this is the '-serial none' serial device type. This means "don't allocate this serial device", but a bug means that following -serial options are not correctly handled, so that -serial none -serial stdio has the unexpected effect that stdio is connected to the first serial port, not the second. This is a very long-standing bug that dates back at least as far as commit 998bbd74b9d81 from 2009. Make the 'none' serial type move forward in the indexing of serial devices like all the other serial types, so that any subsequent -serial options are correctly handled. Note that if your commandline mistakenly had a '-serial none' that was being overridden by a following '-serial something' option, you should delete the unnecessary '-serial none'. This will give you the same behaviour as before, on QEMU versions both with and without this bug fix. Cc: qemu-stable@nongnu.org Reported-by: Bohdan Kostiv <bohdan.kostiv@tii.ae> Fixes: 998bbd74b9d81 ("default devices: core code & serial lines") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- See the discussion of Bohdan's patch on the mailing list for further context: https://lore.kernel.org/qemu-devel/CAA3Sv1LQ8yDUNLCB5WqLVZjsHffrU0uSbL_YYJW_m+Db2PhEeQ@mail.gmail.com/ --- system/vl.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)