[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug
From: |
Kelvin Wang |
Subject: |
Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug |
Date: |
Thu, 24 May 2012 13:43:26 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, May 23, 2012 at 05:12:16PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 23, 2012 at 09:52:06PM +0800, Kelvin Wang wrote:
> > Support the virtio-scsi-pci adapter hot-plug. However, this patch can only
> > make
> > adapter hot-plugable. More effort is needed for LUN hot-plug. Actually,
> > that is
> > the most valuable feature to virtio-scsi.
> >
> > Following the steps as follows can give an intuitive understanding of this
> > feature after applying the patch to QEMU v1.1.0-rc3.
> >
> > 1, Generate a image file:
> > qemu-img create -f qcow2 test.img 2G
> >
> > 2, Run qemu with the option -monitor.
> >
> > 3, In the guest, insert necessary modules:
> > for m in acpiphp pci_hotplug; do sudo modprobe ${m}; done
> >
> > 4, In the qemu monitor,hot add a virtio-scsi-pci adapter:
> > (qemu)pci_add auto storage if=virtio-scsi-pci
> >
> > 5, Check whether the controller was added:
> > Guest: lspci
> > Qemu: (qemu)info qtree
> >
> > Signed-off-by: Kelvin Wang <address@hidden>
> > Signed-off-by: Sheng Liu <address@hidden>
>
> NAK
>
> Do not use pci_add. It is a compatibility command.
> Use the new style device_add.
> Same for if=.
>
> I think you won't need any changes then?
OK, Thanks!
>
> > ---
> > blockdev.c | 19 +++++++++++++++----
> > blockdev.h | 3 ++-
> > hw/pci-hotplug.c | 15 +++++++++++++++
> > 3 files changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 67895b2..ecb82bf 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -31,6 +31,7 @@ static const char *const if_name[IF_COUNT] = {
> > [IF_MTD] = "mtd",
> > [IF_SD] = "sd",
> > [IF_VIRTIO] = "virtio",
> > + [IF_VIRTIO_SCSI] = "virtio-scsi",
>
> weird indentation
>
> > [IF_XEN] = "xen",
> > };
> >
>
> I think blockdev is a legacy thing. You are
> supposed to use the new style device-add. No?
>
> > @@ -436,7 +437,8 @@ DriveInfo *drive_init(QemuOpts *opts, int
> > default_to_scsi)
> >
> > on_write_error = BLOCK_ERR_STOP_ENOSPC;
> > if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
> > - if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type
> > != IF_NONE) {
> > + if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO &&
> > + type != IF_VIRTIO_SCSI && type != IF_NONE) {
>
> code dupicated here and below. add a function?
>
> > error_report("werror is not supported by this bus type");
> > return NULL;
> > }
> > @@ -449,7 +451,8 @@ DriveInfo *drive_init(QemuOpts *opts, int
> > default_to_scsi)
> >
> > on_read_error = BLOCK_ERR_REPORT;
> > if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
> > - if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && type
> > != IF_NONE) {
> > + if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI &&
> > + IF_VIRTIO_SCSI && type != IF_NONE) {
>
> typo? IF_VIRTIO_SCSI is always != 0 ...
Ouch, ashamed for my carelessness.
>
> > error_report("rerror is not supported by this bus type");
> > return NULL;
> > }
> > @@ -461,7 +464,7 @@ DriveInfo *drive_init(QemuOpts *opts, int
> > default_to_scsi)
> > }
> >
> > if ((devaddr = qemu_opt_get(opts, "addr")) != NULL) {
> > - if (type != IF_VIRTIO) {
> > + if (type != IF_VIRTIO || type != IF_VIRTIO_SCSI) {
> > error_report("addr is not supported by this bus type");
> > return NULL;
> > }
> > @@ -579,6 +582,14 @@ DriveInfo *drive_init(QemuOpts *opts, int
> > default_to_scsi)
> > if (devaddr)
> > qemu_opt_set(opts, "addr", devaddr);
> > break;
> > + case IF_VIRTIO_SCSI:
> > + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
> > + qemu_opt_set(opts, "driver", "virtio-scsi-pci");
> > + qemu_opt_set(opts, "drive", dinfo->id);
> > + if (devaddr) {
> > + qemu_opt_set(opts, "addr", devaddr);
> > + }
> > + break;
> > default:
> > abort();
> > }
> > @@ -604,7 +615,7 @@ DriveInfo *drive_init(QemuOpts *opts, int
> > default_to_scsi)
> > ro = 1;
> > } else if (ro == 1) {
> > if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> > - type != IF_NONE && type != IF_PFLASH) {
> > + type != IF_NONE && type != IF_PFLASH && IF_VIRTIO_SCSI) {
> > error_report("readonly not supported by this bus type");
> > goto err;
> > }
> > diff --git a/blockdev.h b/blockdev.h
> > index 260e16b..96f40a5 100644
> > --- a/blockdev.h
> > +++ b/blockdev.h
> > @@ -22,7 +22,8 @@ void blockdev_auto_del(BlockDriverState *bs);
> > typedef enum {
> > IF_DEFAULT = -1, /* for use with drive_add() only */
> > IF_NONE,
> > - IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
> > IF_XEN,
> > + IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
> > IF_XEN,
> > + IF_VIRTIO_SCSI,
>
> weird indentation
>
> > IF_COUNT
> > } BlockInterfaceType;
> >
> > diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
> > index c55d8b9..4f5c022 100644
> > --- a/hw/pci-hotplug.c
> > +++ b/hw/pci-hotplug.c
> > @@ -154,6 +154,8 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
> > type = IF_SCSI;
> > else if (!strcmp(buf, "virtio")) {
> > type = IF_VIRTIO;
> > + } else if (!strcmp(buf, "virtio-scsi")) {
> > + type = IF_VIRTIO_SCSI;
> > } else {
> > monitor_printf(mon, "type %s not a hotpluggable PCI
> > device.\n", buf);
> > return NULL;
> > @@ -211,6 +213,19 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor
> > *mon,
> > if (qdev_init(&dev->qdev) < 0)
> > dev = NULL;
> > break;
> > + case IF_VIRTIO_SCSI:
> > + dev = pci_create(bus, devfn, "virtio-scsi-pci");
> > + if (qdev_init(&dev->qdev) < 0) {
> > + dev = NULL;
> > + }
> > +
> > + if (dev && dinfo) {
> > + if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
> > + qdev_unplug(&dev->qdev, NULL);
> > + dev = NULL;
> > + }
> > + }
> > + break;
> > default:
> > dev = NULL;
> > }
>
> Here I am sure.
> Do not keep adding stuff to pci-hotplug.
> New devices should use device-add.
>
> > --
> > 1.7.1
>
- [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Kelvin Wang, 2012/05/23
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Michael S. Tsirkin, 2012/05/23
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Paolo Bonzini, 2012/05/23
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Michael S. Tsirkin, 2012/05/23
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Kelvin Wang, 2012/05/24
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Michael S. Tsirkin, 2012/05/24
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Paolo Bonzini, 2012/05/24
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Kelvin Wang, 2012/05/24
- Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug, Stefan Hajnoczi, 2012/05/25
Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug,
Kelvin Wang <=