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

[David Matlack <dmatlack@google.com>]

On Sun, May 9, 2021 at 11:32 PM Andrew Jones <drjones@redhat.com> wrote:
>
> 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);
> }

I agree so long as hiding O_RDONLY does not decrease code readability.
I could not find any place in KVM where the R/W permissions on
/dev/kvm played a role, so I'm inclined to agree it's better to hide
the permissions.

I'll send another version with this change along with the comment fix.

Thanks.

>
> Thanks,
> drew
>