[Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[arei.gonglei]

From: Chenliang <chenliang88@huawei.com>

Sometime, we want to modify boot order of vm without shutdown it.
This sets of patches add one qmp to achieve it. And fix some little
bug when device is hotpluged.

Chenliang (5):
  bootindex: add *_boot_device_path function
  bootindex: reset bootindex when vm reset
  bootindex: delete boot index when device is removed
  bootindex: add qmp to set boot index when vm is running
  bootindex: fix memory leak when ppc sets boot index

 hmp.c                     | 11 ++++++++++
 hmp.h                     |  1 +
 hw/block/virtio-blk.c     |  1 +
 hw/i386/kvm/pci-assign.c  |  1 +
 hw/misc/vfio.c            |  1 +
 hw/net/e1000.c            |  1 +
 hw/net/eepro100.c         |  1 +
 hw/net/ne2000.c           |  1 +
 hw/net/rtl8139.c          |  1 +
 hw/net/virtio-net.c       |  1 +
 hw/net/vmxnet3.c          |  1 +
 hw/nvram/fw_cfg.c         | 53 +++++++++++++++++++++++++++++++++++++++------
 hw/ppc/spapr.c            |  1 +
 hw/scsi/scsi-generic.c    |  1 +
 hw/usb/dev-network.c      |  1 +
 hw/usb/host-libusb.c      |  1 +
 hw/usb/redirect.c         |  1 +
 include/hw/nvram/fw_cfg.h |  2 ++
 include/sysemu/sysemu.h   |  4 ++++
 qapi-schema.json          | 16 ++++++++++++++
 qmp-commands.hx           | 16 ++++++++++++++
 qmp.c                     | 14 ++++++++++++
 vl.c                      | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 23 files changed, 179 insertions(+), 7 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[arei.gonglei]

From: Chenliang <chenliang88@huawei.com>

Add del_boot_device_path and modify_boot_device_path. Device should
be removed from boot device list  by del_boot_device_path when device
hotplug. modify_boot_device_path is used to modify deviceboot order.

Signed-off-by: Chenliang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 include/sysemu/sysemu.h |  4 ++++
 vl.c                    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 285c45b..38ef1cd 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
 
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
+void del_boot_device_path(int32_t bootindex, DeviceState *dev,
+                          const char *suffix);
+void modify_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 a1686ef..6264e11 100644
--- a/vl.c
+++ b/vl.c
@@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
     QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
 }
 
+static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
+{
+    bool ret = false;
+    char *devpath_src = qdev_get_fw_dev_path(src);
+    char *devpath_target = qdev_get_fw_dev_path(target);
+
+    if (!strcmp(devpath_src, devpath_target)) {
+        ret = true;
+    }
+
+    g_free(devpath_src);
+    g_free(devpath_target);
+    return ret;
+}
+
+void del_boot_device_path(int32_t bootindex, DeviceState *dev,
+                          const char *suffix)
+{
+    FWBootEntry *i;
+
+    assert(dev != NULL);
+
+    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        if (is_same_fw_dev_path(i->dev, dev)) {
+            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
+                continue;
+            }
+            QTAILQ_REMOVE(&fw_boot_order, i, link);
+            g_free(i->suffix);
+            g_free(i);
+            break;
+        }
+    }
+
+    if (bootindex == -1) {
+        return;
+    }
+
+    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        if (i->bootindex == bootindex) {
+            QTAILQ_REMOVE(&fw_boot_order, i, link);
+            g_free(i->suffix);
+            g_free(i);
+            break;
+        }
+    }
+}
+
+void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
+                             const char *suffix)
+{
+    del_boot_device_path(bootindex, dev, suffix);
+    add_boot_device_path(bootindex, dev, suffix);
+}
+
 DeviceState *get_boot_device(uint32_t position)
 {
     uint32_t counter = 0;
-- 
1.7.12.4

[Qemu-devel] [RFC PATCH 2/5] bootindex: reset bootindex when vm reset

[arei.gonglei]

From: Chenliang <chenliang88@huawei.com>

Reset bootindex when vm reboot. Prepare to achive that modify boot
order when vm is running.

Signed-off-by: Chenliang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/nvram/fw_cfg.c         | 53 ++++++++++++++++++++++++++++++++++++++++-------
 include/hw/nvram/fw_cfg.h |  2 ++
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b71d251..97d4951 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -56,7 +56,6 @@ struct FWCfgState {
     FWCfgFiles *files;
     uint16_t cur_entry;
     uint32_t cur_offset;
-    Notifier machine_ready;
 };
 
 #define JPG_FILE 0
@@ -402,6 +401,25 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key,
     s->entries[arch][key].callback_opaque = callback_opaque;
 }
 
+static void* fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key,
+                                              void *data, size_t len)
+{
+    void *ptr;
+    int arch = !!(key & FW_CFG_ARCH_LOCAL);
+
+    key &= FW_CFG_ENTRY_MASK;
+
+    assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX);
+
+    ptr = s->entries[arch][key].data;
+    s->entries[arch][key].data = data;
+    s->entries[arch][key].len = len;
+    s->entries[arch][key].callback_opaque = NULL;
+    s->entries[arch][key].callback = NULL;
+
+    return ptr;
+}
+
 void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
 {
     fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len);
@@ -499,13 +517,36 @@ void fw_cfg_add_file(FWCfgState *s,  const char *filename,
     fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len);
 }
 
-static void fw_cfg_machine_ready(struct Notifier *n, void *data)
+void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
+                        void *data, size_t len)
+{
+    int i, index;
+
+    assert(s->files);
+
+    index = be32_to_cpu(s->files->count);
+    assert(index < FW_CFG_FILE_SLOTS);
+
+    for (i = 0; i < index; i++) {
+        if (strcmp(filename, s->files->f[i].name) == 0) {
+            return fw_cfg_modify_bytes_read(s, FW_CFG_FILE_FIRST + i,
+                                     data, len);
+        }
+    }
+    /* add new one */
+    fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len);
+    return NULL;
+}
+
+static void fw_cfg_machine_reset(void *opaque)
 {
+    void *ptr;
     size_t len;
-    FWCfgState *s = container_of(n, FWCfgState, machine_ready);
+    FWCfgState *s = opaque;
     char *bootindex = get_boot_devices_list(&len, false);
 
-    fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
+    ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t*)bootindex, len);
+    g_free(ptr);
 }
 
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
@@ -542,9 +583,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     fw_cfg_bootsplash(s);
     fw_cfg_reboot(s);
 
-    s->machine_ready.notify = fw_cfg_machine_ready;
-    qemu_add_machine_init_done_notifier(&s->machine_ready);
-
+    qemu_register_reset(fw_cfg_machine_reset, s);
     return s;
 }
 
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 72b1549..56e1ed7 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -76,6 +76,8 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
 void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
                               FWCfgReadCallback callback, void *callback_opaque,
                               void *data, size_t len);
