From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45854) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eLucb-0003hM-S3 for qemu-devel@nongnu.org; Mon, 04 Dec 2017 12:33:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eLucY-0007uA-0p for qemu-devel@nongnu.org; Mon, 04 Dec 2017 12:33:37 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:57290) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eLucX-0007tw-P0 for qemu-devel@nongnu.org; Mon, 04 Dec 2017 12:33:33 -0500 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vB4HWOo9067828 for ; Mon, 4 Dec 2017 12:33:30 -0500 Received: from e06smtp11.uk.ibm.com (e06smtp11.uk.ibm.com [195.75.94.107]) by mx0a-001b2d01.pphosted.com with ESMTP id 2en945x40h-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 04 Dec 2017 12:33:30 -0500 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 4 Dec 2017 17:33:27 -0000 References: <20171128134648.21530-1-cohuck@redhat.com> <20171128134648.21530-3-cohuck@redhat.com> <7c5e59c3-fdee-604d-3ade-841fbdb045e3@de.ibm.com> <20171204174056.182ec0f0.cohuck@redhat.com> From: Halil Pasic Date: Mon, 4 Dec 2017 18:33:24 +0100 MIME-Version: 1.0 In-Reply-To: <20171204174056.182ec0f0.cohuck@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Message-Id: Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH RFC 2/2] s390x: attach autogenerated nics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , Christian Borntraeger Cc: qemu-devel@nongnu.org, qemu-s390x@nongnu.org, pmorel@linux.vnet.ibm.com On 12/04/2017 05:40 PM, Cornelia Huck wrote: > On Mon, 4 Dec 2017 12:17:06 +0100 > Christian Borntraeger wrote: > >> On 11/28/2017 02:46 PM, Cornelia Huck wrote: >>> The autogenerated nics should be treated as any other device; use >>> qdev_set_id() to have them show up under peripheral-anon. >>> >> I think this is fine, but then I ask myself how x86 does this. So I tried to >> find out how the pc-q35 machine does this but I somehow failed to understand >> how they do it. Do you have any clue? > > It seems they don't. If you start up a machine with only autogenerated > devices, you won't find anything under peripheral{-anon}, but several > devices under unattached. > > So, maybe we should change this for everything? Or just leave it alone? > > (The css-bridge change is a different thing IMO, it clearly should be > attached to the machine.) > IMHO (try to) change everywhere. The devices are attached to the machine, and them showing up as unattached is misleading. IMHO we still to have the 'is it API or not' question/problem so we need to be careful. Another think I was wondering about is ids: there are QMP commands which designate devices by path and there are commands which designate by id (and we even have either-or via the same parameter in case of device_del). Since the paths do not seem to be directly assigned/controlled by the user ([1]] but id's are I would argue that ids are easier to understand and use. Would generating an id for each auto-generated device be a good idea? I'm trying to figure out, how the QAPI is supposed to be used, and feel like. So take my comments with a grain of salt. [1] One can, but does not have to specify the bus. Libvirt does not seem to for virtio-ccw devices. And if one were to, the other patch in the series could break that code. Halil >> >>> Signed-off-by: Cornelia Huck >>> --- >>> hw/s390x/s390-virtio-ccw.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>> index a23b8aec9f..830bae9d0f 100644 >>> --- a/hw/s390x/s390-virtio-ccw.c >>> +++ b/hw/s390x/s390-virtio-ccw.c >>> @@ -35,6 +35,7 @@ >>> #include "cpu_models.h" >>> #include "qapi/qmp/qerror.h" >>> #include "hw/nmi.h" >>> +#include "include/monitor/qdev.h" >>> >>> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) >>> { >>> @@ -259,6 +260,7 @@ static void s390_create_virtio_net(BusState *bus, const char *name) >>> >>> dev = qdev_create(bus, name); >>> qdev_set_nic_properties(dev, nd); >>> + qdev_set_id(dev, NULL); >>> qdev_init_nofail(dev); >>> } >>> } >>> >> >