From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43975) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjsWE-0006AM-Ti for qemu-devel@nongnu.org; Mon, 12 May 2014 11:52:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjsW5-0006W5-QW for qemu-devel@nongnu.org; Mon, 12 May 2014 11:51:58 -0400 Received: from mail-pa0-x236.google.com ([2607:f8b0:400e:c03::236]:50107) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjsW5-0006Vp-Eg for qemu-devel@nongnu.org; Mon, 12 May 2014 11:51:49 -0400 Received: by mail-pa0-f54.google.com with SMTP id bj1so7505504pad.41 for ; Mon, 12 May 2014 08:51:48 -0700 (PDT) Message-ID: <5370EE07.6040809@gmail.com> Date: Mon, 12 May 2014 23:51:35 +0800 From: Jun Li MIME-Version: 1.0 References: <1397658057-12086-1-git-send-email-junmuzi@gmail.com> <1398158504.30055.12.camel@localhost.localdomain> <536EE987.20905@gmail.com> <1399799838.29925.12.camel@localhost.localdomain> In-Reply-To: <1399799838.29925.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 Apfelbaum 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 05/11/2014 05:17 PM, Marcel Apfelbaum wrote: > On Sun, 2014-05-11 at 11:07 +0800, lijun wrote: >> 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. :) > Hi Jun, > I hope you are feeling better! > If you need further assistance, just let us know. Hi Marcel, I just do a glancing debug, find that, when reboot guest, It will call function qemu_system_reset. So I want to add reset "fw_cfg" in function qemu_system_reset. Maybe this can fix your issue. But I have not try due to the limited time. This is just an ideal. As I am a sparetime "developer", so I can just fix this issue at weekend. So sorry. Thanks, Jun Li