From: Alex Williamson <alex.williamson@redhat.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: Liu Yi L <yi.l.liu@intel.com>,
kwankhede@nvidia.com, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org, kevin.tian@intel.com, joro@8bytes.org,
peterx@redhat.com, baolu.lu@linux.intel.com
Subject: Re: [PATCH v4 05/12] vfio_pci: duplicate vfio_pci.c
Date: Wed, 15 Jan 2020 08:12:24 -0700 [thread overview]
Message-ID: <20200115081224.215a994c@w520.home> (raw)
In-Reply-To: <20200115120300.24874a37.cohuck@redhat.com>
On Wed, 15 Jan 2020 12:03:00 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 7 Jan 2020 20:01:42 +0800
> Liu Yi L <yi.l.liu@intel.com> wrote:
>
> > This patch has no code change, just a file copy. In following patches,
> > vfio_pci_common.c will be modified to only include the common functions
> > and related static functions in original vfio_pci.c. Meanwhile, vfio_pci.c
> > will be modified to only include vfio-pci module specific codes.
> >
> > Cc: Kevin Tian <kevin.tian@intel.com>
> > Cc: Lu Baolu <baolu.lu@linux.intel.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > ---
> > drivers/vfio/pci/vfio_pci_common.c | 1708 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 1708 insertions(+)
> > create mode 100644 drivers/vfio/pci/vfio_pci_common.c
>
> This whole procedure of "let's copy the file and rip out unneeded stuff
> later" looks very ugly to me, especially if I'd come across it in the
> future, e.g. during a bisect. This patch only adds a file that is not
> compiled, and later changes will be "rip out unwanted stuff from
> vfio_pci_common.c" instead of the more positive "move common stuff to
> vfio_pci_common.c". I think refactoring/moving interfaces/code that it
> makes sense to share makes this more reviewable, both now and in the
> future.
I think this comes largely at my request from previous reviews. It's
very easy to apply this patch and diff the files to see that nothing
has changed, then review the subsequent patch to see that code is only
added or removed to check that there are no actual code changes. If we
just selectively move code then I think it's left to our inspection to
verify nothing has changed. Maybe this is a dummy step in a bisect,
but I don't see that you lose any granularity. Thanks,
Alex
next prev parent reply other threads:[~2020-01-15 15:12 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-07 12:01 [PATCH v4 00/12] vfio_pci: wrap pci device as a mediated device Liu Yi L
2020-01-07 12:01 ` [PATCH v4 01/12] vfio_pci: refine user config reference in vfio-pci module Liu Yi L
2020-01-09 22:48 ` Alex Williamson
2020-01-16 12:19 ` Liu, Yi L
2020-01-07 12:01 ` [PATCH v4 02/12] vfio_pci: move vfio_pci_is_vga/vfio_vga_disabled to header file Liu Yi L
2020-01-15 10:43 ` Cornelia Huck
2020-01-16 12:46 ` Liu, Yi L
2020-01-07 12:01 ` [PATCH v4 03/12] vfio_pci: refine vfio_pci_driver reference in vfio_pci.c Liu Yi L
2020-01-09 22:48 ` Alex Williamson
2020-01-10 7:35 ` Liu, Yi L
2020-01-07 12:01 ` [PATCH v4 04/12] vfio_pci: make common functions be extern Liu Yi L
2020-01-15 10:56 ` Cornelia Huck
2020-01-16 12:48 ` Liu, Yi L
2020-01-07 12:01 ` [PATCH v4 05/12] vfio_pci: duplicate vfio_pci.c Liu Yi L
2020-01-15 11:03 ` Cornelia Huck
2020-01-15 15:12 ` Alex Williamson [this message]
2020-01-07 12:01 ` [PATCH v4 06/12] vfio_pci: shrink vfio_pci_common.c Liu Yi L
2020-01-07 12:01 ` [PATCH v4 07/12] vfio_pci: shrink vfio_pci.c Liu Yi L
2020-01-08 11:24 ` kbuild test robot
2020-01-09 22:48 ` Alex Williamson
2020-01-16 12:42 ` Liu, Yi L
2020-01-07 12:01 ` [PATCH v4 08/12] vfio_pci: duplicate vfio_pci_private.h to include/linux Liu Yi L
2020-01-07 12:01 ` [PATCH v4 09/12] vfio: split vfio_pci_private.h into two files Liu Yi L
2020-01-09 22:48 ` Alex Williamson
2020-01-16 11:59 ` Liu, Yi L
2020-01-07 12:01 ` [PATCH v4 10/12] vfio: build vfio_pci_common.c into a kernel module Liu Yi L
2020-01-07 12:01 ` [PATCH v4 11/12] samples: add vfio-mdev-pci driver Liu Yi L
2020-01-09 22:48 ` Alex Williamson
2020-01-16 12:33 ` Liu, Yi L
2020-01-16 21:24 ` Alex Williamson
2020-01-18 14:25 ` Liu, Yi L
2020-01-20 21:07 ` Alex Williamson
2020-01-21 7:43 ` Tian, Kevin
2020-01-21 8:43 ` Yan Zhao
2020-01-21 20:04 ` Alex Williamson
2020-01-21 21:54 ` Yan Zhao
2020-01-23 23:33 ` Alex Williamson
2020-01-31 2:26 ` Yan Zhao
2020-01-15 12:30 ` Cornelia Huck
2020-01-16 13:23 ` Liu, Yi L
2020-01-16 17:40 ` Cornelia Huck
2020-01-18 14:23 ` Liu, Yi L
2020-01-20 8:55 ` Cornelia Huck
2020-01-07 12:01 ` [PATCH v4 12/12] samples: refine " Liu Yi L
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=20200115081224.215a994c@w520.home \
--to=alex.williamson@redhat.com \
--cc=baolu.lu@linux.intel.com \
--cc=cohuck@redhat.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterx@redhat.com \
--cc=yi.l.liu@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).