All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Fam Zheng <fam@euphon.net>, Alberto Garcia <berto@igalia.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	"Daniel P. Berrange" <berrange@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH 0/6] Improve consistency of bus init function names
Date: Thu, 23 Sep 2021 11:38:46 -0400	[thread overview]
Message-ID: <20210923113832-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20210923121153.23754-1-peter.maydell@linaro.org>

On Thu, Sep 23, 2021 at 01:11:47PM +0100, Peter Maydell wrote:
> Currently we have a bit of a mishmash of different function
> names for bus creation. There are two basic patterns: you
> can have a function that allocates and returns a new bus
> object; or you can have a function that takes a pointer to
> a bus object and initializes it in-place. We have to some
> extent a convention for those: the allocate-and-return
> function is 'foo_new()', and the 'init in-place' function
> is 'foo_init()'. However many of our bus creation functions
> don't follow that; some use 'foo_new' vs 'foo_new_inplace';
> some use 'foo_new' for the in-place init version; and
> the bottom level qbus functions are 'qbus_create' vs
> 'qbus_create_inplace'. This series tries to bring at least
> scsi, ipack, pci, ide, and qbus into line with the
> _new-vs-_init naming convention.
> 
> The other issue with bus creation functions is that some
> of them take a 'name' argument which can be NULL, and some
> do not. Generally "pass in a specific name" should be the
> rare case, but our API design here is easy to misuse, and
> so a lot of callsites (especially for i2c, sd, ssi) pass
> in names when they should not. Untangling that mess is
> going to be tricky (see other thread for more), but as
> a first step, this series proposes a split between
> foo_bus_new() and foo_bus_new_named() where the latter
> takes a name parameter and the former does not. I do
> this only for scsi (and implicitly ide, whose ide_bus_new
> function already doesn't take a name argument) for the
> moment, as the other bus types have more of a mess of
> "pass name when they should not" callsites, so I didn't
> want to put in too much work before finding out if we
> had agreement on this as a naming convention.
> 
> There are definitely more buses that can be cleaned up
> to follow the init vs new convention, but this series is
> already touching 70 files and trying to do every bus in
> one series seems like a recipe for merge conflicts.
> So this seemed like enough to be going on with...
> 
> thanks
> -- PMM


pci:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

feel free to merge.

> Peter Maydell (6):
>   scsi: Replace scsi_bus_new() with scsi_bus_init(),
>     scsi_bus_init_named()
>   ipack: Rename ipack_bus_new_inplace() to ipack_bus_init()
>   pci: Rename pci_root_bus_new_inplace() to pci_root_bus_init()
>   qbus: Rename qbus_create_inplace() to qbus_init()
>   qbus: Rename qbus_create() to qbus_new()
>   ide: Rename ide_bus_new() to ide_bus_init()
> 
>  include/hw/ide/internal.h     |  4 ++--
>  include/hw/ipack/ipack.h      |  8 ++++----
>  include/hw/pci/pci.h          | 10 +++++-----
>  include/hw/qdev-core.h        |  6 +++---
>  include/hw/scsi/scsi.h        | 30 ++++++++++++++++++++++++++++--
>  hw/audio/intel-hda.c          |  2 +-
>  hw/block/fdc.c                |  2 +-
>  hw/block/swim.c               |  3 +--
>  hw/char/virtio-serial-bus.c   |  4 ++--
>  hw/core/bus.c                 | 13 +++++++------
>  hw/core/sysbus.c              | 10 ++++++----
>  hw/gpio/bcm2835_gpio.c        |  3 +--
>  hw/hyperv/vmbus.c             |  2 +-
>  hw/i2c/core.c                 |  2 +-
>  hw/ide/ahci.c                 |  2 +-
>  hw/ide/cmd646.c               |  2 +-
>  hw/ide/isa.c                  |  2 +-
>  hw/ide/macio.c                |  2 +-
>  hw/ide/microdrive.c           |  2 +-
>  hw/ide/mmio.c                 |  2 +-
>  hw/ide/piix.c                 |  2 +-
>  hw/ide/qdev.c                 |  4 ++--
>  hw/ide/sii3112.c              |  2 +-
>  hw/ide/via.c                  |  2 +-
>  hw/ipack/ipack.c              | 10 +++++-----
>  hw/ipack/tpci200.c            |  4 ++--
>  hw/isa/isa-bus.c              |  2 +-
>  hw/misc/auxbus.c              |  2 +-
>  hw/misc/mac_via.c             |  4 ++--
>  hw/misc/macio/cuda.c          |  4 ++--
>  hw/misc/macio/macio.c         |  4 ++--
>  hw/misc/macio/pmu.c           |  4 ++--
>  hw/nubus/mac-nubus-bridge.c   |  2 +-
>  hw/nvme/ctrl.c                |  4 ++--
>  hw/nvme/subsys.c              |  3 +--
>  hw/pci-host/raven.c           |  4 ++--
>  hw/pci-host/versatile.c       |  6 +++---
>  hw/pci/pci.c                  | 30 +++++++++++++++---------------
>  hw/pci/pci_bridge.c           |  4 ++--
>  hw/ppc/spapr_vio.c            |  2 +-
>  hw/s390x/ap-bridge.c          |  2 +-
>  hw/s390x/css-bridge.c         |  2 +-
>  hw/s390x/event-facility.c     |  4 ++--
>  hw/s390x/s390-pci-bus.c       |  2 +-
>  hw/s390x/virtio-ccw.c         |  3 +--
>  hw/scsi/esp-pci.c             |  2 +-
>  hw/scsi/esp.c                 |  2 +-
>  hw/scsi/lsi53c895a.c          |  2 +-
>  hw/scsi/megasas.c             |  3 +--
>  hw/scsi/mptsas.c              |  2 +-
>  hw/scsi/scsi-bus.c            |  6 +++---
>  hw/scsi/spapr_vscsi.c         |  3 +--
>  hw/scsi/virtio-scsi.c         |  4 ++--
>  hw/scsi/vmw_pvscsi.c          |  3 +--
>  hw/sd/allwinner-sdhost.c      |  4 ++--
>  hw/sd/bcm2835_sdhost.c        |  4 ++--
>  hw/sd/pl181.c                 |  3 +--
>  hw/sd/pxa2xx_mmci.c           |  4 ++--
>  hw/sd/sdhci.c                 |  3 +--
>  hw/sd/ssi-sd.c                |  3 +--
>  hw/ssi/ssi.c                  |  2 +-
>  hw/usb/bus.c                  |  2 +-
>  hw/usb/dev-smartcard-reader.c |  3 +--
>  hw/usb/dev-storage-bot.c      |  3 +--
>  hw/usb/dev-storage-classic.c  |  4 ++--
>  hw/usb/dev-uas.c              |  3 +--
>  hw/virtio/virtio-mmio.c       |  3 +--
>  hw/virtio/virtio-pci.c        |  3 +--
>  hw/xen/xen-bus.c              |  2 +-
>  hw/xen/xen-legacy-backend.c   |  2 +-
>  70 files changed, 156 insertions(+), 142 deletions(-)
> 
> -- 
> 2.20.1



  parent reply	other threads:[~2021-09-23 15:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23 12:11 [PATCH 0/6] Improve consistency of bus init function names Peter Maydell
2021-09-23 12:11 ` [PATCH 1/6] scsi: Replace scsi_bus_new() with scsi_bus_init(), scsi_bus_init_named() Peter Maydell
2021-09-28 15:12   ` Paolo Bonzini
2021-09-23 12:11 ` [PATCH 2/6] ipack: Rename ipack_bus_new_inplace() to ipack_bus_init() Peter Maydell
2021-09-23 12:11 ` [PATCH 3/6] pci: Rename pci_root_bus_new_inplace() to pci_root_bus_init() Peter Maydell
2021-09-23 12:11 ` [PATCH 4/6] qbus: Rename qbus_create_inplace() to qbus_init() Peter Maydell
2021-09-23 12:11 ` [PATCH 5/6] qbus: Rename qbus_create() to qbus_new() Peter Maydell
2021-09-23 16:00   ` Corey Minyard
2021-09-23 12:11 ` [PATCH 6/6] ide: Rename ide_bus_new() to ide_bus_init() Peter Maydell
2021-09-23 18:26   ` John Snow
2021-09-23 13:36 ` [PATCH 0/6] Improve consistency of bus init function names Philippe Mathieu-Daudé
2021-09-23 15:38 ` Michael S. Tsirkin [this message]
2021-09-30  9:39 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210923113832-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=berto@igalia.com \
    --cc=ehabkost@redhat.com \
    --cc=fam@euphon.net \
    --cc=jsnow@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.