Re: [Qemu-trivial] [PATCH] curses: don't initialize curses when qemu is daemonized

[Hitoshi Mitake]

On Fri, Sep 14, 2012 at 11:31 PM, Stefan Hajnoczi <address@hidden> wrote:
> 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.
>

I considered about the combination of -daemonize and -curses, and
concluded that it is odd but not an invalid thing.
So I decided not to treat it as an error.

I'll send the v2 later.

Thanks,
Hitoshi