Re: [Qemu-devel] [PATCH] display: stop using DT_NOGRAPHIC, use DT_NONE

[Andreas Färber]

Am 28.06.2013 13:29, schrieb Michael Tokarev:
> 28.06.2013 15:24, Peter Maydell wrote:
>> On 19 June 2013 13:34, Michael Tokarev <address@hidden> wrote:
>>> It looks like initially there was -nographic option to turn
>>> off display, now there's another option of the same sort,
>>> -display none.  But code in other places of qemu checks for
>>> DT_NOGRAPHIC and does not work well with -display none.
>>> Make DT_NOGRAPHIC an internal version which selects DT_NONE,
>>> and check for that in all other places where previously we
>>> checked for DT_NOGRAPHIC.
>>>
>>> While at it, rename two private variants of display (DT_DEFAULT
>>> and DT_NOGRAPHIC) to use two underscores and make them negative,
>>> and set DT_NONE to 0.
>>>
>>> This should fix the issue of non-working sun serial console
>>> with the suggested replacement of -nographic which is
>>> -display none.
>>
>> Note that "-display none" and "-nographic" aren't exactly
>> equivalent -- the latter is an option which turns on a bunch
>> of behaviour including but not limited to "-display none".
> 
> Exactly.  See ab51b1d568e02c80b1abf9016bda3a86dc1db389
> for a bit more context of this.
> 
>>> I'm not still sure we really want to check for display type
>>> in qemu-char.c where we allow/disallow signals delivery from
>>> terminal, -- for other display types (CURSES) this makes no
>>> good sense.
>>
>> ...in particular I don't think "-display none" should
>> mean "don't allow ctrl-c" (though -nographic should
>> continue to have that effect), and this patch currently
>> introduces that behaviour change.
> 
> As Anthony said before, -nographic is legacy.  So there should
> be some more modern way to control this.  That's exactly the
> change which I don't like myself.  

> But "don't allow ctrl-c"
> which is currently bound to -nographic is equally wrong.

What's wrong about that? Isn't Ctrl+C passed through to the guest in
-nographic mode? I don't see how any other mode inclusing daemonize
would need that.

Andreas

> 
>>> +    DT__DEFAULT = -1,   /* private */
>> I think these could use a slightly longer comment than just "private",
>> eg "private, used internally by vl.c only".
> 
> Okay.
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg