Re: [PATCH] KVM: selftests: Print a message if /dev/kvm is missing

From: Andrew Jones <drjones@redhat.com>
To: David Matlack <dmatlack@google.com>
Cc: kvm list <kvm@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Aaron Lewis <aaronlewis@google.com>
Subject: Re: [PATCH] KVM: selftests: Print a message if /dev/kvm is missing
Date: Mon, 10 May 2021 08:32:36 +0200	[thread overview]
Message-ID: <20210510063236.5ekmlulazelvl2s6@gator> (raw)
In-Reply-To: <CALzav=dk_Z=hQE1Bjpfg8B3su7h2Jvk6RZoEFqBn+qqxmwzHMQ@mail.gmail.com>

On Fri, May 07, 2021 at 01:51:45PM -0700, David Matlack wrote:
> > >  static void vm_open(struct kvm_vm *vm, int perm)
> > >  {
> > > -     vm->kvm_fd = open(KVM_DEV_PATH, perm);
> >
> > I don't think we should change this one, otherwise the user provided
> > perms are ignored.
> 
> Good catch. I don't see any reason to exclude this case, but we do need
> to pass `perm` down to open_kvm_dev_path_or_exit().
>

I've reviewed v2 and gave it an r-b, since I don't have overly strong
opinion about this, but I actually liked that open_kvm_dev_path_or_exit()
didn't take any arguments. To handle this case I would have either left
it open coded, like it was, or created something like

int _open_kvm_dev_path_or_exit(int flags)
{
   int fd = open(KVM_DEV_PATH, flags);
   if (fd < 0)
     ...exit skip...
   return fd;
}

int open_kvm_dev_path_or_exit(void)
{
  return _open_kvm_dev_path_or_exit(O_RDONLY);
}

Thanks,
drew