+void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
+                         size_t len);
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
                         hwaddr crl_addr, hwaddr data_addr);
 
-- 
1.7.12.4

[Qemu-devel] [RFC PATCH 3/5] bootindex: delete boot index when device is removed

[arei.gonglei]

From: Chenliang <chenliang88@huawei.com>

Device should be remove from boot list when it hotplug.

Signed-off-by: Chenliang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/block/virtio-blk.c    | 1 +
 hw/i386/kvm/pci-assign.c | 1 +
 hw/misc/vfio.c           | 1 +
 hw/net/e1000.c           | 1 +
 hw/net/eepro100.c        | 1 +
 hw/net/ne2000.c          | 1 +
 hw/net/rtl8139.c         | 1 +
 hw/net/virtio-net.c      | 1 +
 hw/net/vmxnet3.c         | 1 +
 hw/scsi/scsi-generic.c   | 1 +
 hw/usb/dev-network.c     | 1 +
 hw/usb/host-libusb.c     | 1 +
 hw/usb/redirect.c        | 1 +
 13 files changed, 13 insertions(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 08562ea..3395a42 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -756,6 +756,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
 
+    del_boot_device_path(-1, dev, "/disk@0,0");
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     remove_migration_state_change_notifier(&s->migration_state_notifier);
     virtio_blk_data_plane_destroy(s->dataplane);
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index de33657..4dcd78c 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1853,6 +1853,7 @@ static void assigned_exitfn(struct PCIDevice *pci_dev)
 {
     AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
 
+    del_boot_device_path(-1, &pci_dev->qdev, NULL);
     deassign_device(dev);
     free_assigned_device(dev);
 }
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 7437c2e..2f4bec5 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4220,6 +4220,7 @@ static void vfio_exitfn(PCIDevice *pdev)
     VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
     VFIOGroup *group = vdev->group;
 
+    del_boot_device_path(-1, &pdev->qdev, NULL);
     vfio_unregister_err_notifier(vdev);
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
     vfio_disable_interrupts(vdev);
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 0fc29a0..2ca1acd 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1492,6 +1492,7 @@ pci_e1000_uninit(PCIDevice *dev)
 {
     E1000State *d = E1000(dev);
 
+    del_boot_device_path(-1, DEVICE(dev), "/ethernet-phy@0");
     timer_del(d->autoneg_timer);
     timer_free(d->autoneg_timer);
     timer_del(d->mit_timer);
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index aaa3ff2..9b9734d 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1843,6 +1843,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
 {
     EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
 
+    del_boot_device_path(-1, &pci_dev->qdev, "/ethernet-phy@0");
     memory_region_destroy(&s->mmio_bar);
     memory_region_destroy(&s->io_bar);
     memory_region_destroy(&s->flash_bar);
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index d558b8c..780b74c 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -748,6 +748,7 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
     PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
     NE2000State *s = &d->ne2000;
 
+    del_boot_device_path(-1, &pci_dev->qdev, "/ethernet-phy@0");
     memory_region_destroy(&s->io);
     qemu_del_nic(s->nic);
     qemu_free_irq(s->irq);
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 90bc5ec..fe637da 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3462,6 +3462,7 @@ static void pci_rtl8139_uninit(PCIDevice *dev)
 {
     RTL8139State *s = RTL8139(dev);
 
+    del_boot_device_path(-1, DEVICE(dev), "/ethernet-phy@0");
     memory_region_destroy(&s->bar_io);
     memory_region_destroy(&s->bar_mem);
     if (s->cplus_txbuffer) {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 00b5e07..ebe5394 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1626,6 +1626,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
     virtio_net_set_status(vdev, 0);
 
     unregister_savevm(dev, "virtio-net", n);
+    del_boot_device_path(-1, dev, "/ethernet-phy@0");
 
     g_free(n->netclient_name);
     n->netclient_name = NULL;
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 77bea6f..1e0573f 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2176,6 +2176,7 @@ static void vmxnet3_pci_uninit(PCIDevice *pci_dev)
     VMW_CBPRN("Starting uninit...");
 
     unregister_savevm(dev, "vmxnet3-msix", s);
+    del_boot_device_path(-1, dev, "/ethernet-phy@0");
 
     vmxnet3_net_uninit(s);
 
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 3733d2c..b567319 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -388,6 +388,7 @@ static void scsi_generic_reset(DeviceState *dev)
 
 static void scsi_destroy(SCSIDevice *s)
 {
+    del_boot_device_path(-1, &s->qdev, NULL);
     scsi_device_purge_requests(s, SENSE_CODE(NO_SENSE));
     blockdev_mark_auto_del(s->conf.bs);
 }
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 518d536..3b4c8da 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1329,6 +1329,7 @@ static void usb_net_handle_destroy(USBDevice *dev)
     USBNetState *s = (USBNetState *) dev;
 
     /* TODO: remove the nd_table[] entry */
+    del_boot_device_path(-1, &dev->qdev, "/ethernet@0");
     rndis_clear_responsequeue(s);
     qemu_del_nic(s->nic);
 }
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 33b5b9f..9920ced 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -984,6 +984,7 @@ static void usb_host_handle_destroy(USBDevice *udev)
 {
     USBHostDevice *s = USB_HOST_DEVICE(udev);
 
+    del_boot_device_path(-1, &udev->qdev, NULL);
     qemu_remove_exit_notifier(&s->exit);
     QTAILQ_REMOVE(&hostdevs, s, next);
     usb_host_close(s);
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 4c6187b..271358c 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1416,6 +1416,7 @@ static void usbredir_handle_destroy(USBDevice *udev)
 {
     USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
 
+    del_boot_device_path(-1, &udev->qdev, NULL);
     qemu_chr_delete(dev->cs);
     dev->cs = NULL;
     /* Note must be done after qemu_chr_close, as that causes a close event */
-- 
1.7.12.4

[Qemu-devel] [RFC PATCH 4/5] bootindex: add qmp to set boot index when vm is running

[arei.gonglei]

From: Chenliang <chenliang88@huawei.com>

Signed-off-by: Chenliang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hmp.c            | 11 +++++++++++
 hmp.h            |  1 +
 qapi-schema.json | 16 ++++++++++++++++
 qmp-commands.hx  | 16 ++++++++++++++++
 qmp.c            | 14 ++++++++++++++
 5 files changed, 58 insertions(+)

diff --git a/hmp.c b/hmp.c
index dc3d279..b2c6b6c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1713,3 +1713,14 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 
     monitor_printf(mon, "\n");
 }
+
+void hmp_set_bootindex(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    const char *id = qdict_get_str(qdict, "id");
+    int64_t bootindex = qdict_get_int(qdict, "bootindex");
+    char *suffix = qdict_get_try_str(qdict, "suffix");
+
+    qmp_set_bootindex(id, bootindex, suffix, &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 4fd3c4a..eb2641a 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_info_memdev(Monitor *mon, const QDict *qdict);
+void hmp_set_bootindex(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index e7727a1..b414cae 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1694,6 +1694,22 @@
 { 'command': 'device_del', 'data': {'id': 'str'} }
 
 ##
+# @set_bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: the suffix of the device
+#
+# Returns: Nothing on success
+#          If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.1
+##
+{ 'command': 'set_bootindex', 'data': {'id': 'str', 'bootindex': 'int', 'suffix': 'str'} }
+
+##
 # @DumpGuestMemoryFormat:
 #
 # An enumeration of guest-memory-dump's format.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e4a1c80..03645b6 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3661,3 +3661,19 @@ Example:
                  { "slot": "3", "slot-type": "DIMM", "source": 0, "status": 0}
    ]}
 EQMP
+
+SQMP
+@set-bootindex
+--------------------
+
+Set boot index of one device
+
+Example:
+-> { "execute": "set-bootindex", "arguments": { "id": "ide0-0-1", "bootindex": 1, "suffix": "/disk@0"}}
+
+EQMP
+    {
+        .name       = "set-bootindex",
+        .args_type  = "id:s,bootindex:l,suffix:s?",
+        .mhandler.cmd_new = qmp_marshal_input_set_bootindex,
+    },
diff --git a/qmp.c b/qmp.c
index dca6efb..5ac9401 100644
--- a/qmp.c
+++ b/qmp.c
@@ -659,3 +659,17 @@ ACPIOSTInfoList *qmp_query_acpi_ospm_status(Error **errp)
 
     return head;
 }
+
+void qmp_set_bootindex(const char *id, int64_t bootindex,
+                       const char *suffix, Error **errp)
+{
+    DeviceState *dev;
+
+    dev = qdev_find_recursive(sysbus_get_default(), id);
+    if (NULL == dev) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+        return;
+    }
+
+    modify_boot_device_path(bootindex, dev, strlen(suffix) ? suffix : NULL);
+}
-- 
1.7.12.4

[Qemu-devel] [RFC PATCH 5/5] bootindex: fix memory leak when set boot index

[arei.gonglei]

From: Chenliang <chenliang88@huawei.com>

get_boot_devices_list will malloc some memory, spapr_finalize_fdt
doesn't free it.

Signed-off-by: Chenliang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/ppc/spapr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 82f183f..502868e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -795,6 +795,7 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
 
         }
         ret = fdt_setprop_string(fdt, offset, "qemu,boot-list", bootlist);
+        g_free(bootlist);
     }
 
     if (!spapr->has_graphics) {
-- 
1.7.12.4

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Michael S. Tsirkin]

On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com wrote:
> From: Chenliang <chenliang88@huawei.com>
> 
> Sometime, we want to modify boot order of vm without shutdown it.
> This sets of patches add one qmp to achieve it. And fix some little
> bug when device is hotpluged.
> 
> Chenliang (5):
>   bootindex: add *_boot_device_path function
>   bootindex: reset bootindex when vm reset
>   bootindex: delete boot index when device is removed
>   bootindex: add qmp to set boot index when vm is running
>   bootindex: fix memory leak when ppc sets boot index

Unfortunately at least for PC, boot order is exposed
in fw cfg which can not change while guest is running.
I suspect we need to change how we report boot order to guests.
While we are at it, maybe we can fix the silly bootindex
convention: I think people really want to specify boot *order*,
not boot index.



>  hmp.c                     | 11 ++++++++++
>  hmp.h                     |  1 +
>  hw/block/virtio-blk.c     |  1 +
>  hw/i386/kvm/pci-assign.c  |  1 +
>  hw/misc/vfio.c            |  1 +
>  hw/net/e1000.c            |  1 +
>  hw/net/eepro100.c         |  1 +
>  hw/net/ne2000.c           |  1 +
>  hw/net/rtl8139.c          |  1 +
>  hw/net/virtio-net.c       |  1 +
>  hw/net/vmxnet3.c          |  1 +
>  hw/nvram/fw_cfg.c         | 53 +++++++++++++++++++++++++++++++++++++++------
>  hw/ppc/spapr.c            |  1 +
>  hw/scsi/scsi-generic.c    |  1 +
>  hw/usb/dev-network.c      |  1 +
>  hw/usb/host-libusb.c      |  1 +
>  hw/usb/redirect.c         |  1 +
>  include/hw/nvram/fw_cfg.h |  2 ++
>  include/sysemu/sysemu.h   |  4 ++++
>  qapi-schema.json          | 16 ++++++++++++++
>  qmp-commands.hx           | 16 ++++++++++++++
>  qmp.c                     | 14 ++++++++++++
>  vl.c                      | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  23 files changed, 179 insertions(+), 7 deletions(-)
> 
> -- 
> 1.7.12.4
> 

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Laszlo Ersek]

On 07/07/14 11:29, Michael S. Tsirkin wrote:
> On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com wrote:
>> From: Chenliang <chenliang88@huawei.com>
>>
>> Sometime, we want to modify boot order of vm without shutdown it.
>> This sets of patches add one qmp to achieve it. And fix some little
>> bug when device is hotpluged.
>>
>> Chenliang (5):
>>   bootindex: add *_boot_device_path function
>>   bootindex: reset bootindex when vm reset
>>   bootindex: delete boot index when device is removed
>>   bootindex: add qmp to set boot index when vm is running
>>   bootindex: fix memory leak when ppc sets boot index
> 
> Unfortunately at least for PC, boot order is exposed
> in fw cfg which can not change while guest is running.
> I suspect we need to change how we report boot order to guests.
> While we are at it, maybe we can fix the silly bootindex
> convention: I think people really want to specify boot *order*,
> not boot index.

Please preserve the "bootorder" fw_cfg file, and its format.

I don't have any request in relation to the new (== dynamic) feature ATM.

Thanks
Laszlo

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Gonglei (Arei)]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, July 07, 2014 5:29 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> pbonzini@redhat.com; lersek@redhat.com; kraxel@redhat.com;
> imammedo@redhat.com; dmitry@daynix.com; marcel.a@redhat.com;
> peter.crosthwaite@xilinx.com; rth@twiddle.net; somlo@cmu.edu;
> Huangweidong (C); Luonengjun; Huangpeng (Peter); chenliang (T)
> Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> 
> On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com wrote:
> > From: Chenliang <chenliang88@huawei.com>
> >
> > Sometime, we want to modify boot order of vm without shutdown it.
> > This sets of patches add one qmp to achieve it. And fix some little
> > bug when device is hotpluged.
> >
> > Chenliang (5):
> >   bootindex: add *_boot_device_path function
> >   bootindex: reset bootindex when vm reset
> >   bootindex: delete boot index when device is removed
> >   bootindex: add qmp to set boot index when vm is running
> >   bootindex: fix memory leak when ppc sets boot index
> 
> Unfortunately at least for PC, boot order is exposed
> in fw cfg which can not change while guest is running.

