From patchwork Mon Apr 29 16:28:04 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: fred.konrad@greensocs.com X-Patchwork-Id: 240412 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EC8CC2C00A4 for ; Tue, 30 Apr 2013 02:28:29 +1000 (EST) Received: from localhost ([::1]:52738 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWqwG-0003gh-4h for incoming@patchwork.ozlabs.org; Mon, 29 Apr 2013 12:28:28 -0400 Received: from eggs.gnu.org ([208.118.235.92]:47796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWqw0-0003gE-2R for qemu-devel@nongnu.org; Mon, 29 Apr 2013 12:28:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UWqvy-0001KC-DF for qemu-devel@nongnu.org; Mon, 29 Apr 2013 12:28:12 -0400 Received: from greensocs.com ([87.106.252.221]:40562 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWqvy-0001Jy-4p for qemu-devel@nongnu.org; Mon, 29 Apr 2013 12:28:10 -0400 Received: from localhost (unknown [127.0.0.1]) by s15328186.onlinehome-server.info (Postfix) with ESMTP id 9D67940B250; Mon, 29 Apr 2013 16:28:08 +0000 (UTC) Received: from s15328186.onlinehome-server.info ([127.0.0.1]) by localhost (s15328186.onlinehome-server.info [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id f32Ovs61AR4N; Mon, 29 Apr 2013 18:28:06 +0200 (CEST) Received: by s15328186.onlinehome-server.info (Postfix, from userid 491) id 9AF7440EC61; Mon, 29 Apr 2013 18:28:06 +0200 (CEST) Received: from localhost.localdomain (lan31-11-83-155-143-136.fbx.proxad.net [83.155.143.136]) by s15328186.onlinehome-server.info (Postfix) with ESMTPSA id BC9F740B250; Mon, 29 Apr 2013 18:28:05 +0200 (CEST) Message-ID: <517E9F94.4010302@greensocs.com> Date: Mon, 29 Apr 2013 18:28:04 +0200 From: =?ISO-8859-15?Q?KONRAD_Fr=E9d=E9ric?= User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Paolo Bonzini References: <1367248365-27260-1-git-send-email-fred.konrad@greensocs.com> <1367248365-27260-5-git-send-email-fred.konrad@greensocs.com> <517E9552.5020303@redhat.com> In-Reply-To: <517E9552.5020303@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 87.106.252.221 Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, agraf@suse.de, amit.shah@redhat.com, cornelia.huck@de.ibm.com, afaerber@suse.de Subject: Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility. X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 29/04/2013 17:44, Paolo Bonzini wrote: > Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto: >> From: KONRAD Frederic >> >> The bus name is wrong since the refactoring. >> >> This keep the behaviour of the command line. >> >> Signed-off-by: KONRAD Frederic >> --- >> hw/s390x/s390-virtio-bus.c | 9 +++++++++ >> hw/s390x/virtio-ccw.c | 9 +++++++++ >> hw/scsi/virtio-scsi.c | 14 +++++++++++++- >> hw/virtio/virtio-pci.c | 9 +++++++++ >> include/hw/virtio/virtio-scsi.h | 7 +++++++ >> 5 files changed, 47 insertions(+), 1 deletion(-) >> >> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c >> index 6620d29..e1fd937 100644 >> --- a/hw/s390x/s390-virtio-bus.c >> +++ b/hw/s390x/s390-virtio-bus.c >> @@ -232,6 +232,15 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev) >> { >> VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(s390_dev); >> DeviceState *vdev = DEVICE(&dev->vdev); >> + DeviceState *qdev = DEVICE(s390_dev); >> + >> + /* >> + * For command line compatibility, this set the virtio-scsi-device bus >> + * name as before. >> + */ >> + if (qdev->id) { >> + set_virtio_scsi_bus_name(vdev, qdev->id); >> + } > Could this be simply a qdev property? Yes, that can be a good idea. What about adding a qdev property bus_name and using it in qbus_realize? Like this: len = strlen(bus->parent->id) + 16; If so, change to scsi_bus_new is not needed and the two new functions are not needed. Is that making sense? Fred > Consider that qdev will strdup any bus name you pass, so it is perfectly > ok to do: > > bus_name = g_strdup_printf("%s.0", vs->bus_name); > scsi_bus_new(..., bus_name); > g_free(bus_name); > >> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name) >> +{ >> + VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); >> + if (bus_name) { >> + vs->bus_name = g_malloc(strlen(bus_name) + 3); >> + snprintf(vs->bus_name, strlen(bus_name) + 3, "%s.0", bus_name); > This would use g_strdup_printf, as above. > > Paolo > >> + } >> +} >> + >> int virtio_scsi_common_init(VirtIOSCSICommon *s) >> { >> VirtIODevice *vdev = VIRTIO_DEVICE(s); >> @@ -624,7 +633,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev) >> return ret; >> } >> >> - scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info); >> + scsi_named_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vs->bus_name); >> if (!qdev->hotplugged) { >> scsi_bus_legacy_handle_cmdline(&s->bus); >> } >> @@ -639,6 +648,9 @@ int virtio_scsi_common_exit(VirtIOSCSICommon *vs) >> { >> VirtIODevice *vdev = VIRTIO_DEVICE(vs); >> >> + if (vs->bus_name) { >> + g_free(vs->bus_name); >> + } >> g_free(vs->cmd_vqs); >> virtio_cleanup(vdev); >> return 0; >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index 598876f..14fb8dd 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1106,11 +1106,20 @@ static int virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev) >> VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev); >> DeviceState *vdev = DEVICE(&dev->vdev); >> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev); >> + DeviceState *proxy = DEVICE(vpci_dev); >> >> if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) { >> vpci_dev->nvectors = vs->conf.num_queues + 3; >> } >> >> + /* >> + * For command line compatibility, this set the virtio-scsi-device bus >> + * name as before. >> + */ >> + if (proxy->id) { >> + set_virtio_scsi_bus_name(vdev, proxy->id); >> + } >> + >> qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); >> if (qdev_init(vdev) < 0) { >> return -1; >> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h >> index 4db346b..c356d54 100644 >> --- a/include/hw/virtio/virtio-scsi.h >> +++ b/include/hw/virtio/virtio-scsi.h >> @@ -164,6 +164,9 @@ typedef struct VirtIOSCSICommon { >> VirtQueue *ctrl_vq; >> VirtQueue *event_vq; >> VirtQueue **cmd_vqs; >> + >> + /* bus_name, for command line compatibility */ >> + char *bus_name; >> } VirtIOSCSICommon; >> >> typedef struct { >> @@ -189,5 +192,9 @@ typedef struct { >> int virtio_scsi_common_init(VirtIOSCSICommon *vs); >> int virtio_scsi_common_exit(VirtIOSCSICommon *vs); >> >> +/* >> + * For command line back compatibility, set the bus name before initialisation. >> + */ >> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name); >> >> #endif /* _QEMU_VIRTIO_SCSI_H */ >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 4eb0134..c5d5407 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -421,6 +421,13 @@ static void qbus_realize(BusState *bus, DeviceState *parent, const char *name) if (name) { bus->name = g_strdup(name); + } else if (bus->parent && bus->parent->bus_name) { + /* parent device has bus_name -> use it for bus name */ + len = strlen(bus->parent->bus_name) + 16; + buf = g_malloc(len); + snprintf(buf, len, "%s.%d", bus->parent->bus_name, + bus->parent->num_child_bus); + bus->name = buf; } else if (bus->parent && bus->parent->id) { /* parent device has id -> use it for bus name */