* [PATCH for-8.2 0/3] UI: fix default VC regressions
@ 2023-11-17 14:35 marcandre.lureau
2023-11-17 14:35 ` [PATCH for-8.2 1/3] vl: revert behaviour for -display none marcandre.lureau
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: marcandre.lureau @ 2023-11-17 14:35 UTC (permalink / raw
To: qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Marc-André Lureau,
Paolo Bonzini, dwmw
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
There are a few annoying regressions with the default VCs introduced with the
pixman series. The "vl: revert behaviour for -display none" change solves most
of the issues. Another one is hit when using remote displays, and VCs are not
created as they used to, see: "ui/console: fix default VC when there are no
display". Finally, "ui: use "vc" chardev for dbus, gtk & spice-app" was meant to
be included in the pixman series and also brings back default VCs creation.
Marc-André Lureau (3):
vl: revert behaviour for -display none
ui: use "vc" chardev for dbus, gtk & spice-app
ui/console: fix default VC when there are no display
system/vl.c | 2 +-
ui/console.c | 18 ++++++++----------
ui/dbus.c | 1 +
ui/gtk.c | 1 +
ui/spice-app.c | 1 +
5 files changed, 12 insertions(+), 11 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH for-8.2 1/3] vl: revert behaviour for -display none
2023-11-17 14:35 [PATCH for-8.2 0/3] UI: fix default VC regressions marcandre.lureau
@ 2023-11-17 14:35 ` marcandre.lureau
2023-11-20 12:42 ` Peter Maydell
2023-11-17 14:35 ` [PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app marcandre.lureau
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: marcandre.lureau @ 2023-11-17 14:35 UTC (permalink / raw
To: qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Marc-André Lureau,
Paolo Bonzini, dwmw
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Commit 1bec1cc0d ("ui/console: allow to override the default VC") changed
the behaviour of the "-display none" option, so that it now creates a
QEMU monitor on the terminal. "-display none" should not be tangled up
with whether we create a monitor or a serial terminal; it should purely
and only disable the graphical window. Changing its behaviour like this
breaks command lines which, for example, use semihosting for their
output and don't want a graphical window, as they now get a monitor they
never asked for.
It also breaks the command line we document for Xen in
docs/system/i386/xen.html:
$ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split \
-display none -chardev stdio,mux=on,id=char0,signal=off -mon char0 \
-device xen-console,chardev=char0 -drive file=${GUEST_IMAGE},if=xen
qemu-system-x86_64: cannot use stdio by multiple character devices
qemu-system-x86_64: could not connect serial device to character backend
'stdio'
When qemu is compiled without PIXMAN, by default the serials aren't
muxed with the monitor anymore on stdio. The serials are redirected to
"null" instead, and the monitor isn't set up.
Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
system/vl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/system/vl.c b/system/vl.c
index 5af7ced2a1..14bf0cf0bf 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void)
}
}
- if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) {
+ if (nographic) {
if (default_parallel) {
add_device_config(DEV_PARALLEL, "null");
}
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app
2023-11-17 14:35 [PATCH for-8.2 0/3] UI: fix default VC regressions marcandre.lureau
2023-11-17 14:35 ` [PATCH for-8.2 1/3] vl: revert behaviour for -display none marcandre.lureau
@ 2023-11-17 14:35 ` marcandre.lureau
2023-11-21 9:27 ` Thomas Huth
2023-11-17 14:35 ` [PATCH for-8.2 3/3] ui/console: fix default VC when there are no display marcandre.lureau
2023-11-21 7:37 ` [PATCH for-8.2 0/3] UI: fix default VC regressions Marc-André Lureau
3 siblings, 1 reply; 15+ messages in thread
From: marcandre.lureau @ 2023-11-17 14:35 UTC (permalink / raw
To: qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Marc-André Lureau,
Paolo Bonzini, dwmw
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Those display have their own implementation of "vc" chardev, which
doesn't use pixman. They also don't implement the width/height/cols/rows
options, so qemu_display_get_vc() should return a compatible argument.
This patch was meant to be with the pixman series, when the "vc" field
was introduced. It fixes a regression where VC are created on the
tty (or null) instead of the display own "vc" implementation.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
ui/dbus.c | 1 +
ui/gtk.c | 1 +
ui/spice-app.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/ui/dbus.c b/ui/dbus.c
index 866467ad2e..e08b5de064 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -518,6 +518,7 @@ static QemuDisplay qemu_display_dbus = {
.type = DISPLAY_TYPE_DBUS,
.early_init = early_dbus_init,
.init = dbus_init,
+ .vc = "vc",
};
static void register_dbus(void)
diff --git a/ui/gtk.c b/ui/gtk.c
index be047a41ad..810d7fc796 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2534,6 +2534,7 @@ static QemuDisplay qemu_display_gtk = {
.type = DISPLAY_TYPE_GTK,
.early_init = early_gtk_display_init,
.init = gtk_display_init,
+ .vc = "vc",
};
static void register_gtk(void)
diff --git a/ui/spice-app.c b/ui/spice-app.c
index 405fb7f9f5..a10b4a58fe 100644
--- a/ui/spice-app.c
+++ b/ui/spice-app.c
@@ -220,6 +220,7 @@ static QemuDisplay qemu_display_spice_app = {
.type = DISPLAY_TYPE_SPICE_APP,
.early_init = spice_app_display_early_init,
.init = spice_app_display_init,
+ .vc = "vc",
};
static void register_spice_app(void)
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH for-8.2 3/3] ui/console: fix default VC when there are no display
2023-11-17 14:35 [PATCH for-8.2 0/3] UI: fix default VC regressions marcandre.lureau
2023-11-17 14:35 ` [PATCH for-8.2 1/3] vl: revert behaviour for -display none marcandre.lureau
2023-11-17 14:35 ` [PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app marcandre.lureau
@ 2023-11-17 14:35 ` marcandre.lureau
2023-11-21 9:27 ` Thomas Huth
2023-11-21 7:37 ` [PATCH for-8.2 0/3] UI: fix default VC regressions Marc-André Lureau
3 siblings, 1 reply; 15+ messages in thread
From: marcandre.lureau @ 2023-11-17 14:35 UTC (permalink / raw
To: qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Marc-André Lureau,
Paolo Bonzini, dwmw
From: Marc-André Lureau <marcandre.lureau@redhat.com>
When display is "none", we may still have remote displays (I think it
would be simpler if VNC/Spice were regular display btw). Return the
default VC then, and set them up to fix a regression when using remote
display and it used the TTY instead.
Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC")
Reported-by: German Maglione <gmaglione@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
ui/console.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/ui/console.c b/ui/console.c
index 8e688d3569..7db921e3b7 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1679,19 +1679,17 @@ void qemu_display_init(DisplayState *ds, DisplayOptions *opts)
const char *qemu_display_get_vc(DisplayOptions *opts)
{
- assert(opts->type < DISPLAY_TYPE__MAX);
- if (opts->type == DISPLAY_TYPE_NONE) {
- return NULL;
- }
- assert(dpys[opts->type] != NULL);
- if (dpys[opts->type]->vc) {
- return dpys[opts->type]->vc;
- } else {
#ifdef CONFIG_PIXMAN
- return "vc:80Cx24C";
+ const char *vc = "vc:80Cx24C";
+#else
+ const char *vc = NULL;
#endif
+
+ assert(opts->type < DISPLAY_TYPE__MAX);
+ if (dpys[opts->type] && dpys[opts->type]->vc) {
+ vc = dpys[opts->type]->vc;
}
- return NULL;
+ return vc;
}
void qemu_display_help(void)
--
2.41.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 1/3] vl: revert behaviour for -display none
2023-11-17 14:35 ` [PATCH for-8.2 1/3] vl: revert behaviour for -display none marcandre.lureau
@ 2023-11-20 12:42 ` Peter Maydell
2023-11-20 13:17 ` [EXTERNAL] " David Woodhouse
2023-11-21 10:11 ` David Woodhouse
0 siblings, 2 replies; 15+ messages in thread
From: Peter Maydell @ 2023-11-20 12:42 UTC (permalink / raw
To: marcandre.lureau; +Cc: qemu-devel, Gerd Hoffmann, Paolo Bonzini, dwmw
On Fri, 17 Nov 2023 at 14:35, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Commit 1bec1cc0d ("ui/console: allow to override the default VC") changed
> the behaviour of the "-display none" option, so that it now creates a
> QEMU monitor on the terminal. "-display none" should not be tangled up
> with whether we create a monitor or a serial terminal; it should purely
> and only disable the graphical window. Changing its behaviour like this
> breaks command lines which, for example, use semihosting for their
> output and don't want a graphical window, as they now get a monitor they
> never asked for.
>
> It also breaks the command line we document for Xen in
> docs/system/i386/xen.html:
>
> $ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split \
> -display none -chardev stdio,mux=on,id=char0,signal=off -mon char0 \
> -device xen-console,chardev=char0 -drive file=${GUEST_IMAGE},if=xen
>
> qemu-system-x86_64: cannot use stdio by multiple character devices
> qemu-system-x86_64: could not connect serial device to character backend
> 'stdio'
>
> When qemu is compiled without PIXMAN, by default the serials aren't
> muxed with the monitor anymore on stdio. The serials are redirected to
> "null" instead, and the monitor isn't set up.
>
> Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> system/vl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/system/vl.c b/system/vl.c
> index 5af7ced2a1..14bf0cf0bf 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void)
> }
> }
>
> - if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) {
> + if (nographic) {
> if (default_parallel) {
> add_device_config(DEV_PARALLEL, "null");
> }
This fixes the regression I was seeing with the semihosting
use case. I haven't checked the Xen setup.
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [EXTERNAL] [PATCH for-8.2 1/3] vl: revert behaviour for -display none
2023-11-20 12:42 ` Peter Maydell
@ 2023-11-20 13:17 ` David Woodhouse
2023-11-21 10:11 ` David Woodhouse
1 sibling, 0 replies; 15+ messages in thread
From: David Woodhouse @ 2023-11-20 13:17 UTC (permalink / raw
To: Peter Maydell, marcandre.lureau; +Cc: qemu-devel, Gerd Hoffmann, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]
On Mon, 2023-11-20 at 12:42 +0000, Peter Maydell wrote:
> On Fri, 17 Nov 2023 at 14:35, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Commit 1bec1cc0d ("ui/console: allow to override the default VC")
> > changed
> > the behaviour of the "-display none" option, so that it now creates
> > a
> > QEMU monitor on the terminal. "-display none" should not be tangled
> > up
> > with whether we create a monitor or a serial terminal; it should
> > purely
> > and only disable the graphical window. Changing its behaviour like
> > this
> > breaks command lines which, for example, use semihosting for their
> > output and don't want a graphical window, as they now get a monitor
> > they
> > never asked for.
> >
> > It also breaks the command line we document for Xen in
> > docs/system/i386/xen.html:
> >
> > $ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-
> > irqchip=split \
> > -display none -chardev stdio,mux=on,id=char0,signal=off -mon
> > char0 \
> > -device xen-console,chardev=char0 -drive
> > file=${GUEST_IMAGE},if=xen
> >
> > qemu-system-x86_64: cannot use stdio by multiple character devices
> > qemu-system-x86_64: could not connect serial device to character
> > backend
> > 'stdio'
> >
> > When qemu is compiled without PIXMAN, by default the serials aren't
> > muxed with the monitor anymore on stdio. The serials are redirected
> > to
> > "null" instead, and the monitor isn't set up.
> >
> > Fixes: commit 1bec1cc0d ("ui/console: allow to override the default
> > VC")
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > system/vl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/system/vl.c b/system/vl.c
> > index 5af7ced2a1..14bf0cf0bf 100644
> > --- a/system/vl.c
> > +++ b/system/vl.c
> > @@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void)
> > }
> > }
> >
> > - if (nographic || (!vc && !is_daemonized() &&
> > isatty(STDOUT_FILENO))) {
> > + if (nographic) {
> > if (default_parallel) {
> > add_device_config(DEV_PARALLEL, "null");
> > }
>
> This fixes the regression I was seeing with the semihosting
> use case. I haven't checked the Xen setup.
Should probably work, but we want the better fix for Xen anyway:
https://lore.kernel.org/qemu-devel/20231115172723.1161679-3-dwmw2@infradead.org/
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
2023-11-17 14:35 [PATCH for-8.2 0/3] UI: fix default VC regressions marcandre.lureau
` (2 preceding siblings ...)
2023-11-17 14:35 ` [PATCH for-8.2 3/3] ui/console: fix default VC when there are no display marcandre.lureau
@ 2023-11-21 7:37 ` Marc-André Lureau
2023-11-21 10:15 ` Woodhouse, David via
3 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2023-11-21 7:37 UTC (permalink / raw
To: qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Paolo Bonzini, dwmw,
Philippe Mathieu-Daudé, gmaglione@gmail.com, Thomas Huth
Hi
On Fri, Nov 17, 2023 at 6:36 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> There are a few annoying regressions with the default VCs introduced with the
> pixman series. The "vl: revert behaviour for -display none" change solves most
> of the issues. Another one is hit when using remote displays, and VCs are not
> created as they used to, see: "ui/console: fix default VC when there are no
> display". Finally, "ui: use "vc" chardev for dbus, gtk & spice-app" was meant to
> be included in the pixman series and also brings back default VCs creation.
>
> Marc-André Lureau (3):
> vl: revert behaviour for -display none
> ui: use "vc" chardev for dbus, gtk & spice-app
> ui/console: fix default VC when there are no display
I wish to send a PR (rc1 today), together with "[PATCH] vl: add
missing display_remote++".
Some R-B/A-B appreciated! thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app
2023-11-17 14:35 ` [PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app marcandre.lureau
@ 2023-11-21 9:27 ` Thomas Huth
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2023-11-21 9:27 UTC (permalink / raw
To: marcandre.lureau, qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Paolo Bonzini, dwmw
On 17/11/2023 15.35, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Those display have their own implementation of "vc" chardev, which
> doesn't use pixman. They also don't implement the width/height/cols/rows
> options, so qemu_display_get_vc() should return a compatible argument.
>
> This patch was meant to be with the pixman series, when the "vc" field
> was introduced. It fixes a regression where VC are created on the
> tty (or null) instead of the display own "vc" implementation.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> ui/dbus.c | 1 +
> ui/gtk.c | 1 +
> ui/spice-app.c | 1 +
> 3 files changed, 3 insertions(+)
FWIW,
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 3/3] ui/console: fix default VC when there are no display
2023-11-17 14:35 ` [PATCH for-8.2 3/3] ui/console: fix default VC when there are no display marcandre.lureau
@ 2023-11-21 9:27 ` Thomas Huth
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2023-11-21 9:27 UTC (permalink / raw
To: marcandre.lureau, qemu-devel
Cc: Gerd Hoffmann, peter.maydell, Paolo Bonzini, dwmw
On 17/11/2023 15.35, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> When display is "none", we may still have remote displays (I think it
> would be simpler if VNC/Spice were regular display btw). Return the
> default VC then, and set them up to fix a regression when using remote
> display and it used the TTY instead.
>
> Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC")
> Reported-by: German Maglione <gmaglione@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> ui/console.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 1/3] vl: revert behaviour for -display none
2023-11-20 12:42 ` Peter Maydell
2023-11-20 13:17 ` [EXTERNAL] " David Woodhouse
@ 2023-11-21 10:11 ` David Woodhouse
1 sibling, 0 replies; 15+ messages in thread
From: David Woodhouse @ 2023-11-21 10:11 UTC (permalink / raw
To: Peter Maydell, marcandre.lureau; +Cc: qemu-devel, Gerd Hoffmann, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 465 bytes --]
On Mon, 2023-11-20 at 12:42 +0000, Peter Maydell wrote:
>
> This fixes the regression I was seeing with the semihosting
> use case. I haven't checked the Xen setup.
>
> Tested-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
It does also work for the Xen command line (with my other fix
reverted).
Tested-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
2023-11-21 7:37 ` [PATCH for-8.2 0/3] UI: fix default VC regressions Marc-André Lureau
@ 2023-11-21 10:15 ` Woodhouse, David via
2023-11-21 10:37 ` Marc-André Lureau
0 siblings, 1 reply; 15+ messages in thread
From: Woodhouse, David via @ 2023-11-21 10:15 UTC (permalink / raw
To: marcandre.lureau@gmail.com, qemu-devel@nongnu.org
Cc: kraxel@redhat.com, gmaglione@gmail.com, peter.maydell@linaro.org,
pbonzini@redhat.com, thuth@redhat.com, philmd@linaro.org
[-- Attachment #1.1: Type: text/plain, Size: 1347 bytes --]
On Tue, 2023-11-21 at 11:37 +0400, Marc-André Lureau wrote:
> On Fri, Nov 17, 2023 at 6:36 PM <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi,
> >
> > There are a few annoying regressions with the default VCs introduced with the
> > pixman series. The "vl: revert behaviour for -display none" change solves most
> > of the issues. Another one is hit when using remote displays, and VCs are not
> > created as they used to, see: "ui/console: fix default VC when there are no
> > display". Finally, "ui: use "vc" chardev for dbus, gtk & spice-app" was meant to
> > be included in the pixman series and also brings back default VCs creation.
> >
> > Marc-André Lureau (3):
> > vl: revert behaviour for -display none
> > ui: use "vc" chardev for dbus, gtk & spice-app
> > ui/console: fix default VC when there are no display
>
> I wish to send a PR (rc1 today), together with "[PATCH] vl: add
> missing display_remote++".
>
> Some R-B/A-B appreciated! thanks
Not sure I can give coherent review on the other two, but the first
patch does fix the Xen command line and looks sane.
Please could I ask you to also include
https://lore.kernel.org/qemu-devel/20231115172723.1161679-3-dwmw2@infradead.org/
in the series as you push it?
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5957 bytes --]
[-- Attachment #2.1: Type: text/plain, Size: 215 bytes --]
Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
[-- Attachment #2.2: Type: text/html, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
2023-11-21 10:15 ` Woodhouse, David via
@ 2023-11-21 10:37 ` Marc-André Lureau
2023-11-21 10:42 ` David Woodhouse
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2023-11-21 10:37 UTC (permalink / raw
To: Woodhouse, David
Cc: qemu-devel@nongnu.org, kraxel@redhat.com, gmaglione@gmail.com,
peter.maydell@linaro.org, pbonzini@redhat.com, thuth@redhat.com,
philmd@linaro.org
Hi David
On Tue, Nov 21, 2023 at 2:15 PM Woodhouse, David <dwmw@amazon.co.uk> wrote:
>
> On Tue, 2023-11-21 at 11:37 +0400, Marc-André Lureau wrote:
> > On Fri, Nov 17, 2023 at 6:36 PM <marcandre.lureau@redhat.com> wrote:
> > >
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Hi,
> > >
> > > There are a few annoying regressions with the default VCs introduced with the
> > > pixman series. The "vl: revert behaviour for -display none" change solves most
> > > of the issues. Another one is hit when using remote displays, and VCs are not
> > > created as they used to, see: "ui/console: fix default VC when there are no
> > > display". Finally, "ui: use "vc" chardev for dbus, gtk & spice-app" was meant to
> > > be included in the pixman series and also brings back default VCs creation.
> > >
> > > Marc-André Lureau (3):
> > > vl: revert behaviour for -display none
> > > ui: use "vc" chardev for dbus, gtk & spice-app
> > > ui/console: fix default VC when there are no display
> >
> > I wish to send a PR (rc1 today), together with "[PATCH] vl: add
> > missing display_remote++".
> >
> > Some R-B/A-B appreciated! thanks
>
> Not sure I can give coherent review on the other two, but the first
> patch does fix the Xen command line and looks sane.
>
> Please could I ask you to also include
> https://lore.kernel.org/qemu-devel/20231115172723.1161679-3-dwmw2@infradead.org/
> in the series as you push it?
>
>
Thanks for the quick test. I am bit reluctant to push your change in
8.2 too. It's a change in behaviour at this point, not simply a fix.
But as the maintainer of Xen stuff, you have perhaps the final call.?
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
2023-11-21 10:37 ` Marc-André Lureau
@ 2023-11-21 10:42 ` David Woodhouse
2023-11-21 10:45 ` Marc-André Lureau
0 siblings, 1 reply; 15+ messages in thread
From: David Woodhouse @ 2023-11-21 10:42 UTC (permalink / raw
To: Marc-André Lureau
Cc: qemu-devel@nongnu.org, kraxel@redhat.com, gmaglione@gmail.com,
peter.maydell@linaro.org, pbonzini@redhat.com, thuth@redhat.com,
philmd@linaro.org
[-- Attachment #1: Type: text/plain, Size: 523 bytes --]
On Tue, 2023-11-21 at 14:37 +0400, Marc-André Lureau wrote:
>
> Thanks for the quick test. I am bit reluctant to push your change in
> 8.2 too. It's a change in behaviour at this point, not simply a fix.
> But as the maintainer of Xen stuff, you have perhaps the final call.?
it's not a change in behaviour yet. Being able to add xen-console
devices on the command line at all is new in 8.2, so it only ends up
being a "change" if we do it after the 8.2 release, which is why I'm
keen to do it now.
Thanks.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
2023-11-21 10:42 ` David Woodhouse
@ 2023-11-21 10:45 ` Marc-André Lureau
2023-11-21 10:47 ` David Woodhouse
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2023-11-21 10:45 UTC (permalink / raw
To: David Woodhouse
Cc: qemu-devel@nongnu.org, kraxel@redhat.com, gmaglione@gmail.com,
peter.maydell@linaro.org, pbonzini@redhat.com, thuth@redhat.com,
philmd@linaro.org
Hi
On Tue, Nov 21, 2023 at 2:42 PM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Tue, 2023-11-21 at 14:37 +0400, Marc-André Lureau wrote:
> >
> > Thanks for the quick test. I am bit reluctant to push your change in
> > 8.2 too. It's a change in behaviour at this point, not simply a fix.
> > But as the maintainer of Xen stuff, you have perhaps the final call.?
>
> it's not a change in behaviour yet. Being able to add xen-console
> devices on the command line at all is new in 8.2, so it only ends up
> being a "change" if we do it after the 8.2 release, which is why I'm
> keen to do it now.
>
I didn't realize that. Perhaps it's best to go through the Xen queue.
I already sent a PR for UI regressions, as we are close to rc1.
thanks!
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
2023-11-21 10:45 ` Marc-André Lureau
@ 2023-11-21 10:47 ` David Woodhouse
0 siblings, 0 replies; 15+ messages in thread
From: David Woodhouse @ 2023-11-21 10:47 UTC (permalink / raw
To: Marc-André Lureau
Cc: qemu-devel@nongnu.org, kraxel@redhat.com, gmaglione@gmail.com,
peter.maydell@linaro.org, pbonzini@redhat.com, thuth@redhat.com,
philmd@linaro.org
[-- Attachment #1: Type: text/plain, Size: 949 bytes --]
On Tue, 2023-11-21 at 14:45 +0400, Marc-André Lureau wrote:
> Hi
>
> On Tue, Nov 21, 2023 at 2:42 PM David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > On Tue, 2023-11-21 at 14:37 +0400, Marc-André Lureau wrote:
> > >
> > > Thanks for the quick test. I am bit reluctant to push your change in
> > > 8.2 too. It's a change in behaviour at this point, not simply a fix.
> > > But as the maintainer of Xen stuff, you have perhaps the final call.?
> >
> > it's not a change in behaviour yet. Being able to add xen-console
> > devices on the command line at all is new in 8.2, so it only ends up
> > being a "change" if we do it after the 8.2 release, which is why I'm
> > keen to do it now.
> >
>
> I didn't realize that. Perhaps it's best to go through the Xen queue.
> I already sent a PR for UI regressions, as we are close to rc1.
Makes sense. Could I trouble you for a R-b for it and then I'll do so?
Thanks.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-11-21 10:47 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-17 14:35 [PATCH for-8.2 0/3] UI: fix default VC regressions marcandre.lureau
2023-11-17 14:35 ` [PATCH for-8.2 1/3] vl: revert behaviour for -display none marcandre.lureau
2023-11-20 12:42 ` Peter Maydell
2023-11-20 13:17 ` [EXTERNAL] " David Woodhouse
2023-11-21 10:11 ` David Woodhouse
2023-11-17 14:35 ` [PATCH for-8.2 2/3] ui: use "vc" chardev for dbus, gtk & spice-app marcandre.lureau
2023-11-21 9:27 ` Thomas Huth
2023-11-17 14:35 ` [PATCH for-8.2 3/3] ui/console: fix default VC when there are no display marcandre.lureau
2023-11-21 9:27 ` Thomas Huth
2023-11-21 7:37 ` [PATCH for-8.2 0/3] UI: fix default VC regressions Marc-André Lureau
2023-11-21 10:15 ` Woodhouse, David via
2023-11-21 10:37 ` Marc-André Lureau
2023-11-21 10:42 ` David Woodhouse
2023-11-21 10:45 ` Marc-André Lureau
2023-11-21 10:47 ` David Woodhouse
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.