Yes, so we should assure it take effect after the guest rebooting. 

> I suspect we need to change how we report boot order to guests.
> While we are at it, maybe we can fix the silly bootindex
> convention: I think people really want to specify boot *order*,
> not boot index.
> 
Agreed.

But at present, the boot index can be used for the boot order 
except "-boot" command line. Because "-boot" only can assign
the guest booting from HD or Network or Floppy etc.. but cannot
assign the index of hard disks or PXE net cards, which not be enough
for many scenes, such as P2V, or two different system hard disks
(vda/sda/hda).


Best regards,
-Gonglei

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Gonglei (Arei)]

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, July 07, 2014 6:04 PM
> To: Michael S. Tsirkin; Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> pbonzini@redhat.com; kraxel@redhat.com; imammedo@redhat.com;
> dmitry@daynix.com; marcel.a@redhat.com; peter.crosthwaite@xilinx.com;
> rth@twiddle.net; somlo@cmu.edu; Huangweidong (C); Luonengjun;
> Huangpeng (Peter); chenliang (T)
> Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> 
> On 07/07/14 11:29, Michael S. Tsirkin wrote:
> > On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com
> wrote:
> >> From: Chenliang <chenliang88@huawei.com>
> >>
> >> Sometime, we want to modify boot order of vm without shutdown it.
> >> This sets of patches add one qmp to achieve it. And fix some little
> >> bug when device is hotpluged.
> >>
> >> Chenliang (5):
> >>   bootindex: add *_boot_device_path function
> >>   bootindex: reset bootindex when vm reset
> >>   bootindex: delete boot index when device is removed
> >>   bootindex: add qmp to set boot index when vm is running
> >>   bootindex: fix memory leak when ppc sets boot index
> >
> > Unfortunately at least for PC, boot order is exposed
> > in fw cfg which can not change while guest is running.
> > I suspect we need to change how we report boot order to guests.
> > While we are at it, maybe we can fix the silly bootindex
> > convention: I think people really want to specify boot *order*,
> > not boot index.
> 
> Please preserve the "bootorder" fw_cfg file, and its format.
> 
> I don't have any request in relation to the new (== dynamic) feature ATM.
> 
Sorry, I can't understand your meaning exactly. 
Would you explain it? Thanks!

