[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH] curses: don't initialize curses when qemu is
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-trivial] [PATCH] curses: don't initialize curses when qemu is daemonized |
Date: |
Fri, 14 Sep 2012 15:31:38 +0100 |
On Fri, Sep 14, 2012 at 3:28 PM, Hitoshi Mitake <address@hidden> wrote:
> On Fri, Sep 14, 2012 at 9:40 PM, Stefan Hajnoczi <address@hidden> wrote:
>> On Fri, Sep 14, 2012 at 12:25 PM, Hitoshi Mitake <address@hidden> wrote:
>>> On Fri, Sep 14, 2012 at 4:53 PM, Stefan Hajnoczi <address@hidden> wrote:
>>>> On Wed, Sep 12, 2012 at 09:38:30PM +0900, Hitoshi Mitake wrote:
>>>>> Current qemu initializes curses even if -daemonize option is
>>>>> passed. This cause problem because shell prompt appears without
>>>>> calling endwin().
>>>>>
>>>>> This patch adds new function, is_daemonized(), to OS dependent
>>>>> code. With this function, curses_display_init() can check that qemu is
>>>>> daemonized or not. If daemonized, curses_display_init() returns
>>>>> immediately.
>>>>>
>>>>> Of course, -daemonize && -curses doesn't make sense. Users shouldn't
>>>>> pass the arguments at the same time. But the problem is very painful
>>>>> because Ctrl-C cannot be delivered to the terminal. So if this
>>>>> approach isn't acceptable, -daemonize && -curses combination should be
>>>>> treated as error.
>>>>
>>>> I think it makes sense to explicitly check that -daemonize is used with
>>>> -nographic | -vnc | -spice.
>>>
>>> Yes. But -curses doesn't make sense when it is passed with -daemonize.
>>>
>>>>
>>>> Making curses aware of daemonize seems strange and doesn't stop the same
>>>> issue with SDL.
>>>>
>>>
>>> What is the same issue with SDL?
>>
>> You are right, it's fine to have an SDL display when QEMU is daemonized.
>>
>> What bugged me about the patch is that ui/curses.c shouldn't know
>> about daemonize. It would be cleaner to check this before
>> initializing the curses display. An error could be printed instead of
>> silently skipping curses initialization.
>>
>> Can you move the check to vl.c:main() so it calls is_daemonized() and
>> exits with an error message?
>>
>
> I agree with moving is_daemonized() to main(). I'll include this change
> in the next version.
>
> But I'm worrying about the way of printing error message and exiting.
> This may break scripts which execute qemu with -daemonize && -curses.
>
> So I like the way of skipping initialization of curses silently.
> It doesn't change the semantics of command line option of qemu.
>
> I'd like to hear your comments.
It's a trade-off. Silently skipping curses initialization allows
users with broken command-lines to continue, but it may confuse new
users who try out this combination (they will not get a clear error
message explaining what is going on).
I think it's better to be explicit because the risk of breaking
existing user's setups is low. If you prefer the other way, that's
fine by me too.
Stefan