From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57931) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WldR2-000715-N2 for qemu-devel@nongnu.org; Sat, 17 May 2014 08:10:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WldQt-00061D-La for qemu-devel@nongnu.org; Sat, 17 May 2014 08:09:52 -0400 Received: from mail-pa0-x229.google.com ([2607:f8b0:400e:c03::229]:46323) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WldQt-000616-B3 for qemu-devel@nongnu.org; Sat, 17 May 2014 08:09:43 -0400 Received: by mail-pa0-f41.google.com with SMTP id lj1so3704318pab.28 for ; Sat, 17 May 2014 05:09:42 -0700 (PDT) Message-ID: <5377517C.7060408@gmail.com> Date: Sat, 17 May 2014 20:09:32 +0800 From: Jun Li MIME-Version: 1.0 References: <1397658057-12086-1-git-send-email-junmuzi@gmail.com> <20140515150748.GA29436@redhat.com> In-Reply-To: <20140515150748.GA29436@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] Add remove_boot_device_path() function for hot-unplug device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: kwolf@redhat.com, aliguori@amazon.com, juli@redhat.com, stefanha@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, pbonzini@redhat.com, afaerber@suse.de On 05/15/2014 11:07 PM, Michael S. Tsirkin wrote: > On Wed, Apr 16, 2014 at 10:20:57PM +0800, Jun Li wrote: >> Add remove_boot_device_path() function to remove bootindex when hot-unplug >> a device. This patch fixed virtio-blk/virtio-net/scsi-disk/scsi-generic device. >> So it has fixed bug1086603, ref: >> https://bugzilla.redhat.com/show_bug.cgi?id=1086603 >> >> Make some changes based on Andreas's good suggestion. >> >> Signed-off-by: Jun Li >> --- >> hw/block/virtio-blk.c | 1 + >> hw/net/virtio-net.c | 1 + >> hw/scsi/scsi-disk.c | 6 ++++-- >> hw/scsi/scsi-generic.c | 3 +++ >> include/sysemu/sysemu.h | 2 ++ >> vl.c | 16 ++++++++++++++++ >> 6 files changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c >> index 8a568e5..ecdd266 100644 >> --- a/hw/block/virtio-blk.c >> +++ b/hw/block/virtio-blk.c >> @@ -752,6 +752,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp) >> unregister_savevm(dev, "virtio-blk", s); >> blockdev_mark_auto_del(s->bs); >> virtio_cleanup(vdev); >> + remove_boot_device_path(s->conf->bootindex, dev, "/disk@0,0"); >> } >> >> static Property virtio_blk_properties[] = { >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 33bd233..520c029 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -1633,6 +1633,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp) >> g_free(n->vqs); >> qemu_del_nic(n->nic); >> virtio_cleanup(vdev); >> + remove_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0"); >> } >> >> static void virtio_net_instance_init(Object *obj) >> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c >> index 48a28ae..bb2176a 100644 >> --- a/hw/scsi/scsi-disk.c >> +++ b/hw/scsi/scsi-disk.c >> @@ -2150,12 +2150,14 @@ static void scsi_disk_reset(DeviceState *dev) >> s->tray_open = 0; >> } >> >> -static void scsi_destroy(SCSIDevice *dev) >> +static void scsi_destroy(SCSIDevice *sdev) >> { >> - SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); >> + SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, sdev); >> + DeviceState *dev = DEVICE(sdev); >> >> scsi_device_purge_requests(&s->qdev, SENSE_CODE(NO_SENSE)); >> blockdev_mark_auto_del(s->qdev.conf.bs); >> + remove_boot_device_path(s->qdev.conf.bootindex, dev, NULL); >> } >> >> static void scsi_disk_resize_cb(void *opaque) >> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c >> index 8d92e0d..2531a44 100644 >> --- a/hw/scsi/scsi-generic.c >> +++ b/hw/scsi/scsi-generic.c >> @@ -388,8 +388,11 @@ static void scsi_generic_reset(DeviceState *dev) >> >> static void scsi_destroy(SCSIDevice *s) >> { >> + DeviceState *dev = DEVICE(s); >> + >> scsi_device_purge_requests(s, SENSE_CODE(NO_SENSE)); >> blockdev_mark_auto_del(s->conf.bs); >> + remove_boot_device_path(s->conf.bootindex, dev, NULL); >> } >> >> static int scsi_generic_initfn(SCSIDevice *s) >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index ba5c7f8..f7ad1e2 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -193,6 +193,8 @@ void rtc_change_mon_event(struct tm *tm); >> >> void add_boot_device_path(int32_t bootindex, DeviceState *dev, >> const char *suffix); >> +void remove_boot_device_path(int32_t bootindex, DeviceState *dev, >> + const char *suffix); >> char *get_boot_devices_list(size_t *size, bool ignore_suffixes); >> >> DeviceState *get_boot_device(uint32_t position); >> diff --git a/vl.c b/vl.c >> index 9975e5a..1713c68 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1184,6 +1184,22 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev, >> QTAILQ_INSERT_TAIL(&fw_boot_order, node, link); >> } >> >> +void remove_boot_device_path(int32_t bootindex, DeviceState *dev, >> + const char *suffix) > Why do we need suffix here? > It seems unused. Hi Michael, I just want to keep the same as function "add_boot_device_path". Such as: void add_boot_device_path(int32_t bootindex, DeviceState *dev, const char *suffix) I also find another function has define mon, but not be used in this function. Such as: ---file vl.c--- do_usb_del(Monitor *mon, const QDict *qdict) { const char *devname = qdict_get_str(qdict, "devname"); if (usb_device_del(devname) < 0) { error_report("could not delete USB device '%s'", devname); } } Best Regards, Jun Li >> +{ >> + FWBootEntry *node, *next_node; >> + >> + if (bootindex == -1) { >> + return; >> + } >> + >> + QTAILQ_FOREACH_SAFE(node, &fw_boot_order, link, next_node) >> + if (node->bootindex == bootindex) { >> + QTAILQ_REMOVE(&fw_boot_order, node, link); >> + return; >> + } >> +} >> + >> DeviceState *get_boot_device(uint32_t position) >> { >> uint32_t counter = 0; >> -- >> 1.8.3.1 >>