> Thanks
> Laszlo

Best regards,
-Gonglei

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Michael S. Tsirkin]

On Mon, Jul 07, 2014 at 11:08:32AM +0000, Gonglei (Arei) wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Monday, July 07, 2014 5:29 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> > stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> > alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> > kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> > pbonzini@redhat.com; lersek@redhat.com; kraxel@redhat.com;
> > imammedo@redhat.com; dmitry@daynix.com; marcel.a@redhat.com;
> > peter.crosthwaite@xilinx.com; rth@twiddle.net; somlo@cmu.edu;
> > Huangweidong (C); Luonengjun; Huangpeng (Peter); chenliang (T)
> > Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> > 
> > On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com wrote:
> > > From: Chenliang <chenliang88@huawei.com>
> > >
> > > Sometime, we want to modify boot order of vm without shutdown it.
> > > This sets of patches add one qmp to achieve it. And fix some little
> > > bug when device is hotpluged.
> > >
> > > Chenliang (5):
> > >   bootindex: add *_boot_device_path function
> > >   bootindex: reset bootindex when vm reset
> > >   bootindex: delete boot index when device is removed
> > >   bootindex: add qmp to set boot index when vm is running
> > >   bootindex: fix memory leak when ppc sets boot index
> > 
> > Unfortunately at least for PC, boot order is exposed
> > in fw cfg which can not change while guest is running.
> 
> Yes, so we should assure it take effect after the guest rebooting. 

Does this patch do it like this?
I didn't get it. How is this handled?
Maybe more code comments would be helpful to make this
clear to readers.

> > I suspect we need to change how we report boot order to guests.
> > While we are at it, maybe we can fix the silly bootindex
> > convention: I think people really want to specify boot *order*,
> > not boot index.
> > 
> Agreed.
> 
> But at present, the boot index can be used for the boot order 
> except "-boot" command line. Because "-boot" only can assign
> the guest booting from HD or Network or Floppy etc.. but cannot
> assign the index of hard disks or PXE net cards, which not be enough
> for many scenes, such as P2V, or two different system hard disks
> (vda/sda/hda).
> 
> 
> Best regards,
> -Gonglei

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Laszlo Ersek]

On 07/07/14 13:12, Gonglei (Arei) wrote:
> 
> 
> 
> 
> 
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Monday, July 07, 2014 6:04 PM
>> To: Michael S. Tsirkin; Gonglei (Arei)
>> Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
>> stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
>> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
>> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
>> pbonzini@redhat.com; kraxel@redhat.com; imammedo@redhat.com;
>> dmitry@daynix.com; marcel.a@redhat.com; peter.crosthwaite@xilinx.com;
>> rth@twiddle.net; somlo@cmu.edu; Huangweidong (C); Luonengjun;
>> Huangpeng (Peter); chenliang (T)
>> Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
>>
>> On 07/07/14 11:29, Michael S. Tsirkin wrote:
>>> On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com
>> wrote:
>>>> From: Chenliang <chenliang88@huawei.com>
>>>>
>>>> Sometime, we want to modify boot order of vm without shutdown it.
>>>> This sets of patches add one qmp to achieve it. And fix some little
>>>> bug when device is hotpluged.
>>>>
>>>> Chenliang (5):
>>>>   bootindex: add *_boot_device_path function
>>>>   bootindex: reset bootindex when vm reset
>>>>   bootindex: delete boot index when device is removed
>>>>   bootindex: add qmp to set boot index when vm is running
>>>>   bootindex: fix memory leak when ppc sets boot index
>>>
>>> Unfortunately at least for PC, boot order is exposed
>>> in fw cfg which can not change while guest is running.
>>> I suspect we need to change how we report boot order to guests.
>>> While we are at it, maybe we can fix the silly bootindex
>>> convention: I think people really want to specify boot *order*,
>>> not boot index.
>>
>> Please preserve the "bootorder" fw_cfg file, and its format.
>>
>> I don't have any request in relation to the new (== dynamic) feature ATM.
>>
> Sorry, I can't understand your meaning exactly. 
> Would you explain it? Thanks!

