| Message ID | 20250115163542.291424-8-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/7] hw/xen: Add xs_node_read() helper function | expand |
On Wed, Jan 15, 2025 at 04:27:25PM +0000, David Woodhouse wrote: > diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c > index e61902461b..9e7f6da343 100644 > --- a/hw/char/xen_console.c > +++ b/hw/char/xen_console.c > @@ -581,19 +581,27 @@ static void xen_console_device_create(XenBackendInstance *backend, > output); > goto fail; > } > - } else if (number) { > - cd = serial_hd(number); > - if (!cd) { > - error_prepend(errp, "console: No serial device #%ld found: ", > - number); > - goto fail; > - } > + } else if (errno != ENOENT) { > + error_prepend(errp, "console: No valid chardev found: "); > + goto fail; > } else { > - /* No 'output' node on primary console: use null. */ > - cd = qemu_chr_new(label, "null", NULL); > - if (!cd) { > - error_setg(errp, "console: failed to create null device"); > - goto fail; > + if (errp) { I don't think you need this check, with ERRP_GUARD() macro `errp` is never NULL. > + error_free(*errp); After this, I think you still need *errp = NULL; > + } > + if (number) { > + cd = serial_hd(number); > + if (!cd) { > + error_setg(errp, "console: No serial device #%ld found: ", That error message doesn't need the ": " at the end anymore. With those fixed: Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Cheers,
On Wed, 2025-01-15 at 17:49 +0100, Anthony PERARD wrote: > > With those fixed: Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> Thanks. I've pushed those changes and am watching the pipeline at https://gitlab.com/dwmw2/qemu/-/pipelines/1626804720 I'll probably send a pull request tomorrow. The patch now looks like this: From 8b44a3e39f36540818d99ef8cf79e64bba1ed9c3 Mon Sep 17 00:00:00 2001 From: David Woodhouse <dwmw@amazon.co.uk> Date: Wed, 15 Jan 2025 15:46:06 +0000 Subject: [PATCH] hw/xen: Fix errp handling in xen_console When attempting to read the 'output' node, interpret any error *other* than ENOENT as a fatal error. For ENOENT, fall back to serial_hd() to find a character device, or create a null device. Do not attempt to prepend to errp when serial_hd() fails; the error isn't relevant (and prior to this change, wasn't set anyway). Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech> --- hw/char/xen_console.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c index e61902461b..d03c188d1d 100644 --- a/hw/char/xen_console.c +++ b/hw/char/xen_console.c @@ -569,7 +569,7 @@ static void xen_console_device_create(XenBackendInstance *backend, snprintf(label, sizeof(label), "xencons%ld", number); - output = xs_node_read(xsh, XBT_NULL, NULL, NULL, "%s/%s", fe, "output"); + output = xs_node_read(xsh, XBT_NULL, NULL, errp, "%s/%s", fe, "output"); if (output) { /* * FIXME: sure we want to support implicit @@ -581,19 +581,27 @@ static void xen_console_device_create(XenBackendInstance *backend, output); goto fail; } - } else if (number) { - cd = serial_hd(number); - if (!cd) { - error_prepend(errp, "console: No serial device #%ld found: ", - number); - goto fail; - } + } else if (errno != ENOENT) { + error_prepend(errp, "console: No valid chardev found: "); + goto fail; } else { - /* No 'output' node on primary console: use null. */ - cd = qemu_chr_new(label, "null", NULL); - if (!cd) { - error_setg(errp, "console: failed to create null device"); - goto fail; + error_free(*errp); + *errp = NULL; + + if (number) { + cd = serial_hd(number); + if (!cd) { + error_setg(errp, "console: No serial device #%ld found", + number); + goto fail; + } + } else { + /* No 'output' node on primary console: use null. */ + cd = qemu_chr_new(label, "null", NULL); + if (!cd) { + error_setg(errp, "console: failed to create null device"); + goto fail; + } } }
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c index e61902461b..9e7f6da343 100644 --- a/hw/char/xen_console.c +++ b/hw/char/xen_console.c @@ -569,7 +569,7 @@ static void xen_console_device_create(XenBackendInstance *backend, snprintf(label, sizeof(label), "xencons%ld", number); - output = xs_node_read(xsh, XBT_NULL, NULL, NULL, "%s/%s", fe, "output"); + output = xs_node_read(xsh, XBT_NULL, NULL, errp, "%s/%s", fe, "output"); if (output) { /* * FIXME: sure we want to support implicit @@ -581,19 +581,27 @@ static void xen_console_device_create(XenBackendInstance *backend, output); goto fail; } - } else if (number) { - cd = serial_hd(number); - if (!cd) { - error_prepend(errp, "console: No serial device #%ld found: ", - number); - goto fail; - } + } else if (errno != ENOENT) { + error_prepend(errp, "console: No valid chardev found: "); + goto fail; } else { - /* No 'output' node on primary console: use null. */ - cd = qemu_chr_new(label, "null", NULL); - if (!cd) { - error_setg(errp, "console: failed to create null device"); - goto fail; + if (errp) { + error_free(*errp); + } + if (number) { + cd = serial_hd(number); + if (!cd) { + error_setg(errp, "console: No serial device #%ld found: ", + number); + goto fail; + } + } else { + /* No 'output' node on primary console: use null. */ + cd = qemu_chr_new(label, "null", NULL); + if (!cd) { + error_setg(errp, "console: failed to create null device"); + goto fail; + } } }