From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43626) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXQqC-0006Im-0n for qemu-devel@nongnu.org; Thu, 24 May 2012 01:44:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SXQq9-0003t2-LV for qemu-devel@nongnu.org; Thu, 24 May 2012 01:44:03 -0400 Received: from e28smtp07.in.ibm.com ([122.248.162.7]:56325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXQq0-0003kt-Kz for qemu-devel@nongnu.org; Thu, 24 May 2012 01:44:01 -0400 Received: from /spool/local by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 24 May 2012 11:13:32 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q4O5hTkT7078230 for ; Thu, 24 May 2012 11:13:29 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q4OBCidu020972 for ; Thu, 24 May 2012 21:12:45 +1000 Date: Thu, 24 May 2012 13:43:26 +0800 From: Kelvin Wang Message-ID: <20120524054326.GA19496@chinaltcdragon.cn.ibm.com> References: <20120523135206.GA14654@chinaltcdragon.cn.ibm.com> <20120523141202.GC29930@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120523141202.GC29930@redhat.com> Subject: Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug Reply-To: Kelvin Wang List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Stefan Hajnoczi , Paolo Bonzini , qemu-devel 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 > > Signed-off-by: Sheng Liu > > 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 >