On Mon, Mar 11, 2013 at 02:26:58PM -0500, Anthony Liguori wrote:
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Fri, Mar 08, 2013 at 07:36:28AM -0600, Anthony Liguori wrote:
Markus Armbruster <armbru@redhat.com> writes:
"Michael S. Tsirkin" <mst@redhat.com> writes:
On Thu, Mar 07, 2013 at 08:57:52PM +0100, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst@redhat.com> writes:
> libvirt has a long-standing bug: when removing the device, > it can request removal but does not know when the > removal completes. Add an event so we can fix this in a robust way. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Speaking as the acting QMP maintainer, just to avoid misunderstandings: there's disagreement on the event's design, namely when it should fire, and how it should name the device. I don't want the discussion preempted by a commit.
Yes, you are asking for more functionality, but can I add this in a follow-up commit please? I prefer this patch as is, as it can be backported to stable branches and downstreams. Upstream a follow up patch can add fields and more triggers which won't apply to any downstreams.
If you want to address my review comments in a separate patch, go right ahead. Please post both together as a series, for coherent review and to simplify patch tracking.
I'm asking for two things:
1. Event member path. Fair to call this "more functionality". I agree that backporting it to pre-QOM versions isn't practical.
2. Sane event trigger condition: on any device deletion, not just when the device happens to have a qdev ID. This isn't "more", it's "different".
Ack.
Regards,
Anthony Liguori
So how does one get the path that you require?
ERROR:qom/object.c:1011:object_get_canonical_path: assertion failed: (prop != NULL)
Can you share your patch? This means something is wrong. All devices have a canonical path.
Regards,
Anthony Liguori
I figured it out - we were trying to get the path after the device was detached from the parent. We'll just have to calculate the path before unparenting and pass it in. -- MST