qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Date: Mon, 22 Sep 2014 13:43:34 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Paolo Bonzini <address@hidden> writes:

> Il 22/09/2014 10:34, Michael S. Tsirkin ha scritto:
>> Basically this patch brings back historical HMP behaviour.
>> As far as I could tell, it wasn't changed intentionally.
>
> It was changed intentionally.  Or rather, the change was known at the
> time Stefan made it.

Commit hash?

>> So how about applying this first and then making more
>> changes on top if required?
>
> No.
>
> There is no reason to add alias-specific fields to ObjectProperty, and
> the implementation is not even complete because you can alias property X
> of object A to property Y of object B.

I think there are two questions here.  The first one is whether to
accept the regression and move on.  The second one is whether this patch
is a satisfactory fix for the regression.

I'm inclined to answer the first question in the negative.  We discussed
the relative usefulness of the help information before and after the
regression, as well as after ripping out legacy_name entirely.  We agree
that help has always been somewhat cryptic.  I'd certainly support a
patch that replaces it wholesale by something better.  I'm opposed,
however, to degrading it further, if we can help it.

The fact that such a degradation has slipped in already can't change my
opinion.  I view it as regression.

Evidence of a deliberate, informed decision to regress could change it.
That's why I ask for the commit hash.

The "if we can help it" condition ties the first to the second question.

I haven't got this part of the code present in my mind at the moment,
but I'm willing to take your assertion of a layering violation for
granted.  Is this violation necessary for fixing the regression, or is
it just an artifact of this particular fix?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]