Re: [Qemu-devel] [patch 2/6] Use machine options to emulate -no-kvm-irqchip

[Jan Kiszka]

On 2012-10-03 20:26, Marcelo Tosatti wrote:
> On Wed, Oct 03, 2012 at 07:24:48PM +0200, Jan Kiszka wrote:
>> On 2012-10-03 19:16, Anthony Liguori wrote:
>>> Jan Kiszka <address@hidden> writes:
>>>
>>>> On 2012-10-03 17:03, Marcelo Tosatti wrote:
>>>>> On Wed, Oct 03, 2012 at 09:40:17AM -0500, Anthony Liguori wrote:
>>>>>> Marcelo Tosatti <address@hidden> writes:
>>>>>>
>>>>>>> Commit 3ad763fcba5bd0ec5a79d4a9b6baeef119dd4a3d from qemu-kvm.git.
>>>>>>>
>>>>>>> From: Jan Kiszka <address@hidden>
>>>>>>>     
>>>>>>> Upstream is moving towards this mechanism, so start using it in qemu-kvm
>>>>>>> already to configure the specific defaults: kvm enabled on, just like
>>>>>>> in-kernel irqchips.
>>>>>>>
>>>>>>> Signed-off-by: Marcelo Tosatti <address@hidden>
>>>>>>
>>>>>>
>>>>>> Reviewed-by: Anthony Liguori <address@hidden>
>>>>>>
>>>>>> Although it's a little odd to have From: Jan without a SoB...
>>>>>
>>>>> Agree, Jan can you ACK?
>>>>
>>>> I wasn't able to join the call yesterday: Is there a removal schedule
>>>> associated with those switches? Also, why pushing things upstream, even
>>>> when only for one release, that have been loudly deprecated for a while
>>>> in qemu-kvm? Some switches are lacking deprecated warnings on the
>>>> console, and -no-kvm is missing completely. I tend to focus on patch 1 &
>>>> 5, dropping the rest - based on relevance for production use.
>>>
>>> The distros need to keep these flags to do the switch.
>>
>> Why? Should be documented in commit log.
>>
>>>  I see no point
>>> in deprecating them since they're trivially easy to maintain.
>>
>> Given the level of cr** we already have in the command line, they are
>> kind of noise, yes. But even then, these patches are not consistent as
>> pointed out above.
>>
>> Also, they should not be documented to avoid being spread. That's what
>> we did with other deprecated switches in QEMU.
>>
>> Jan
> 
> Jan,
> 
> You're comments to the patch are:
> 
> - No documentation.

See e.g. how -M is handled in qemu-options.hx.

> - Expiration date.

Anthony said "forever", but I think we should remove all those that
issue deprecation warnings after 1-2 years.

> - Changelog explaining what?? (didnt get that). Perhaps better changelog
>   in general?

I'm still failing to understand who could depend on -no-kvm-irqchip or
-no-kvm-pit. And I don't understand why -no-kvm was not included. Soe
the reasons for include -X should be provided. Also check your patch
subjects again, at least one was wrong.

Jan

signature.asc
Description: OpenPGP digital signature