[Qemu-devel] [PATCH 0/6] modify boot order of guest, and take effect after rebooting

[Qemu-devel] [PATCH 0/6] modify boot order of guest, and take effect after rebooting

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Sometimes, we want to modify boot order of a guest, but no need to
shutdown it. We can call dynamic changing bootindex of a guest, which
can be assured taking effect just after the guest rebooting.

For example, in P2V scene, we boot a guest and then attach a
new system disk, for copying some thing. We want to assign the
new disk as the booting disk, which means its bootindex=1.

Different nics can be assigen different bootindex dynamically
also make sense.

The patchsets add one qmp interface, and add an fw_cfg_machine_reset()
to achieve it. Please review, Thanks.

Gonglei (6):
  bootindex: add {del,modify}_boot_device_path function
  fw_cfg: add fw_cfg_machine_reset function
  bootindex: delete bootindex when device is removed
  qmp: add set-bootindex command
  qemu-monitor: HMP set-bootindex wrapper
  spapr: fix possible memory leak

 hmp-commands.hx           | 15 +++++++++++++
 hmp.c                     | 13 +++++++++++
 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         | 54 ++++++++++++++++++++++++++++++++++++++------
 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           | 24 ++++++++++++++++++++
 qmp.c                     | 17 ++++++++++++++
 vl.c                      | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 24 files changed, 210 insertions(+), 7 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/6] bootindex: add {del, modify}_boot_device_path function

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When we want to change one device's bootindex, we should do three
things. On the on hand, remove the device from global fw_boot_order list,
regardless attaching suffix or not delete. On the other hand, delete
original object of the assigned bootindex. Finally add the new device's
bootindex into the global fw_boot_order list.

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

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..e368627 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -209,6 +209,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 fe451aa..e304a93 100644
--- a/vl.c
+++ b/vl.c
@@ -1248,6 +1248,63 @@ 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(bootindex >= 0 || suffix != 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;
+            }
+            /* if suffix is NULL, remove all entries of the assigend dev */
+            QTAILQ_REMOVE(&fw_boot_order, i, link);
+            g_free(i->suffix);
+            g_free(i);
+            break;
+        }
+    }
+
+    if (bootindex < 0) {
+        return;
+    }
+    /* find the object of assigned bootindex, and then remove it */
+    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] [PATCH 2/6] fw_cfg: add fw_cfg_machine_reset function

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

We must assure that the changed bootindex can take effect
when guest is rebooted. So we introduce fw_cfg_machine_reset(),
which change the fw_cfg file's bootindex data using the new
global fw_boot_order list.

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

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b71d251..a24a44d 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,26 @@ 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);
+
+    /* return the old data to the function caller, avoid memory leak */
+    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 +518,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 +584,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] [PATCH 3/6] bootindex: delete bootindex when device is removed

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Device should be removed from global boot list when
it is hot-unplugged.

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 c241c50..51775a2 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -786,6 +786,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 0b9eba0..fef1281 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4304,6 +4304,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 3263e3f..55be32b 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 268eff9..1c22a41 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1654,6 +1654,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 c189147..b0ac2d4 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 44522d9..59feeec 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] [PATCH 4/6] qmp: add set-bootindex command

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Adds "set-bootindex id=xx,bootindex=xx,suffix=xx" QMP command.

Example QMP command:
-> { "execute": "set-bootindex", "arguments": { "id": "ide0-0-1", "bootindex": 1, "suffix": "/disk@0"}}
<- { "return": {} }

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Chenliang <chenliang88@huawei.com>
---
 qapi-schema.json | 16 ++++++++++++++++
 qmp-commands.hx  | 24 ++++++++++++++++++++++++
 qmp.c            | 17 +++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,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: #optional a suffix of the device
+#
+# Returns: Nothing on success
+#          If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ '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 4be4765..2c89a97 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -330,6 +330,30 @@ Example:
 <- { "return": {} }
 
 EQMP
