All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: kvm-devel@lists.sourceforge.net
Cc: Avi Kivity <avi@qumranet.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [kvm-devel] [PATCH][RFC] KVM: prepare user interface for smp guests
Date: Mon, 30 Oct 2006 01:01:10 +0100	[thread overview]
Message-ID: <200610300101.11245.arnd@arndb.de> (raw)
In-Reply-To: <4544AD24.4040801@qumranet.com>

On Sunday 29 October 2006 14:31, Avi Kivity wrote:
> +       r = -EEXIST;
> +       if (vcpu->vmcs)
> +               goto out_unlock;
> +
> +       r = -ENOMEM;
> +       filp = get_empty_filp();
> +       if (!filp)
> +               goto out_unlock;
> +
> +       r = get_unused_fd();
> +       if (r < 0)
> +               goto out_free_filp;
> +
> +       fd = r;
>  
>         vcpu->host_fx_image = (char*)ALIGN((hva_t)vcpu->fx_buf,
>                                            FX_IMAGE_ALIGN);
> @@ -1372,10 +1428,25 @@ static int kvm_dev_ioctl_create_vcpu(str
>         if (r < 0)
>                 goto out_free_vcpus;
>  
> -       return 0;
> +       filp->f_dentry = dget(kvm_filp->f_dentry);
> +       filp->f_vfsmnt = mntget(kvm_filp->f_vfsmnt);
> +       filp->f_mode = kvm_filp->f_mode;
> +       allow_write_access(filp);
> +       cdev_get(filp->f_dentry->d_inode->i_cdev);
> +       kvm_get(kvm);
> +       filp->f_op = fops_get(&kvm_vcpu_ops);
> +       filp->private_data = vcpu;
> +       fd_install(fd, filp);

Separating the objects into different file descriptors sounds like a
good idea, but reusing an open dentry/inode with a new file and different
file operations is a rather unusual way to do it. Your concept of allocating
a new context on each open is already weird, but there have been other
examples of that before.

I'd suggest going to a syscall-based model with your own file system right
away, even if you don't use the spufs approach but something in the middle:

* You do a trivial nonmountable new file system with anonymous objects,
  similar to eventpollfs, and hand out file descriptors to inodes in it,
  for both the kvm and the vcpu objects.
* You replace the syscall you'd normally use to hand out a new kvm instance
  with an ioctl on /dev/kvm, and don't allow any other operations on that
  device.

This would be a much more consistant object model, compared with other
generic kernel functionality that is not bound to an actual device.
You still have all the flexibility of a loadable module without core
kernel changes for the development phase, and can easily switch to real
syscalls when merging it into mainline.

I really think that a small number of syscalls is where you should be
heading, whether you use a file system or not, but I understand that
ioctls are convenient for development.

	Arnd <><

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc: linux-kernel <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH][RFC] KVM: prepare user interface for smp guests
Date: Mon, 30 Oct 2006 01:01:10 +0100	[thread overview]
Message-ID: <200610300101.11245.arnd@arndb.de> (raw)
In-Reply-To: <4544AD24.4040801-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

On Sunday 29 October 2006 14:31, Avi Kivity wrote:
> +       r = -EEXIST;
> +       if (vcpu->vmcs)
> +               goto out_unlock;
> +
> +       r = -ENOMEM;
> +       filp = get_empty_filp();
> +       if (!filp)
> +               goto out_unlock;
> +
> +       r = get_unused_fd();
> +       if (r < 0)
> +               goto out_free_filp;
> +
> +       fd = r;
>  
>         vcpu->host_fx_image = (char*)ALIGN((hva_t)vcpu->fx_buf,
>                                            FX_IMAGE_ALIGN);
> @@ -1372,10 +1428,25 @@ static int kvm_dev_ioctl_create_vcpu(str
>         if (r < 0)
>                 goto out_free_vcpus;
>  
> -       return 0;
> +       filp->f_dentry = dget(kvm_filp->f_dentry);
> +       filp->f_vfsmnt = mntget(kvm_filp->f_vfsmnt);
> +       filp->f_mode = kvm_filp->f_mode;
> +       allow_write_access(filp);
> +       cdev_get(filp->f_dentry->d_inode->i_cdev);
> +       kvm_get(kvm);
> +       filp->f_op = fops_get(&kvm_vcpu_ops);
> +       filp->private_data = vcpu;
> +       fd_install(fd, filp);

Separating the objects into different file descriptors sounds like a
good idea, but reusing an open dentry/inode with a new file and different
file operations is a rather unusual way to do it. Your concept of allocating
a new context on each open is already weird, but there have been other
examples of that before.

I'd suggest going to a syscall-based model with your own file system right
away, even if you don't use the spufs approach but something in the middle:

* You do a trivial nonmountable new file system with anonymous objects,
  similar to eventpollfs, and hand out file descriptors to inodes in it,
  for both the kvm and the vcpu objects.
* You replace the syscall you'd normally use to hand out a new kvm instance
  with an ioctl on /dev/kvm, and don't allow any other operations on that
  device.

This would be a much more consistant object model, compared with other
generic kernel functionality that is not bound to an actual device.
You still have all the flexibility of a loadable module without core
kernel changes for the development phase, and can easily switch to real
syscalls when merging it into mainline.

I really think that a small number of syscalls is where you should be
heading, whether you use a file system or not, but I understand that
ioctls are convenient for development.

	Arnd <><

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

  reply	other threads:[~2006-10-30  0:01 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-29 13:31 [PATCH][RFC] KVM: prepare user interface for smp guests Avi Kivity
2006-10-29 13:31 ` Avi Kivity
2006-10-30  0:01 ` Arnd Bergmann [this message]
2006-10-30  0:01   ` Arnd Bergmann
2006-10-30  9:08   ` [kvm-devel] " Avi Kivity
2006-10-30  9:08     ` Avi Kivity
2006-10-30 12:19     ` [kvm-devel] " Arnd Bergmann
2006-10-30 12:19       ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200610300101.11245.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=avi@qumranet.com \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.