[PATCH 2/2] vbox: Fix starting domains

[Martin Kletzander]

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 at redhat.com>
>>> ---
>>> 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?

>Michal
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230123/8f8c93b7/attachment-0001.sig>