From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=34863 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PDGt2-0003vR-Py
	for qemu-devel@nongnu.org; Tue, 02 Nov 2010 09:26:55 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1PDGt0-0001Iv-3k
	for qemu-devel@nongnu.org; Tue, 02 Nov 2010 09:26:52 -0400
Received: from mx1.redhat.com ([209.132.183.28]:16659)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1PDGsz-0001Ia-TM
	for qemu-devel@nongnu.org; Tue, 02 Nov 2010 09:26:50 -0400
From: Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 06/40] qdev-ify: xen backends
References: <1288623713-28062-1-git-send-email-agraf@suse.de>
	<1288623713-28062-7-git-send-email-agraf@suse.de>
	<m3lj5c2g20.fsf@blackfin.pond.sub.org> <4CCFEB54.5030103@redhat.com>
Date: Tue, 02 Nov 2010 14:26:46 +0100
In-Reply-To: <4CCFEB54.5030103@redhat.com> (Gerd Hoffmann's message of "Tue,
	02 Nov 2010 11:43:32 +0100")
Message-ID: <m339rj26w9.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Alexander Graf <agraf@suse.de>, qemu-devel Developers <qemu-devel@nongnu.org>

Gerd Hoffmann <kraxel@redhat.com> writes:

> On 11/02/10 11:08, Markus Armbruster wrote:
>> Alexander Graf<agraf@suse.de>  writes:
>>
>>> From: Gerd Hoffmann<kraxel@redhat.com>
>>>
>>> This patch converts the xen backend code to qdev.
>>
>> qdev conversions are always welcome.  This one's not complete (search
>> for #if 0).
>
> It is a tricky one too.
>
> Creating the xen backend device instances is controlled via xenstore
> (either emulated in case of xenner or xenstored when running on
> Xen). When creating block/net backends via qemu command line switches
> all qemu does is creating the xenstore entries.  Having a external
> entity (i.e. xend) creating the xenstore entries works too.
>
> This workflow is a bit hard to fit into the qdev model ...

I'm fine with imperfect qdev conversions, as long as the issues make
things no worse then they were before (check, I think), they're clearly
documented in the source (check, although the comment could be
improved), and in the commit message (fail, but easy enough to fix).

>>> +    do {
>>> +        done = 1;
>>> +        QLIST_FOREACH(qdev,&xenbus->qbus.children, sibling) {
>>> +            xendev = container_of(qdev, struct XenDevice, qdev);
>
> [ ... ]
>
>>> +            done = 0;
>>> +            break;
>>> +        }
>>> +    } while (!done);
>>
>> This loop nest confuses me.  Why can't we just QLIST_FOREACH_SAFE()?
>
> Just historical reasons I guess.  QLIST_FOREACH_SAFE() wasn't there
> from the start but got added later.

I'd like that to be cleaned up then.