+    {
+        .name       = "set-bootindex",
+        .args_type  = "id:s,bootindex:l,suffix:s?",
+        .mhandler.cmd_new = qmp_marshal_input_set_bootindex,
+    },
+
+SQMP
+set-bootindex
+--------------------
+
+Set bootindex of a device
+
+Arguments:
+
+- "id": the device's ID (json-string)
+- "bootindex": the device's bootindex (json-int)
+- "suffix": the device's suffix in global boot list (json-string, optional)
+
+Example:
+
+-> { "execute": "set-bootindex", "arguments": { "id": "ide0-0-1", "bootindex": 1, "suffix": "/disk@0"}}
+<- { "return": {} }
+
+EQMP
 
     {
         .name       = "send-key",
diff --git a/qmp.c b/qmp.c
index 0d2553a..f2c3c14 100644
--- a/qmp.c
+++ b/qmp.c
@@ -684,6 +684,23 @@ void qmp_object_del(const char *id, Error **errp)
     object_unparent(obj);
 }
 
+void qmp_set_bootindex(const char *id, int64_t bootindex,
+                     bool has_suffix, 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;
+    }
+    if (has_suffix) {
+        modify_boot_device_path(bootindex, dev, suffix);
+    } else {
+        modify_boot_device_path(bootindex, dev, NULL);
+    }
+}
+
 MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
 {
     MemoryDeviceInfoList *head = NULL;
-- 
1.7.12.4

[Qemu-devel] [PATCH 5/6] qemu-monitor: HMP set-bootindex wrapper

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Add HMP set-bootindex wrapper to allow setting
devcie's bootindex via monitor.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Chenliang <chenliang88@huawei.com>
---
 hmp-commands.hx | 15 +++++++++++++++
 hmp.c           | 13 +++++++++++++
 hmp.h           |  1 +
 3 files changed, 29 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d0943b1..31ef24e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -688,6 +688,21 @@ Remove device @var{id}.
 ETEXI
 
     {
+        .name       = "set-bootindex",
+        .args_type  = "id:s,bootindex:l,suffix:s?",
+        .params     = "device bootindex [suffix]",
+        .help       = "set bootindex of a device(e.g. set-bootindex disk0 1 '/disk@0')",
+        .mhandler.cmd = hmp_set_bootindex,
+    },
+
+STEXI
+@item set-bootindex @var{id} @var{bootindex}
+@findex set-bootindex
+
+Set bootindex of a device.
+ETEXI
+
+    {
         .name       = "cpu",
         .args_type  = "index:i",
         .params     = "index",
diff --git a/hmp.c b/hmp.c
index 4d1838e..95f7eeb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1714,3 +1714,16 @@ 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");
+    bool has_suffix = qdict_haskey(qdict, "suffix");
+    const char *suffix = qdict_get_try_str(qdict, "suffix");
+
+    qmp_set_bootindex(id, bootindex, has_suffix, 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);
-- 
1.7.12.4

[Qemu-devel] [PATCH 6/6] spapr: fix possible memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

get_boot_devices_list() will malloc 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 d01978f..edff5ce 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -745,6 +745,7 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
 
     cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
 
+    g_free(bootlist);
     g_free(fdt);
 }
 
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 1/6] bootindex: add {del, modify}_boot_device_path function

[Gerd Hoffmann]

On Do, 2014-07-24 at 16:38 +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> When we want to change one device's bootindex, we should do three
> things. On the on hand, remove the device from global fw_boot_order list,
> regardless attaching suffix or not delete. On the other hand, delete
> original object of the assigned bootindex. Finally add the new device's
> bootindex into the global fw_boot_order list.

Hmm.  I think we should simply lookup the device and modify the
bootindex, leaving the entry as-is otherwise.  In case the new bootindex
is already used by another device just throw an error.

> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> +                          const char *suffix);

Should be del_boot_device_path(DeviceState *dev) and simply delete all
entries belonging to the device.  Patch #3 can be much simpler then as
we can call the function from generic device cleanup code.

> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> +                             const char *suffix);

No need for suffix here if we just update the existing entry.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/6] bootindex: add {del, modify}_boot_device_path function

[Gonglei (Arei)]