I meant that whatever features you introduce, please make sure that a
guest looking for the "bootorder" fw_cfg file will find it, and that the
contents and the format of that fw_cfg file stays the same, for the same
qemu command line options.

I'm not implying that your current series changes the "bootorder" fw_cfg
file (I have not looked at your patches); this is just a general request
I make out of caution.

Thanks
Laszlo

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Gonglei (Arei)]

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, July 07, 2014 10:40 PM
> To: Gonglei (Arei); Michael S. Tsirkin
> Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> pbonzini@redhat.com; kraxel@redhat.com; imammedo@redhat.com;
> dmitry@daynix.com; marcel.a@redhat.com; peter.crosthwaite@xilinx.com;
> rth@twiddle.net; somlo@cmu.edu; Huangweidong (C); Luonengjun;
> Huangpeng (Peter); chenliang (T)
> Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> 
> On 07/07/14 13:12, Gonglei (Arei) wrote:
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Monday, July 07, 2014 6:04 PM
> >> To: Michael S. Tsirkin; Gonglei (Arei)
> >> Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> >> stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> >> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> >> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> >> pbonzini@redhat.com; kraxel@redhat.com; imammedo@redhat.com;
> >> dmitry@daynix.com; marcel.a@redhat.com; peter.crosthwaite@xilinx.com;
> >> rth@twiddle.net; somlo@cmu.edu; Huangweidong (C); Luonengjun;
> >> Huangpeng (Peter); chenliang (T)
> >> Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> >>
> >> On 07/07/14 11:29, Michael S. Tsirkin wrote:
> >>> On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com
> >> wrote:
> >>>> From: Chenliang <chenliang88@huawei.com>
> >>>>
> >>>> Sometime, we want to modify boot order of vm without shutdown it.
> >>>> This sets of patches add one qmp to achieve it. And fix some little
> >>>> bug when device is hotpluged.
> >>>>
> >>>> Chenliang (5):
> >>>>   bootindex: add *_boot_device_path function
> >>>>   bootindex: reset bootindex when vm reset
> >>>>   bootindex: delete boot index when device is removed
> >>>>   bootindex: add qmp to set boot index when vm is running
> >>>>   bootindex: fix memory leak when ppc sets boot index
> >>>
> >>> Unfortunately at least for PC, boot order is exposed
> >>> in fw cfg which can not change while guest is running.
> >>> I suspect we need to change how we report boot order to guests.
> >>> While we are at it, maybe we can fix the silly bootindex
> >>> convention: I think people really want to specify boot *order*,
> >>> not boot index.
> >>
> >> Please preserve the "bootorder" fw_cfg file, and its format.
> >>
> >> I don't have any request in relation to the new (== dynamic) feature ATM.
> >>
> > Sorry, I can't understand your meaning exactly.
> > Would you explain it? Thanks!
> 
> I meant that whatever features you introduce, please make sure that a
> guest looking for the "bootorder" fw_cfg file will find it, and that the
> contents and the format of that fw_cfg file stays the same, for the same
> qemu command line options.
> 
Sure.

> I'm not implying that your current series changes the "bootorder" fw_cfg
> file (I have not looked at your patches); this is just a general request
> I make out of caution.
> 
OK, thanks, Laszlo.

> Thanks
> Laszlo

Best regards,
-Gonglei

Re: [Qemu-devel] [RFC PATCH 0/5] modify boot order when vm is running

[Gonglei (Arei)]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, July 07, 2014 9:08 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> pbonzini@redhat.com; lersek@redhat.com; kraxel@redhat.com;
> imammedo@redhat.com; dmitry@daynix.com; marcel.a@redhat.com;
> peter.crosthwaite@xilinx.com; rth@twiddle.net; somlo@cmu.edu;
> Huangweidong (C); Luonengjun; Huangpeng (Peter); chenliang (T)
> Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> 
> On Mon, Jul 07, 2014 at 11:08:32AM +0000, Gonglei (Arei) wrote:
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Monday, July 07, 2014 5:29 PM
> > > To: Gonglei (Arei)
> > > Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de;
> > > stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru;
> > > alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> > > kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> > > pbonzini@redhat.com; lersek@redhat.com; kraxel@redhat.com;
> > > imammedo@redhat.com; dmitry@daynix.com; marcel.a@redhat.com;
> > > peter.crosthwaite@xilinx.com; rth@twiddle.net; somlo@cmu.edu;
> > > Huangweidong (C); Luonengjun; Huangpeng (Peter); chenliang (T)
> > > Subject: Re: [RFC PATCH 0/5] modify boot order when vm is running
> > >
> > > On Mon, Jul 07, 2014 at 05:10:56PM +0800, arei.gonglei@huawei.com
> wrote:
> > > > From: Chenliang <chenliang88@huawei.com>
> > > >
> > > > Sometime, we want to modify boot order of vm without shutdown it.
> > > > This sets of patches add one qmp to achieve it. And fix some little
> > > > bug when device is hotpluged.
> > > >
> > > > Chenliang (5):
> > > >   bootindex: add *_boot_device_path function
> > > >   bootindex: reset bootindex when vm reset
> > > >   bootindex: delete boot index when device is removed
> > > >   bootindex: add qmp to set boot index when vm is running
> > > >   bootindex: fix memory leak when ppc sets boot index
> > >
> > > Unfortunately at least for PC, boot order is exposed
> > > in fw cfg which can not change while guest is running.
> >
> > Yes, so we should assure it take effect after the guest rebooting.
> 
> Does this patch do it like this?

Yes.

> I didn't get it. How is this handled?

By change the order of global fw_boot_order, when the guest rebooting,
Seabios will read the fw_cfg file again. 

[RFC PATCH 1/5] bootindex: add *_boot_device_path function

do this work.

> Maybe more code comments would be helpful to make this
> clear to readers.
> 

OK.

We will add more comments for non RFC patchsets 
after QEMU 2.1 "hard freeze ".

Thanks.

Best regards,
-Gonglei

> > > I suspect we need to change how we report boot order to guests.
> > > While we are at it, maybe we can fix the silly bootindex
> > > convention: I think people really want to specify boot *order*,
> > > not boot index.
> > >
> > Agreed.
> >
> > But at present, the boot index can be used for the boot order
> > except "-boot" command line. Because "-boot" only can assign
> > the guest booting from HD or Network or Floppy etc.. but cannot
> > assign the index of hard disks or PXE net cards, which not be enough
> > for many scenes, such as P2V, or two different system hard disks
> > (vda/sda/hda).
> >
> >
> > Best regards,
> > -Gonglei

Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[Amos Kong]

