From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44416) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjK7a-0002ff-Ou for qemu-devel@nongnu.org; Sat, 10 May 2014 23:08:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjK7R-0008Dv-KH for qemu-devel@nongnu.org; Sat, 10 May 2014 23:08:14 -0400 Received: from mail-pd0-x233.google.com ([2607:f8b0:400e:c02::233]:59895) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjK7R-0008Dn-5o for qemu-devel@nongnu.org; Sat, 10 May 2014 23:08:05 -0400 Received: by mail-pd0-f179.google.com with SMTP id g10so5197294pdj.38 for ; Sat, 10 May 2014 20:08:03 -0700 (PDT) Message-ID: <536EE987.20905@gmail.com> Date: Sun, 11 May 2014 11:07:51 +0800 From: lijun MIME-Version: 1.0 References: <1397658057-12086-1-git-send-email-junmuzi@gmail.com> <1398158504.30055.12.camel@localhost.localdomain> In-Reply-To: <1398158504.30055.12.camel@localhost.localdomain> Content-Type: text/plain; charset=UTF-8; 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: marcel.a@redhat.com Cc: kwolf@redhat.com, aliguori@amazon.com, mst@redhat.com, juli@redhat.com, stefanha@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, pbonzini@redhat.com, afaerber@suse.de On 04/22/2014 05:21 PM, Marcel Apfelbaum wrote: > On Wed, 2014-04-16 at 22:20 +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) >> +{ >> + 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); > Hi Liu. > Sorry for the late response, but I was in vacation :) . > Your patch looks OK, I once wrote another one on the same issue > (it touched the same problem, but it was *completely* different). > > Here is the *real* problem: fw_boot_order list is not queried > again on guest reboot, so 'touching' the list has no effect. > While your code is correct (as far as I can tell), it seems > that his place is after the above problem is solved. > > No one is currently working on this, feel free > to take this challenge. I'll help however I can. Hi Marcel, Sorry, so later response. I got a heavy sick these days. I have see these discussion emails between you and Paolo. After guest reboot, seabios will got devices info from boards(this boards be generated by qemu command line). Just as Paolo's explanations, but I am not familiar with seabios. I will do some research about this. Thank you very much. BTW, My first name is Jun, my last name is Li, my English name is woodpecker, you could call me jun or woodpecker. :) Best Regards, Jun Li > >> + return; >> + } >> +} >> + >> DeviceState *get_boot_device(uint32_t position) >> { >> uint32_t counter = 0; > >