Hi, Gerd

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Thursday, July 24, 2014 6:09 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;
> mst@redhat.com; pbonzini@redhat.com; lersek@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: [PATCH 1/6] bootindex: add {del,modify}_boot_device_path
> function
> 
> On Do, 2014-07-24 at 16:38 +0800, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > When we want to change one device's bootindex, we should do three
> > things. On the on hand, remove the device from global fw_boot_order list,
> > regardless attaching suffix or not delete. On the other hand, delete
> > original object of the assigned bootindex. Finally add the new device's
> > bootindex into the global fw_boot_order list.
> 
> Hmm.  I think we should simply lookup the device and modify the
> bootindex, leaving the entry as-is otherwise.  In case the new bootindex
> is already used by another device just throw an error.
> 
If we just throw an error but not change the bootindex is already used,
we cannot achieve our purpose. For example, we configure a hard disk,
which bootindex=1, a nic which bootindex=2. If we want to boot the guest
from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the 
bootindex=1 always be used.
 
> > +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > +                          const char *suffix);
> 
> Should be del_boot_device_path(DeviceState *dev) and simply delete all
> entries belonging to the device.  Patch #3 can be much simpler then as
> we can call the function from generic device cleanup code.
> 
Because the IDE device may configure two kind of disk, HD and CDROM, we
have to distinguish them by suffix.

> > +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> > +                             const char *suffix);
> 
> No need for suffix here if we just update the existing entry.
> 
> cheers,
>   Gerd
> 

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 1/6] bootindex: add {del, modify}_boot_device_path function

[Gerd Hoffmann]

  Hi,

> > Hmm.  I think we should simply lookup the device and modify the
> > bootindex, leaving the entry as-is otherwise.  In case the new bootindex
> > is already used by another device just throw an error.
> > 
> If we just throw an error but not change the bootindex is already used,
> we cannot achieve our purpose. For example, we configure a hard disk,
> which bootindex=1, a nic which bootindex=2. If we want to boot the guest
> from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the 
> bootindex=1 always be used.

No.  The devices are simply sorted by bootindex.  You don't have to use
'1'.  And you can have holes in your numbering and use --for example --
3+5.

So you can start qemu with hd=2,cdrom=3,nic=4, then set nic=1 or cdrom=1
for a guest install, change it back when done.

> > Should be del_boot_device_path(DeviceState *dev) and simply delete all
> > entries belonging to the device.  Patch #3 can be much simpler then as
> > we can call the function from generic device cleanup code.
> > 
> Because the IDE device may configure two kind of disk, HD and CDROM, we
> have to distinguish them by suffix.

Yes, the suffix indicated whenever the device is a disk or cdrom.  But
you'll never have both cdrom+disk paths attached to a single device.
Therefore the device is enough to identify the bootpath entry, you don't
need the suffix for that.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/6] bootindex: add {del, modify}_boot_device_path function

[Gonglei (Arei)]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Thursday, July 24, 2014 9:25 PM
> Subject: Re: [PATCH 1/6] bootindex: add {del,modify}_boot_device_path
> function
> 
>   Hi,
> 
> > > Hmm.  I think we should simply lookup the device and modify the
> > > bootindex, leaving the entry as-is otherwise.  In case the new bootindex
> > > is already used by another device just throw an error.
> > >
> > If we just throw an error but not change the bootindex is already used,
> > we cannot achieve our purpose. For example, we configure a hard disk,
> > which bootindex=1, a nic which bootindex=2. If we want to boot the guest
> > from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the
> > bootindex=1 always be used.
> 
> No.  The devices are simply sorted by bootindex.  You don't have to use
> '1'.  And you can have holes in your numbering and use --for example --
> 3+5.
> 
Yes, you are right. I make a mistake because libvirt will default assign the first 
disk as bootindex=1.

> So you can start qemu with hd=2,cdrom=3,nic=4, then set nic=1 or cdrom=1
> for a guest install, change it back when done.
>
Yes.
 
> > > Should be del_boot_device_path(DeviceState *dev) and simply delete all
> > > entries belonging to the device.  Patch #3 can be much simpler then as
> > > we can call the function from generic device cleanup code.
> > >
> > Because the IDE device may configure two kind of disk, HD and CDROM, we
> > have to distinguish them by suffix.
> 
> Yes, the suffix indicated whenever the device is a disk or cdrom.  But
> you'll never have both cdrom+disk paths attached to a single device.
> Therefore the device is enough to identify the bootpath entry, you don't
> need the suffix for that.
> 
Agreed. Thank you so much, Gerd. 
A new version will be posted later.

Best regards,
-Gonglei