All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Liu <wl@xen.org>
To: Paul Durrant <paul@xen.org>
Cc: xen-devel@lists.xenproject.org,
	Paul Durrant <pdurrant@amazon.com>,
	Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>,
	Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
	Anthony PERARD <anthony.perard@citrix.com>
Subject: Re: [PATCH v5 01/23] xl / libxl: s/pcidev/pci and remove DEFINE_DEVICE_TYPE_STRUCT_X
Date: Fri, 4 Dec 2020 11:15:57 +0000	[thread overview]
Message-ID: <20201204111557.34ya4w4wvqvyonao@liuwe-devbox-debian-v2> (raw)
In-Reply-To: <20201204111326.5pxgqertdm3tk7y2@liuwe-devbox-debian-v2>

On Fri, Dec 04, 2020 at 11:13:26AM +0000, Wei Liu wrote:
> On Thu, Dec 03, 2020 at 02:25:12PM +0000, Paul Durrant wrote:
> > From: Paul Durrant <pdurrant@amazon.com>
> > 
> > The seemingly arbitrary use of 'pci' and 'pcidev' in the code in libxl_pci.c
> > is confusing and also compromises use of some macros used for other device
> > types. Indeed it seems that DEFINE_DEVICE_TYPE_STRUCT_X exists solely because
> > of this duality.
> > 
> > This patch purges use of 'pcidev' from the libxl code, allowing evaluation of
> > DEFINE_DEVICE_TYPE_STRUCT_X to be replaced with DEFINE_DEVICE_TYPE_STRUCT,
> > hence allowing removal of the former.
> > 
> > For consistency the xl and libs/util code is also modified, but in this case
> > it is purely cosmetic.
> > 
> > NOTE: Some of the more gross formatting errors (such as lack of spaces after
> >       keywords) that came into context have been fixed in libxl_pci.c.
> > 
> > Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> > Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> > ---
> > Cc: Ian Jackson <iwj@xenproject.org>
> > Cc: Wei Liu <wl@xen.org>
> > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > 
> 
> This is going to break libxl callers because the name "pcidev" is
> visible from the public header.
> 
> I agree this is confusing and inconsistent, but we didn't go extra
> length to maintain the inconsistency for no reason.
> 
> If you really want to change it, I won't stand in the way. In fact, I'm
> all for consistency. I think the flag you added should help alleviate
> the fallout. Also, you will need to submit patches to libvirt (the only
> user I know of) to make use of the flag and switch to the new name and
> then request such patch(es) be backported to all maintained version of
> libvirt.
> 
> See https://github.com/libvirt/libvirt/blob/0d05d51b715390e08cd112f83e03b6776412aaeb/src/libxl/libxl_conf.c#L2273

Pasted in the wrong line. This is the correct one I had in mind:

https://github.com/libvirt/libvirt/blob/0d05d51b715390e08cd112f83e03b6776412aaeb/src/libxl/libxl_conf.c#L2320

Wei.


  reply	other threads:[~2020-12-04 11:16 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03 14:25 [PATCH v5 00/23] xl / libxl: named PCI pass-through devices Paul Durrant
2020-12-03 14:25 ` [PATCH v5 01/23] xl / libxl: s/pcidev/pci and remove DEFINE_DEVICE_TYPE_STRUCT_X Paul Durrant
2020-12-04 11:13   ` Wei Liu
2020-12-04 11:15     ` Wei Liu [this message]
2020-12-04 11:19     ` Paul Durrant
2020-12-04 11:21       ` Wei Liu
2020-12-04 11:23         ` Paul Durrant
2020-12-07 16:18           ` Paul Durrant
2020-12-07 16:23             ` Durrant, Paul
2020-12-03 14:25 ` [PATCH v5 02/23] libxl: make libxl__device_list() work correctly for LIBXL__DEVICE_KIND_PCI Paul Durrant
2020-12-04 11:17   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 03/23] libxl: Make sure devices added by pci-attach are reflected in the config Paul Durrant
2020-12-04 11:18   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 04/23] libxl: add/recover 'rdm_policy' to/from PCI backend in xenstore Paul Durrant
2020-12-04 11:19   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 05/23] libxl: s/detatched/detached in libxl_pci.c Paul Durrant
2020-12-04 11:19   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 06/23] libxl: remove extraneous arguments to do_pci_remove() " Paul Durrant
2020-12-04 11:19   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 07/23] libxl: stop using aodev->device_config in libxl__device_pci_add() Paul Durrant
2020-12-04 11:22   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 08/23] libxl: generalise 'driver_path' xenstore access functions in libxl_pci.c Paul Durrant
2020-12-04 11:22   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 09/23] libxl: remove unnecessary check from libxl__device_pci_add() Paul Durrant
2020-12-04 11:22   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 10/23] libxl: remove get_all_assigned_devices() from libxl_pci.c Paul Durrant
2020-12-04 11:26   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 11/23] libxl: make sure callers of libxl_device_pci_list() free the list after use Paul Durrant
2020-12-04 11:28   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 12/23] libxl: add libxl_device_pci_assignable_list_free() Paul Durrant
2020-12-04 11:29   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 13/23] libxl: use COMPARE_PCI() macro is_pci_in_array() Paul Durrant
2020-12-04 11:29   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 14/23] docs/man: extract documentation of PCI_SPEC_STRING from the xl.cfg manpage Paul Durrant
2020-12-04 11:29   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 15/23] docs/man: improve documentation of PCI_SPEC_STRING Paul Durrant
2020-12-04 11:30   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 16/23] docs/man: fix xl(1) documentation for 'pci' operations Paul Durrant
2020-12-04 11:31   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 17/23] libxl: introduce 'libxl_pci_bdf' in the idl Paul Durrant
2020-12-04 11:31   ` Wei Liu
2020-12-04 13:07   ` Nick Rosbrook
2020-12-03 14:25 ` [PATCH v5 18/23] libxlu: introduce xlu_pci_parse_spec_string() Paul Durrant
2020-12-04 11:34   ` Wei Liu
2020-12-07 19:13     ` Paul Durrant
2020-12-03 14:25 ` [PATCH v5 19/23] libxl: modify libxl_device_pci_assignable_add/remove/list/list_free() Paul Durrant
2020-12-04 11:35   ` Wei Liu
2020-12-04 12:16     ` Paul Durrant
2020-12-03 14:25 ` [PATCH v5 20/23] docs/man: modify xl(1) in preparation for naming of assignable devices Paul Durrant
2020-12-04 11:35   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 21/23] xl / libxl: support " Paul Durrant
2020-12-04 11:37   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 22/23] docs/man: modify xl-pci-configuration(5) to add 'name' field to PCI_SPEC_STRING Paul Durrant
2020-12-04 11:38   ` Wei Liu
2020-12-03 14:25 ` [PATCH v5 23/23] xl / libxl: support 'xl pci-attach/detach' by name Paul Durrant
2020-12-04 11:38   ` Wei Liu

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=20201204111557.34ya4w4wvqvyonao@liuwe-devbox-debian-v2 \
    --to=wl@xen.org \
    --cc=anthony.perard@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=oleksandr_andrushchenko@epam.com \
    --cc=paul@xen.org \
    --cc=pdurrant@amazon.com \
    --cc=xen-devel@lists.xenproject.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.