Re: [PATCH] kvm tools: Add initial virtio-scsi support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 9, 2012 at 3:51 AM, Asias He <asias.hejun@xxxxxxxxx> wrote:
> This patch brings virito-scsi support to kvm tool.
>
> With the introduce of tcm_vhost (vhost-scsi)
>
>    tcm_vhost: Initial merge for vhost level target fabric driver
>
> we can implement virito-scsi by simply having vhost-scsi to handle the
> SCSI command.
>
> Howto use:
> 1) Setup the tcm_vhost target through /sys/kernel/config
>
>    [Stefan Hajnoczi, Thanks for the script to setup tcm_vhost]
>
>    ** Setup wwpn and tpgt
>    $ wwpn="naa.0"
>    $ tpgt=/sys/kernel/config/target/vhost/$wwpn/tpgt_0
>    $ nexus=$tpgt/nexus
>    $ mkdir -p $tpgt
>    $ echo -n $wwpn > $nexus
>
>    ** Setup lun using /dev/ram
>    $ n=0
>    $ lun=$tpgt/lun/lun_${n}
>    $ data=/sys/kernel/config/target/core/iblock_0/data_${n}
>    $ ram=/dev/ram${n}
>    $ mkdir -p $lun
>    $ mkdir -p $data
>    $ echo -n udev_path=${ram} > $data/control
>    $ echo -n 1 > $data/enable
>    $ ln -s $data $lun
>
> 2) Run kvm tool with the new disk option '-d scsi:$wwpn:$tpgt', e.g
>    $ lkvm run -k /boot/bzImage -d ~/img/sid.img -d scsi:naa.0:0
>
> Signed-off-by: Asias He <asias.hejun@xxxxxxxxx>
> Cc: Nicholas A. Bellinger <nab@xxxxxxxxxxxxxxx>
> Cc: Stefan Hajnoczi <stefanha@xxxxxxxxxxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>

I've included some comments below but overall, looks good to me. Sasha?

> ---
> diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
> index 7ae17f8..54e4047 100644
> --- a/tools/kvm/include/kvm/disk-image.h
> +++ b/tools/kvm/include/kvm/disk-image.h
> @@ -41,6 +41,8 @@ struct disk_image_operations {
>
>  struct disk_image_params {
>         const char *filename;
> +       const char *wwpn;
> +       const char *tpgt;

Maybe it's just me but "wwpn" and "tpgt" really could use a comment
explaining what they are...

>         bool readonly;
>         bool direct;
>  };
> @@ -57,6 +59,8 @@ struct disk_image {
>  #ifdef CONFIG_HAS_AIO
>         io_context_t                    ctx;
>  #endif
> +       const char                      *wwpn;
> +       const char                      *tpgt;
>  };
>
>  struct disk_image *disk_image__open(const char *filename, bool readonly, bool direct);
> diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
> index 1fb969f..740442a 100644
> --- a/tools/kvm/virtio/blk.c
> +++ b/tools/kvm/virtio/blk.c
> @@ -290,6 +290,8 @@ int virtio_blk__init(struct kvm *kvm)
>         int i, r = 0;
>
>         for (i = 0; i < kvm->nr_disks; i++) {
> +               if (kvm->disks[i]->wwpn)
> +                       continue;
>                 r = virtio_blk__init_one(kvm, kvm->disks[i]);
>                 if (r < 0)
>                         goto cleanup;
> diff --git a/tools/kvm/virtio/scsi.c b/tools/kvm/virtio/scsi.c
> new file mode 100644
> index 0000000..5bcb00c
> --- /dev/null
> +++ b/tools/kvm/virtio/scsi.c
> @@ -0,0 +1,332 @@
> +#include "kvm/virtio-scsi.h"
> +#include "kvm/virtio-pci-dev.h"
> +#include "kvm/disk-image.h"
> +#include "kvm/kvm.h"
> +#include "kvm/pci.h"
> +#include "kvm/ioeventfd.h"
> +#include "kvm/guest_compat.h"
> +#include "kvm/virtio-pci.h"
> +#include "kvm/virtio.h"
> +
> +#include <linux/kernel.h>
> +#include <linux/virtio_scsi.h>
> +#include <linux/vhost.h>
> +
> +/*----------------------------------------------------*/
> +/* TODO: Remove this when tcm_vhost goes upstream */
> +#define TRANSPORT_IQN_LEN              224
> +#define VHOST_SCSI_ABI_VERSION         0
> +struct vhost_scsi_target {
> +       int abi_version;
> +       unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
> +       unsigned short vhost_tpgt;
> +};
> +/* VHOST_SCSI specific defines */
> +#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> +#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> +/*----------------------------------------------------*/

You might as well move the above block into a small helper header file.

> +
> +
> +#define VIRTIO_SCSI_QUEUE_SIZE         128
> +#define NUM_VIRT_QUEUES                        3
> +
> +static LIST_HEAD(sdevs);
> +static int compat_id = -1;
> +
> +struct scsi_dev {
> +       struct virt_queue               vqs[NUM_VIRT_QUEUES];
> +       struct virtio_scsi_config       scsi_config;
> +       struct vhost_scsi_target        target;
> +       u32                             features;
> +       int                             vhost_fd;
> +       struct virtio_device            vdev;
> +       struct list_head                list;
> +       struct kvm                      *kvm;
> +};
> +
> +static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
> +{
> +       struct scsi_dev *sdev = dev;
> +
> +       ((u8 *)(&sdev->scsi_config))[offset] = data;

Can you introduce a helper function for this, please?

> +}
> +
> +static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
> +{
> +       struct scsi_dev *sdev = dev;
> +
> +       return ((u8 *)(&sdev->scsi_config))[offset];

Ditto.

> +}
> +
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux