Hi On Tue, Oct 22, 2024 at 12:23 AM Andrew Keesler <ankees...@google.com> wrote:
> Hi Marc-André - > > The ability to set the name with QMP qom-set seems like a nice behavior. > > Note that the ultimate goal of this name is to propagate it downstream to > a device (see next patch[0] for a sample propagation to virtio-gpu). > > In order to accomplish this, would it work to expose this new "head_name" > property via a qemu_graphic_console_get_head_name(QemuConsole *c) function > that: > > 1. verifies that c is indeed a QemuGraphicConsole with > QEMU_IS_GRAPHIC_CONSOLE(), and > 2. returns c->head_name (similar to qemu_console_get_name() from [0])? > > We'd probably need a similar function > qemu_graphic_console_get_head_name(QemuConsole *c, const char *name) in > order to > set the head_name from a display (e.g., VNC) - correct me if you were > thinking > Right (qemu_graphic_console_set_head_name), get/set exposed to QOM via object_class_property_add_str() > of going a different direction with this interface, though. My main goal > is to > provide some way for a user to inject a display EDID name from the command > line. > > Also, just to verify my understanding - there would never be a QemuConsole > that > a) is NOT a QemuGraphicConsole AND b) is associated with an EDID in a > guest, > correct? > > Seems correct. (fwiw, I think we should have all UI info(s) as part of QemuUIInfo, including the head name, but this would require further refactoring to avoid some copy etc) > [0] https://lists.gnu.org/archive/html/qemu-devel/2024-10/msg03238.html > > On Mon, Oct 21, 2024 at 7:14 AM Marc-André Lureau < > marcandre.lur...@redhat.com> wrote: > >> Hi Roque >> >> On Fri, Oct 18, 2024 at 1:53 AM Roque Arcudia Hernandez >> <roq...@google.com> wrote: >> > >> > From: Andrew Keesler <ankees...@google.com> >> > >> > Thanks to 72d277a7, 1ed2cb32, and others, EDID (Extended Display >> Identification >> > Data) is propagated by QEMU such that a virtual display presents >> legitimate >> > metadata (e.g., name, serial number, preferred resolutions, etc.) to its >> > connected guest. >> > >> > This change propagates an optional user-provided display name to >> > QemuConsole. Future changes will update downstream devices to leverage >> this >> > display name for various uses, the primary one being providing a custom >> EDID >> > name to guests. Future changes will also update other displays (e.g., >> spice) >> > with a similar option to propagate a display name to downstream devices. >> > >> > Currently, every virtio-gpu virtual display has the same name: "QEMU >> > Monitor". We hope to be able to inject the EDID name of virtual >> displays in >> > order to test guest behavior that is specific to display names. We >> provide the >> > ability to inject the display name from the display configuration as >> that most >> > closely resembles how real displays work (hardware displays contain >> static EDID >> > information that is provided to every connected host). >> > >> > It should also be noted that EDID names longer than 12 bytes will be >> truncated >> > per spec (I think?). >> > >> > Signed-off-by: Andrew Keesler <ankees...@google.com> >> > Signed-off-by: Roque Arcudia Hernandez <roq...@google.com> >> > --- >> > include/ui/console.h | 1 + >> > ui/console-priv.h | 1 + >> > ui/console.c | 8 ++++++++ >> > ui/vnc.c | 8 +++++++- >> > 4 files changed, 17 insertions(+), 1 deletion(-) >> > >> > diff --git a/include/ui/console.h b/include/ui/console.h >> > index 5832d52a8a..74ab03ed72 100644 >> > --- a/include/ui/console.h >> > +++ b/include/ui/console.h >> > @@ -408,6 +408,7 @@ int qemu_console_get_index(QemuConsole *con); >> > uint32_t qemu_console_get_head(QemuConsole *con); >> > int qemu_console_get_width(QemuConsole *con, int fallback); >> > int qemu_console_get_height(QemuConsole *con, int fallback); >> > +void qemu_console_set_name(QemuConsole *con, const char *name); >> > /* Return the low-level window id for the console */ >> > int qemu_console_get_window_id(QemuConsole *con); >> > /* Set the low-level window id for the console */ >> > diff --git a/ui/console-priv.h b/ui/console-priv.h >> > index 43ceb8122f..9f2769843f 100644 >> > --- a/ui/console-priv.h >> > +++ b/ui/console-priv.h >> > @@ -18,6 +18,7 @@ struct QemuConsole { >> > Object parent; >> > >> > int index; >> > + const char *name; >> > DisplayState *ds; >> > DisplaySurface *surface; >> > DisplayScanout scanout; >> > diff --git a/ui/console.c b/ui/console.c >> > index 5165f17125..f377fd8417 100644 >> > --- a/ui/console.c >> > +++ b/ui/console.c >> > @@ -1452,6 +1452,14 @@ int qemu_console_get_height(QemuConsole *con, >> int fallback) >> > } >> > } >> > >> > +void qemu_console_set_name(QemuConsole *con, const char *name) >> > +{ >> > + if (con == NULL) { >> > + return; >> > + } >> > + con->name = name; >> > +} >> > + >> > int qemu_invalidate_text_consoles(void) >> > { >> > QemuConsole *s; >> > diff --git a/ui/vnc.c b/ui/vnc.c >> > index 93a8dbd253..7d6acc5c2e 100644 >> > --- a/ui/vnc.c >> > +++ b/ui/vnc.c >> > @@ -3595,6 +3595,9 @@ static QemuOptsList qemu_vnc_opts = { >> > },{ >> > .name = "power-control", >> > .type = QEMU_OPT_BOOL, >> > + },{ >> > + .name = "name", >> > + .type = QEMU_OPT_STRING, >> > }, >> > { /* end of list */ } >> > }, >> > @@ -4016,7 +4019,7 @@ void vnc_display_open(const char *id, Error >> **errp) >> > QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id); >> > g_autoptr(SocketAddressList) saddr_list = NULL; >> > g_autoptr(SocketAddressList) wsaddr_list = NULL; >> > - const char *share, *device_id; >> > + const char *share, *device_id, *name; >> > QemuConsole *con; >> > bool password = false; >> > bool reverse = false; >> > @@ -4217,6 +4220,9 @@ void vnc_display_open(const char *id, Error >> **errp) >> > } >> > qkbd_state_set_delay(vd->kbd, key_delay_ms); >> > >> > + name = qemu_opt_get(opts, "name"); >> > + qemu_console_set_name(vd->dcl.con, name); >> >> Why not expose a "head_name" property in QemuGraphicConsole? >> >> This way it should be possible to set the name with QMP qom-set. >> >> -- Marc-André Lureau
