On 1/23/23 15:22, Martin Kletzander wrote:
> On Mon, Jan 23, 2023 at 02:35:40PM +0100, Michal Prívozník wrote:
>> On 1/23/23 10:45, Martin Kletzander wrote:
>>> On Mon, Jan 23, 2023 at 10:31:53AM +0100, Michal Privoznik wrote:
>>>> When starting a VirtualBox domain, we try to guess which frontend
>>>> to use. While the whole algorithm looks a bit outdated, it may
>>>> happen that we tell VirtualBox to use "gui" frontend, but not
>>>> which DISPLAY= to use.
>>>>
>>>> I haven't found any documentation on the algorithm we use, but if
>>>> I make us fallback onto DISPLAY=:0 when no other configuration is
>>>> found then I'm able to start my guests just fine.
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>>>> ---
>>>> src/vbox/vbox_common.c | 13 ++++++-------
>>>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
>>>> index bd77641d39..5269f9b23f 100644
>>>> --- a/src/vbox/vbox_common.c
>>>> +++ b/src/vbox/vbox_common.c
>>>> @@ -2121,13 +2121,12 @@ vboxStartMachine(virDomainPtr dom, int
>>>> maxDomID, IMachine *machine, vboxIID *iid
>>>> VBOX_UTF8_FREE(valueDisplayUtf8);
>>>>
>>>> if (guiPresent) {
>>>> - if (guiDisplay) {
>>>> - char *displayutf8;
>>>> - displayutf8 = g_strdup_printf("DISPLAY=%s", guiDisplay);
>>>> - VBOX_UTF8_TO_UTF16(displayutf8, &env);
>>>> - VIR_FREE(displayutf8);
>>>> - VIR_FREE(guiDisplay);
>>>> - }
>>>> + char *displayutf8;
>>>> +
>>>> + displayutf8 = g_strdup_printf("DISPLAY=%s", guiDisplay ?
>>>> guiDisplay : ":0");
>>>> + VBOX_UTF8_TO_UTF16(displayutf8, &env);
>>>
>>> This might get overwritten when using SDL couple lines below. I suggest
>>> you default to :0 only if none other option remains. It feels dirty
>>> just guessing the display number, but let's say that's something we'll
>>> have to live with in the vbox driver.
>>
>> I'm failing to see how guiPresent and sdlPresent can be set at the same
>> time. But I guess I can join those two cases together, sure.
>>
>
> Sorry, I missed that, I'm probably too cautious, although there are some
> other places where we're trying to be equally cautious,
> e.g. vboxAttachDisplay:
>
> https://gitlab.com/libvirt/libvirt/-/blob/master/src/vbox/vbox_common.c#L1651
>
> But in any case, doesn't the block under sdlDisplay conditional need the
> same treatment?
Yeah, that's why I've sent v2:
https://listman.redhat.com/archives/libvir-list/2023-January/237296.html
Michal
