WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [kvm-devel] [PATCH] Lguest implemention of virtio draft

To: kvm-devel@xxxxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] Re: [kvm-devel] [PATCH] Lguest implemention of virtio draft III
From: Arnd Bergmann <arnd@xxxxxxxx>
Date: Sat, 16 Jun 2007 15:50:52 +0200
Cc: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>, Xen Mailing List <xen-devel@xxxxxxxxxxxxxxxxxxx>, "jmk@xxxxxxxxxxxxxxxxxxx" <jmk@xxxxxxxxxxxxxxxxxxx>, Christian Borntraeger <cborntra@xxxxxxxxxx>, Rusty Russell <rusty@xxxxxxxxxxxxxxx>, virtualization <virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx>, Latchesar Ionkov <lionkov@xxxxxxxx>, Suzanne McIntosh <skranjac@xxxxxxxxxx>, Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
Delivery-date: Tue, 19 Jun 2007 08:57:03 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1182000514.6237.273.camel@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1181217762.14054.192.camel@xxxxxxxxxxxxxxxxxxxxx> <1181999920.6237.263.camel@xxxxxxxxxxxxxxxxxxxxx> <1182000514.6237.273.camel@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.6
On Saturday 16 June 2007, Rusty Russell wrote:
> This is a bonus patch for those wondering how a virtio implementation
> can look.

Thanks, I assumed it was something like this, but having the code
in front of me makes it a lot easier to comment on it.

> +static struct lguest_driver lguest_virtnet_drv = {
> +     .name = "lguestvirtnet",
> +     .owner = THIS_MODULE,
> +     .device_type = LGUEST_DEVICE_T_VIRTNET,
> +     .probe = lguest_virtnet_probe,
> +};
> +
> +static __init int lguest_virtnet_init(void)
> +{
> +     return register_lguest_driver(&lguest_virtnet_drv);
> +}
> +device_initcall(lguest_virtnet_init);
> +
> +/* Example block driver code. */
> +#include <linux/virtio_blk.h>
> +#include <linux/genhd.h>
> +#include <linux/blkdev.h>
> +static int lguest_virtblk_probe(struct lguest_device *lgdev)
> +{
> +     struct lguest_virtio_device *lgv;
> +     struct gendisk *disk;
> +     unsigned long sectors;
> +     int err;
> +
> +     lgv = lg_new_virtio(lgdev);
> +     if (!lgv)
> +             return -ENOMEM;
> +
> +     /* Page is initially used to pass capacity. */
> +     sectors = *(unsigned long *)lgv->in.desc;
> +     *(unsigned long *)lgv->in.desc = 0;
> +
> +     lgv->priv = disk = virtblk_probe(&lgv->vdev);
> +     if (IS_ERR(lgv->priv)) {
> +             err = PTR_ERR(lgv->priv);
> +             goto destroy;
> +     }
> +     set_capacity(disk, sectors);
> +     blk_queue_max_hw_segments(disk->queue, NUM_DESCS-1);
> +
> +     err = lg_setup_interrupt(lgv, disk->disk_name);
> +     if (err)
> +             goto unprobe;
> +     add_disk(disk);
> +     return 0;
> +
> +unprobe:
> +     virtblk_remove(disk);
> +destroy:
> +     lg_destroy_virtio(lgv);
> +     return err;
> +}
> +
> +static struct lguest_driver lguest_virtblk_drv = {
> +     .name = "lguestvirtblk",
> +     .owner = THIS_MODULE,
> +     .device_type = LGUEST_DEVICE_T_VIRTBLK,
> +     .probe = lguest_virtblk_probe,
> +};
> +
> +static __init int lguest_virtblk_init(void)
> +{
> +     return register_lguest_driver(&lguest_virtblk_drv);
> +}
> +device_initcall(lguest_virtblk_init);

This is the part that I find puzzling. My idea would be that the drivers
and devices you register are _not_ specific to the type of virtio bus.

The problem I see with your approach is that drivers for specific devices
can't be loaded as separate modules, right now all of them have to be
loaded or statically linked into the kernel in order to load the
lguest_virtio module.

I'm sure you know how this works for other buses, but I'll explain anyway
so that everyone knows what I'm talking about. What I have in mind is
to have a single 'struct bus_type virtio_bus_type', which is used by
all virtio implementations. The 'struct virtio_device' is derived from
'struct device', meaning that a 'struct device' is embedded in it.
Each virtio implementation can either use the virtio_device directly,
or derive its own kvm_virtio_device/lguest_virtio_device/... from it.
When lguest scans its devices, it registers the lguest_virtio_device
with the generic virtio_bus_type, not an lguest specific one.
Similarly, the block/net/... drivers each register themselves as
a virtio_driver with the virtio_bus_type, not with an implementation
specific guest.

Do you see a problem with the approach that I described here, or did
you just not bother implementing it so far?

        Arnd <><

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>