[PATCH for-8.2 0/3] UI: fix default VC regressions

marcandre.lureau@redhat.com posted 3 patches 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231117143506.1521718-1-marcandre.lureau@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
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(-)
[PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by marcandre.lureau@redhat.com 1 year ago
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


Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by Marc-André Lureau 1 year ago
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
Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by Woodhouse, David via 1 year ago
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?




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.


Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by Marc-André Lureau 1 year ago
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
Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by David Woodhouse 1 year ago
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.
Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by Marc-André Lureau 1 year ago
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
Re: [PATCH for-8.2 0/3] UI: fix default VC regressions
Posted by David Woodhouse 1 year ago
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.