On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gonglei@huawei.com wrote:
> From: Chenliang <chenliang88@huawei.com>
> 
> Add del_boot_device_path and modify_boot_device_path. Device should
> be removed from boot device list  by del_boot_device_path when device
> hotplug. modify_boot_device_path is used to modify deviceboot order.

s/hotplug/is unhotplugged/

same issue in commitlog of patch 3/5
 
> Signed-off-by: Chenliang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  include/sysemu/sysemu.h |  4 ++++
>  vl.c                    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 285c45b..38ef1cd 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
>  
>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>                            const char *suffix);
> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> +                          const char *suffix);
> +void modify_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 a1686ef..6264e11 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>      QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
>  }
>  
> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
> +{
> +    bool ret = false;
> +    char *devpath_src = qdev_get_fw_dev_path(src);
> +    char *devpath_target = qdev_get_fw_dev_path(target);
> +
> +    if (!strcmp(devpath_src, devpath_target)) {
> +        ret = true;
> +    }
> +
> +    g_free(devpath_src);
> +    g_free(devpath_target);
> +    return ret;
> +}
> +
> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> +                          const char *suffix)
> +{
> +    FWBootEntry *i;
> +
> +    assert(dev != NULL);
> +

assert(booindex >= 0 || suffix != NULL);

> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +        if (is_same_fw_dev_path(i->dev, dev)) {

               if (!suffix) {
                   break;
               }

> +            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
> +                continue;
> +            }

If suffix is NULL, then all the entries will be removed?

> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
> +            g_free(i->suffix);
> +            g_free(i);
> +            break;
> +        }
> +    }
> +
> +    if (bootindex == -1) {

       if (bootindex < 0) {

> +        return;
> +    }
> +
> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +        if (i->bootindex == bootindex) {
> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
> +            g_free(i->suffix);
> +            g_free(i);
> +            break;
> +        }
> +    }
> +}
> +
> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> +                             const char *suffix)
> +{
> +    del_boot_device_path(bootindex, dev, suffix);
> +    add_boot_device_path(bootindex, dev, suffix);

Why do you directly modify existed entry?

> +}
> +
>  DeviceState *get_boot_device(uint32_t position)
>  {
>      uint32_t counter = 0;
> -- 
> 1.7.12.4
> 

-- 
			Amos.

Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[ChenLiang]

On 2014/7/8 16:33, Amos Kong wrote:

> On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gonglei@huawei.com wrote:
>> From: Chenliang <chenliang88@huawei.com>
>>
>> Add del_boot_device_path and modify_boot_device_path. Device should
>> be removed from boot device list  by del_boot_device_path when device
>> hotplug. modify_boot_device_path is used to modify deviceboot order.
> 
> s/hotplug/is unhotplugged/
> 
> same issue in commitlog of patch 3/5
>  
>> Signed-off-by: Chenliang <chenliang88@huawei.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  include/sysemu/sysemu.h |  4 ++++
>>  vl.c                    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 285c45b..38ef1cd 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
>>  
>>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>>                            const char *suffix);
>> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                          const char *suffix);
>> +void modify_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 a1686ef..6264e11 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>>      QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
>>  }
>>  
>> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
>> +{
>> +    bool ret = false;
>> +    char *devpath_src = qdev_get_fw_dev_path(src);
>> +    char *devpath_target = qdev_get_fw_dev_path(target);
>> +
>> +    if (!strcmp(devpath_src, devpath_target)) {
>> +        ret = true;
>> +    }
>> +
>> +    g_free(devpath_src);
>> +    g_free(devpath_target);
>> +    return ret;
>> +}
>> +
>> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                          const char *suffix)
>> +{
>> +    FWBootEntry *i;
>> +
>> +    assert(dev != NULL);
>> +
> 
> assert(booindex >= 0 || suffix != NULL);
> 
>> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
>> +        if (is_same_fw_dev_path(i->dev, dev)) {
> 
>                if (!suffix) {
>                    break;
>                }
> 
>> +            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
>> +                continue;
>> +            }
> 
> If suffix is NULL, then all the entries will be removed?


yes, it will be if caller don't give suffix.

> 
>> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
>> +            g_free(i->suffix);
>> +            g_free(i);
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (bootindex == -1) {
> 
>    if (bootindex < 0) {


acked

> 
>> +        return;
>> +    }
>> +
>> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
>> +        if (i->bootindex == bootindex) {
>> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
>> +            g_free(i->suffix);
>> +            g_free(i);
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                             const char *suffix)
>> +{
>> +    del_boot_device_path(bootindex, dev, suffix);
>> +    add_boot_device_path(bootindex, dev, suffix);
> 
> Why do you directly modify existed entry?


Sometimes, in old boot device list:

device_id      bootindex
net0           1
net1           2
net2           3

we want to make vm reboot from net2, we can do it like this:

modify_boot_device_path(bootindex=1, DeviceState=net2, suffix=NULL), the new boot device list will like this:

device_id     bootindex
net2          1
net1          2


Best regards
Chenliang

> 
>> +}
>> +
>>  DeviceState *get_boot_device(uint32_t position)
>>  {
>>      uint32_t counter = 0;
>> -- 
>> 1.7.12.4
>>
> 

Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[Gonglei (Arei)]

> -----Original Message-----
> From: chenliang (T)
> Sent: Tuesday, July 08, 2014 7:03 PM
> To: Amos Kong
> Cc: Gonglei (Arei); qemu-devel@nongnu.org; afaerber@suse.de;
> agraf@suse.de; stefanha@redhat.com; aik@ozlabs.ru;
> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> mst@redhat.com; pbonzini@redhat.com; lersek@redhat.com;
> kraxel@redhat.com; imammedo@redhat.com; dmitry@daynix.com;
> marcel.a@redhat.com; peter.crosthwaite@xilinx.com; rth@twiddle.net;
> somlo@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter)
> Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function
> 
> On 2014/7/8 16:33, Amos Kong wrote:
> 
> > On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gonglei@huawei.com
> wrote:
> >> From: Chenliang <chenliang88@huawei.com>
> >>
> >> Add del_boot_device_path and modify_boot_device_path. Device should
> >> be removed from boot device list  by del_boot_device_path when device
> >> hotplug. modify_boot_device_path is used to modify deviceboot order.
> >
> > s/hotplug/is unhotplugged/
> >
> > same issue in commitlog of patch 3/5
> >

Yep, thanks!

> >> Signed-off-by: Chenliang <chenliang88@huawei.com>
> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >> ---
> >>  include/sysemu/sysemu.h |  4 ++++
> >>  vl.c                    | 55
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 59 insertions(+)
> >>
> >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> >> index 285c45b..38ef1cd 100644
> >> --- a/include/sysemu/sysemu.h
> >> +++ b/include/sysemu/sysemu.h
> >> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
> >>
> >>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> >>                            const char *suffix);
> >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> >> +                          const char *suffix);
> >> +void modify_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 a1686ef..6264e11 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex,
> DeviceState *dev,
> >>      QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
> >>  }
> >>
> >> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
> >> +{
> >> +    bool ret = false;
> >> +    char *devpath_src = qdev_get_fw_dev_path(src);
> >> +    char *devpath_target = qdev_get_fw_dev_path(target);
> >> +
> >> +    if (!strcmp(devpath_src, devpath_target)) {
> >> +        ret = true;
> >> +    }
> >> +
> >> +    g_free(devpath_src);
> >> +    g_free(devpath_target);
> >> +    return ret;
> >> +}
> >> +
> >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> >> +                          const char *suffix)
> >> +{
> >> +    FWBootEntry *i;
> >> +
> >> +    assert(dev != NULL);
> >> +
> >
> > assert(booindex >= 0 || suffix != NULL);
> >

suffix call be NULL. 

> >> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> >> +        if (is_same_fw_dev_path(i->dev, dev)) {
> >
> >                if (!suffix) {
> >                    break;
> >                }
> >
> >> +            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
> >> +                continue;
> >> +            }
> >
> > If suffix is NULL, then all the entries will be removed?

If suffix is NULL, the entry will be checked by the bootindex as below
QTAILQ_FOREACH loop. If suffix and bootindex are all not match,
no entry will not be deleted from the global fw_boot_order list.

> 
> 
> yes, it will be if caller don't give suffix.
> 
> >
> >> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
> >> +            g_free(i->suffix);
> >> +            g_free(i);
> >> +            break;
> >> +        }
> >> +    }
> >> +
> >> +    if (bootindex == -1) {
> >
> >    if (bootindex < 0) {
> 
> 
> acked
> 
> >
> >> +        return;
> >> +    }
> >> +
> >> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> >> +        if (i->bootindex == bootindex) {
> >> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
> >> +            g_free(i->suffix);
> >> +            g_free(i);
> >> +            break;
> >> +        }
> >> +    }
> >> +}
> >> +
> >> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> >> +                             const char *suffix)
> >> +{
> >> +    del_boot_device_path(bootindex, dev, suffix);
> >> +    add_boot_device_path(bootindex, dev, suffix);
> >
> > Why do you directly modify existed entry?
> 
> 
> Sometimes, in old boot device list:
> 
> device_id      bootindex
> net0           1
> net1           2
> net2           3
> 
> we want to make vm reboot from net2, we can do it like this:
> 
> modify_boot_device_path(bootindex=1, DeviceState=net2, suffix=NULL), the
> new boot device list will like this:
> 
> device_id     bootindex
> net2          1
> net1          2
> 

Yes. 
the visual bootindex of net0 will be deleted, and then we can look 
it as default value.

Best regards,
-Gonglei

Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[Amos Kong]

On Tue, Jul 08, 2014 at 01:22:53PM +0000, Gonglei (Arei) wrote:
> > -----Original Message-----
> > From: chenliang (T)
> > Sent: Tuesday, July 08, 2014 7:03 PM
> > To: Amos Kong
> > Cc: Gonglei (Arei); qemu-devel@nongnu.org; afaerber@suse.de;
> > agraf@suse.de; stefanha@redhat.com; aik@ozlabs.ru;
> > alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> > kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> > mst@redhat.com; pbonzini@redhat.com; lersek@redhat.com;
> > kraxel@redhat.com; imammedo@redhat.com; dmitry@daynix.com;
> > marcel.a@redhat.com; peter.crosthwaite@xilinx.com; rth@twiddle.net;
> > somlo@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter)
> > Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function
> > 
> > On 2014/7/8 16:33, Amos Kong wrote:
> > 
> > > On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gonglei@huawei.com
> > wrote:
> > >> From: Chenliang <chenliang88@huawei.com>
> > >>
> > >> Add del_boot_device_path and modify_boot_device_path. Device should
> > >> be removed from boot device list  by del_boot_device_path when device
> > >> hotplug. modify_boot_device_path is used to modify deviceboot order.
> > >
> > > s/hotplug/is unhotplugged/
> > >
> > > same issue in commitlog of patch 3/5
> > >
> 
> Yep, thanks!
> 
> > >> Signed-off-by: Chenliang <chenliang88@huawei.com>
> > >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > >> ---
> > >>  include/sysemu/sysemu.h |  4 ++++
> > >>  vl.c                    | 55
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > >>  2 files changed, 59 insertions(+)
> > >>
> > >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > >> index 285c45b..38ef1cd 100644
> > >> --- a/include/sysemu/sysemu.h
> > >> +++ b/include/sysemu/sysemu.h
> > >> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
> > >>
> > >>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >>                            const char *suffix);
> > >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >> +                          const char *suffix);
> > >> +void modify_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 a1686ef..6264e11 100644
> > >> --- a/vl.c
> > >> +++ b/vl.c
> > >> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex,
> > DeviceState *dev,
> > >>      QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
> > >>  }
> > >>
> > >> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
> > >> +{
> > >> +    bool ret = false;
> > >> +    char *devpath_src = qdev_get_fw_dev_path(src);
> > >> +    char *devpath_target = qdev_get_fw_dev_path(target);
> > >> +
> > >> +    if (!strcmp(devpath_src, devpath_target)) {
> > >> +        ret = true;
> > >> +    }
> > >> +
> > >> +    g_free(devpath_src);
> > >> +    g_free(devpath_target);
> > >> +    return ret;
> > >> +}
> > >> +
> > >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >> +                          const char *suffix)
> > >> +{
> > >> +    FWBootEntry *i;
> > >> +
> > >> +    assert(dev != NULL);
> > >> +
> > >
> > > assert(booindex >= 0 || suffix != NULL);
> > >
> 
> suffix call be NULL. 
> 
> > >> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > >> +        if (is_same_fw_dev_path(i->dev, dev)) {
> > >
> > >                if (!suffix) {
> > >                    break;
> > >                }

If suffix is NULL, at least we should do nothing in the loop.

> > >
> > >> +            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
> > >> +                continue;
> > >> +            }

How about this one?

                   if (!suffix) {
                        break;
                   } else (i->suffix && strcmp(i->suffix, suffix)) {
                        continue;
                   }


> > >
> > > If suffix is NULL, then all the entries will be removed?
> 
> If suffix is NULL, the entry will be checked by the bootindex as below
> QTAILQ_FOREACH loop. If suffix and bootindex are all not match,
> no entry will not be deleted from the global fw_boot_order list.

This is why I added "assert(booindex >= 0 || suffix != NULL);" on
above.
                                         ^^^^
> > 
> > 
> > yes, it will be if caller don't give suffix.
> > 
> > >
> > >> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
> > >> +            g_free(i->suffix);
> > >> +            g_free(i);
> > >> +            break;
> > >> +        }
> > >> +    }
> > >> +
> > >> +    if (bootindex == -1) {
> > >
> > >    if (bootindex < 0) {
> > 
> > 
> > acked
> > 
> > >
> > >> +        return;
> > >> +    }
> > >> +
> > >> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > >> +        if (i->bootindex == bootindex) {
> > >> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
> > >> +            g_free(i->suffix);
> > >> +            g_free(i);
> > >> +            break;
> > >> +        }
> > >> +    }
> > >> +}
> > >> +
> > >> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >> +                             const char *suffix)
> > >> +{
> > >> +    del_boot_device_path(bootindex, dev, suffix);
> > >> +    add_boot_device_path(bootindex, dev, suffix);
> > >
> > > Why do you directly modify existed entry?
> > 
> > 
> > Sometimes, in old boot device list:
> > 
> > device_id      bootindex
> > net0           1
> > net1           2
> > net2           3
> > 
> > we want to make vm reboot from net2, we can do it like this:
> > 
> > modify_boot_device_path(bootindex=1, DeviceState=net2, suffix=NULL), the
> > new boot device list will like this:
> > 
> > device_id     bootindex
> > net2          1
> > net1          2
> > 
> 
> Yes. 
> the visual bootindex of net0 will be deleted, and then we can look 
> it as default value.
> 
> Best regards,
> -Gonglei

-- 
			Amos.

Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[Gonglei (Arei)]

> -----Original Message-----
> From: Amos Kong [mailto:akong@redhat.com]
> Sent: Tuesday, July 08, 2014 10:55 PM
> To: Gonglei (Arei)
> Cc: chenliang (T); qemu-devel@nongnu.org; afaerber@suse.de;
> agraf@suse.de; stefanha@redhat.com; aik@ozlabs.ru;
> alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> mst@redhat.com; pbonzini@redhat.com; lersek@redhat.com;
> kraxel@redhat.com; imammedo@redhat.com; dmitry@daynix.com;
> marcel.a@redhat.com; peter.crosthwaite@xilinx.com; rth@twiddle.net;
> somlo@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter)
> Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function
> 
> On Tue, Jul 08, 2014 at 01:22:53PM +0000, Gonglei (Arei) wrote:
> > > -----Original Message-----
> > > From: chenliang (T)
> > > Sent: Tuesday, July 08, 2014 7:03 PM
> > > To: Amos Kong
> > > Cc: Gonglei (Arei); qemu-devel@nongnu.org; afaerber@suse.de;
> > > agraf@suse.de; stefanha@redhat.com; aik@ozlabs.ru;
> > > alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com;
> > > kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com;
> > > mst@redhat.com; pbonzini@redhat.com; lersek@redhat.com;
> > > kraxel@redhat.com; imammedo@redhat.com; dmitry@daynix.com;
> > > marcel.a@redhat.com; peter.crosthwaite@xilinx.com; rth@twiddle.net;
> > > somlo@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter)
> > > Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function
> > >
> > > On 2014/7/8 16:33, Amos Kong wrote:
> > >
> > > > On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gonglei@huawei.com
> > > wrote:
> > > >> From: Chenliang <chenliang88@huawei.com>
> > > >>
> > > >> Add del_boot_device_path and modify_boot_device_path. Device should
> > > >> be removed from boot device list  by del_boot_device_path when
> device
> > > >> hotplug. modify_boot_device_path is used to modify deviceboot order.
> > > >
> > > > s/hotplug/is unhotplugged/
> > > >
> > > > same issue in commitlog of patch 3/5
> > > >
> >
> > Yep, thanks!
> >
> > > >> Signed-off-by: Chenliang <chenliang88@huawei.com>
> > > >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > >> ---
> > > >>  include/sysemu/sysemu.h |  4 ++++
> > > >>  vl.c                    | 55
> > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >>  2 files changed, 59 insertions(+)
> > > >>
> > > >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > >> index 285c45b..38ef1cd 100644
> > > >> --- a/include/sysemu/sysemu.h
> > > >> +++ b/include/sysemu/sysemu.h
> > > >> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict
> *qdict);
> > > >>
> > > >>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> > > >>                            const char *suffix);
> > > >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > > >> +                          const char *suffix);
> > > >> +void modify_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 a1686ef..6264e11 100644
> > > >> --- a/vl.c
> > > >> +++ b/vl.c
> > > >> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t
> bootindex,
> > > DeviceState *dev,
> > > >>      QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
> > > >>  }
> > > >>
> > > >> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState
> *target)
> > > >> +{
> > > >> +    bool ret = false;
> > > >> +    char *devpath_src = qdev_get_fw_dev_path(src);
> > > >> +    char *devpath_target = qdev_get_fw_dev_path(target);
> > > >> +
> > > >> +    if (!strcmp(devpath_src, devpath_target)) {
> > > >> +        ret = true;
> > > >> +    }
> > > >> +
> > > >> +    g_free(devpath_src);
> > > >> +    g_free(devpath_target);
> > > >> +    return ret;
> > > >> +}
> > > >> +
> > > >> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > > >> +                          const char *suffix)
> > > >> +{
> > > >> +    FWBootEntry *i;
> > > >> +
> > > >> +    assert(dev != NULL);
> > > >> +
> > > >
> > > > assert(booindex >= 0 || suffix != NULL);
> > > >
> >
> > suffix call be NULL.
> >
> > > >> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > > >> +        if (is_same_fw_dev_path(i->dev, dev)) {
> > > >
> > > >                if (!suffix) {
> > > >                    break;
> > > >                }
> 
> If suffix is NULL, at least we should do nothing in the loop.
> 
> > > >
> > > >> +            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
> > > >> +                continue;
> > > >> +            }
> 
> How about this one?
> 
>                    if (!suffix) {
>                         break;
>                    } else (i->suffix && strcmp(i->suffix, suffix)) {
>                         continue;
>                    }
> 
> 
Agreed.

It's more better, which will reduce useless loop, thanks!

> > > >
> > > > If suffix is NULL, then all the entries will be removed?
> >
> > If suffix is NULL, the entry will be checked by the bootindex as below
> > QTAILQ_FOREACH loop. If suffix and bootindex are all not match,
> > no entry will not be deleted from the global fw_boot_order list.
> 
> This is why I added "assert(booindex >= 0 || suffix != NULL);" on
> above.
>   
>                                      ^^^^

OK. You are right. Thanks, Amos.


Best regards,
-Gonglei