mbox series

[0/4] block/xen_disk: legacy code removal and cleanup

Message ID 1525089699-13411-1-git-send-email-paul.durrant@citrix.com
Headers show
Series block/xen_disk: legacy code removal and cleanup | expand

Message

Paul Durrant April 30, 2018, 12:01 p.m. UTC
The grant copy operation was added to libxengnttab in Xen 4.8.0 (released
nearly 18 months ago) but the xen_disk PV backend QEMU is still carrying
a significant amount of code purely to remain compatible with older
versions of Xen.

As can be inferred from the diff stats below, removing this support for
older versions of Xen from QEMU reduces the size of the xen_disk source by
more than 350 lines (~25%). The majority of this is done in patches #1
and #2. Further simplifications are made in patch #3 and then some cosmetic
work is done in patch #4.

Paul Durrant (4):
  block/xen_disk: remove persistent grant code
  block/xen_disk: remove use of grant map/unmap
  block/xen_disk: use a single entry iovec
  block/xen_disk: be consistent with use of xendev and blkdev->xendev

 hw/block/xen_disk.c | 590 ++++++++++------------------------------------------
 1 file changed, 109 insertions(+), 481 deletions(-)
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

Comments

Anthony PERARD May 2, 2018, 3:58 p.m. UTC | #1
On Mon, Apr 30, 2018 at 01:01:35PM +0100, Paul Durrant wrote:
> The grant copy operation was added to libxengnttab in Xen 4.8.0 (released
> nearly 18 months ago) but the xen_disk PV backend QEMU is still carrying
> a significant amount of code purely to remain compatible with older
> versions of Xen.
> 
> As can be inferred from the diff stats below, removing this support for
> older versions of Xen from QEMU reduces the size of the xen_disk source by
> more than 350 lines (~25%). The majority of this is done in patches #1
> and #2. Further simplifications are made in patch #3 and then some cosmetic
> work is done in patch #4.

FIY, I don't like this patch series. We've been checking that QEMU
builds against older version. I've check that it builds against 4.5 and
newer.

Also the fact that FreeBSD doesn't have support for grant copy probably
mean that it is too soon to remove the compatibility code from qemu.

Regards,
Paul Durrant May 2, 2018, 4:03 p.m. UTC | #2
> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 02 May 2018 16:58
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>; Kevin Wolf
> <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>
> Subject: Re: [PATCH 0/4] block/xen_disk: legacy code removal and cleanup
> 
> On Mon, Apr 30, 2018 at 01:01:35PM +0100, Paul Durrant wrote:
> > The grant copy operation was added to libxengnttab in Xen 4.8.0 (released
> > nearly 18 months ago) but the xen_disk PV backend QEMU is still carrying
> > a significant amount of code purely to remain compatible with older
> > versions of Xen.
> >
> > As can be inferred from the diff stats below, removing this support for
> > older versions of Xen from QEMU reduces the size of the xen_disk source
> by
> > more than 350 lines (~25%). The majority of this is done in patches #1
> > and #2. Further simplifications are made in patch #3 and then some
> cosmetic
> > work is done in patch #4.
> 
> FIY, I don't like this patch series. We've been checking that QEMU
> builds against older version. I've check that it builds against 4.5 and
> newer.
> 

Ok, I can grant copy emulation in QEMU then should it not exist for the particular Xen/OS combo.

> Also the fact that FreeBSD doesn't have support for grant copy probably
> mean that it is too soon to remove the compatibility code from qemu.
> 

On another thread I'd already agreed to emulate grant copy in libxengnttab for those OS where it is not supported, but if you prefer it to be in QEMU I'll put it there.

  Paul

> Regards,
> 
> --
> Anthony PERARD
Anthony PERARD May 3, 2018, 9:55 a.m. UTC | #3
On Wed, May 02, 2018 at 05:03:09PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > Sent: 02 May 2018 16:58
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> > devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>; Kevin Wolf
> > <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>
> > Subject: Re: [PATCH 0/4] block/xen_disk: legacy code removal and cleanup
> > 
> > On Mon, Apr 30, 2018 at 01:01:35PM +0100, Paul Durrant wrote:
> > > The grant copy operation was added to libxengnttab in Xen 4.8.0 (released
> > > nearly 18 months ago) but the xen_disk PV backend QEMU is still carrying
> > > a significant amount of code purely to remain compatible with older
> > > versions of Xen.
> > >
> > > As can be inferred from the diff stats below, removing this support for
> > > older versions of Xen from QEMU reduces the size of the xen_disk source
> > by
> > > more than 350 lines (~25%). The majority of this is done in patches #1
> > > and #2. Further simplifications are made in patch #3 and then some
> > cosmetic
> > > work is done in patch #4.
> > 
> > FIY, I don't like this patch series. We've been checking that QEMU
> > builds against older version. I've check that it builds against 4.5 and
> > newer.
> > 
> 
> Ok, I can grant copy emulation in QEMU then should it not exist for the particular Xen/OS combo.
> 
> > Also the fact that FreeBSD doesn't have support for grant copy probably
> > mean that it is too soon to remove the compatibility code from qemu.
> > 
> 
> On another thread I'd already agreed to emulate grant copy in libxengnttab for those OS where it is not supported, but if you prefer it to be in QEMU I'll put it there.

Yes, I think it will be better from QEMU point of view.

Thanks,