Re: [Qemu-devel] [PATCH 1/2] linux-headers: update to pre-4.20 (kvm/queue)

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 9 Oct 2018 13:59:36 +0000
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Tue, Oct 09, 2018 at 03:42:27PM +0200, Vitaly Kuznetsov wrote:
> > Roman Kagan <rkagan@virtuozzo.com> writes:
> >   
> > > On Tue, Oct 09, 2018 at 03:01:12PM +0200, Vitaly Kuznetsov wrote:  
> > >> Roman Kagan <rkagan@virtuozzo.com> writes:
> > >>   
> > >> > On Mon, Oct 08, 2018 at 01:27:25PM +0200, Vitaly Kuznetsov wrote:  
> > >> >> This brings us three new defines: KVM_CAP_ARM_INJECT_SERROR_ESR,
> > >> >> KVM_CAP_MSR_PLATFORM_INFO and KVM_CAP_HYPERV_SEND_IPI.
> > >> >> 
> > >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > >> >> ---
> > >> >>  linux-headers/linux/kvm.h | 7 +++++--
> > >> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> > >> >> 
> > >> >> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> > >> >> index 66790724f1..043f1e58b6 100644
> > >> >> --- a/linux-headers/linux/kvm.h
> > >> >> +++ b/linux-headers/linux/kvm.h
> > >> >> @@ -9,7 +9,7 @@
> > >> >>   */
> > >> >>  
> > >> >>  #include <linux/types.h>
> > >> >> -
> > >> >> +#include <linux/compiler.h>
> > >> >>  #include <linux/ioctl.h>
> > >> >>  #include <asm/kvm.h>
> > >> >>  
> > >> >> @@ -481,7 +481,7 @@ struct kvm_dirty_log {
> > >> >>  	__u32 slot;
> > >> >>  	__u32 padding1;
> > >> >>  	union {
> > >> >> -		void *dirty_bitmap; /* one bit per page */
> > >> >> +		void __user *dirty_bitmap; /* one bit per page */  
> > >> >
> > >> > Hmm, I thought "__user" and friends are supposed to get stripped while
> > >> > "make headers_install".  
> > >> 
> > >> ... and of course I forgot about this, will send v2, thanks!  
> > >
> > > Are you trying to say you didn't follow the regular linux header
> > > harvesting process, which is to use scripts/update-linux-headers.sh
> > > (which does 'make headers_install' and then applies further sanitizing,
> > > so that problems like this wouldn't have popped up)?  I think you
> > > should, unless this is an interim patchset pending the merge of the
> > > corresponding KVM changes in the mainline kernel.  
> > 
> > Honestly I didn't know about the standard process but looking at the
> > output:
> > 
> > $ scripts/update-linux-headers.sh PATH/TO/LINUX
> > ...
> > $ git diff --stat
> >  include/standard-headers/linux/input.h |  9 +++++----
> >  linux-headers/asm-arm/kvm.h            | 13 +++++++++++++
> >  linux-headers/asm-arm64/kvm.h          | 13 +++++++++++++
> >  linux-headers/asm-s390/kvm.h           |  2 ++
> >  linux-headers/asm-x86/kvm.h            |  1 +
> >  linux-headers/linux/kvm.h              |  3 +++
> >  linux-headers/linux/vfio.h             |  2 ++
> >  linux-headers/linux/vhost.h            |  2 +-
> >  8 files changed, 40 insertions(+), 5 deletions(-)
> > 
> > I'm not sure if it would make sense to do this instead: the definition I
> > need is currently in kvm/queue and I'm not exactly sure that updating
> > other headers from there (e.g. linux/input.h) makes any sense.  
> 
> Another possible problem is that constants may change on their way from
> kvm/queue to mainline, so people tend to avoid pulling headers from
> anything but Linus' tree.
> 
> Anyway that's not my call.  Paolo, Eduardo, what's your take on this?

I'm neither Paolo nor Eduardo, but my take is that we can sync headers
against something that we can reasonably expect to end up in mainline
soon. So, I think syncing against current kvm/next would be fine (we're
in the late -rc phase), but I'd avoid syncing against kvm/queue.