Re: [PATCH 00/30] virtiofs daemon (base)

[Daniel P . Berrangé]

On Thu, Oct 24, 2019 at 12:14:39PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (address@hidden) wrote:
> > On Thu, Oct 24, 2019 at 06:59:33AM -0400, Michael S. Tsirkin wrote:
> > > On Mon, Oct 21, 2019 at 03:33:57PM +0100, Dr. David Alan Gilbert wrote:
> > > > * address@hidden (address@hidden) wrote:
> > > > > Patchew URL: https://patchew.org/QEMU/address@hidden/
> > > > > 
> > > > > 
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > This series seems to have some coding style problems. See output 
> > > > > below for
> > > > > more information:
> > > > > 
> > > > > Subject: [PATCH 00/30] virtiofs daemon (base)
> > > > > Type: series
> > > > > Message-id: address@hidden
> > > > > 
> > > > > === TEST SCRIPT BEGIN ===
> > > > > #!/bin/bash
> > > > > git rev-parse base > /dev/null || exit 0
> > > > > git config --local diff.renamelimit 0
> > > > > git config --local diff.renames True
> > > > > git config --local diff.algorithm histogram
> > > > > ./scripts/checkpatch.pl --mailback base..
> > > > 
> > > > Expecting checkpatch to be broken here; most of the files
> > > > follow FUSE's formatting.
> > > > 
> > > > Dave
> > > 
> > > I wonder what do others think about this.
> > > One problem with such inconsistencies is that people tend to copy code
> > > around, which tends to result in a mess.
> > 
> > IIUC, most of this code is simpy copied as-is from the fuse or linux
> > git repos. I'm wondering what the intention is for it long term ?
> > 
> > For header files, I would have expected us to be able to compile against
> > the -devel package provided by the kernel or fuse packages. I can
> > understand if we want to import the headers if the VSOCK additions to
> > them are not yet widely available in distros though. If this is the case
> > we should put a time limit on how long we'd keep these copied headers
> > around for before dropping them. It would be fine to violate QEMU coding
> > style in this case as its not code QEMU would "maintain" long term - just
> > a read-only import.
> 
> The headers are really two types;  one are external definitions, the
> other are internal parts of libfuse.  I'd expect to keep the internal
> parts long term; teh external parts hmm; where would we pull them in
> externally from?

The fuse3-devel RPM on Fedora has some, but not all, of the fuse headers
the patches copy in. Not sure if that's enough though.

The kernel-devel RPM has fuse.h which seems to match fuse_kernel.h header
being imported.

Obviously that would mean a configure check to see if the required
headers exist or not & are new enough for VSOCK

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|