[Qemu-devel] [PATCH v6 00/27] modify boot order of guest, and take effect after rebooting

[Qemu-devel] [PATCH v6 00/27] 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.

This patch series do belows works:
 1. add an fw_cfg_machine_reset() assure re-read global fw_boot_order list
   during vm rebooting.
 2. Set/update bootindex on reset instead of realize/init.
 3. Switch the property from qdev to qom, then use the set
    callback to also update the fw_cfg file.

 Note:
 - Do not support change pci option rom's bootindex.
 - Do not handle those devices which don't have use the bootindex property.

changes since v5:
 rework by Gerd and Markus's suggestion(Thanks a lot):
 - Set/update bootindex on reset instead of realize/init.
 - Switch the property from qdev to qom, then use the set
   callback to also update the fw_cfg file.
 - using qom-set instead of 'set-bootindex' qmp interface,
   remove it.

 This is a huge change relative to the previous version. 

Changes since v4:
 - using error_setg() instead of qerror_report() in patch 1/8.
 - call del_boot_device_path() from device_finalize() instead
  of placing it into each individual device in patch 4/8.

Changes since v3:
 - rework del_* and modify_* function, because of virtio devices' specialation.
   For example, virtio-net's id is NULL, and its parent virtio-net-pci's id was assigned.
   Though the global fw_boot_order stored the virtio-net device.
 - call dell_boot_device_path in each individual device avoiding waste resouce.
 - introduce qmp "query-bootindex" command
 - introcude hmp "info bootindex" command
 - Fixes by Eric's reviewing comments, thanks.

Changes since v2:
 *address Gerd's reviewing suggestion:
 - use the old entry's suffix, if the caller do not pass it in.
 - call del_boot_device_path() from device_finalize() instead
   of placing it into each individual device.

Changes since v1:
 *rework by Gerd's suggestion:
 - split modify and del fw_boot_order for single function.
 - change modify bootindex's realization which simply lookup
   the device and modify the bootindex. if the new bootindex
   has already used by another device just throw an error.
 - change to del_boot_device_path(DeviceState *dev) and simply delete all
   entries belonging to the device.

For Convenience of testing, my test case based on Andreas's patch series:

 [PATCH qom-next 0/4] qom: HMP commands to replace info qtree
 http://thread.gmane.org/gmane.comp.emulators.qemu/271513

However, there is no direct relation with this bootindex patch series.

./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive file=/home/win7_32_2U,if=none,id=drive-ide0-0-0 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -drive file=/home/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-0-1 -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 -vnc 0.0.0.0:10 -netdev type=user,id=net0 -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -drive file=/home/virtio-win-1.5.3.vfd,if=none,id=drive-fdc0-0-0,format=raw -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=5,id=floppy1 -qmp unix:/tmp/qmp,server,nowait -monitor stdio -netdev type=user,id=net1 -device e1000,netdev=net1,bootindex=2,id=nic -boot menu=on
QEMU 2.1.50 monitor - type 'help' for more information
(qemu) qom-get /machine/peripheral/nic1 bootindex
3 (0x3)
(qemu) qom-set /machine/peripheral/nic1 bootindex 3
The bootindex 3 has already been used
(qemu) qom-set /machine/peripheral/nic1 bootindex 0
(qemu) qom-get /machine/peripheral/nic1 bootindex
0 (0x0)
(qemu) qom-set /machine/peripheral/floppy1 bootindexA 3
The bootindex 3 has already been used
(qemu) system_reset
(qemu) qom-get /machine/peripheral/nic1 bootindex
0 (0x0)
(qemu) qom-get /machine/peripheral/floppy1 bootindexA
5 (0x5)
(qemu) qom-set /machine/peripheral/floppy1 bootindexA 3
(qemu) qom-get /machine/peripheral/floppy1 bootindexA
3 (0x3)
(qemu) 


Gonglei (27):
  bootindex: add check bootindex function
  bootindex: add del_boot_device_path function
  fw_cfg: add fw_cfg_machine_reset function
  bootindex: rework add_boot_device_path function
  bootindex: support to set a existent device's bootindex to -1
  bootindex: move setting bootindex on reset() instead of
    realize/init()
  vl.c: add setter/getter functions for bootindex property
  virtio-net: add bootindex to qom property
  e1000: add bootindex to qom property
  eepro100: add bootindex to qom property
  ne2000: add bootindex to qom property
  pcnet: add bootindex to qom property
  rtl8139: add bootindex to qom property
  spapr_lian: add bootindex to qom property
  vmxnet3: add bootindex to qom property
  usb-net: add bootindex to qom property
  net: remove bootindex property from qdev to qom
  host-libusb: remove bootindex property from qdev to qom
  pci-assign: remove bootindex property from qdev to qom
  vfio: remove bootindex property from qdev to qom
  redirect: remove bootindex property from qdev to qom
  isa-fdc: remove bootindexA/B property from qdev to qom
  ide: add bootindex to qom property
  scsi: add bootindex to qom property
  virtio-blk: add bootindex to qom property
  block: remove bootindex property from qdev to qom
  bootindex: delete bootindex when device is removed

 hw/block/fdc.c            |  53 +++++++++++++++++++++---
 hw/block/virtio-blk.c     |  24 ++++++++++-
 hw/core/qdev.c            |   4 ++
 hw/i386/kvm/pci-assign.c  |  29 ++++++++++++--
 hw/ide/qdev.c             |  43 ++++++++++++++++++--
 hw/misc/vfio.c            |  29 +++++++++++++-
 hw/net/e1000.c            |  31 +++++++++++++-
 hw/net/eepro100.c         |  37 +++++++++++++++--
 hw/net/lance.c            |  29 ++++++++++++++
 hw/net/ne2000.c           |  38 +++++++++++++++++-
 hw/net/pcnet-pci.c        |  31 ++++++++++++++
 hw/net/pcnet.c            |  19 ++++++++-
 hw/net/pcnet.h            |   5 +++
 hw/net/rtl8139.c          |  29 +++++++++++++-
 hw/net/spapr_llan.c       |  29 +++++++++++++-
 hw/net/virtio-net.c       |  26 +++++++++++-
 hw/net/vmxnet3.c          |  29 +++++++++++++-
 hw/nvram/fw_cfg.c         |  55 +++++++++++++++++++++++--
 hw/scsi/scsi-disk.c       |  34 +++++++++++++++-
 hw/scsi/scsi-generic.c    |   9 +++--
 hw/usb/dev-network.c      |  29 +++++++++++++-
 hw/usb/host-libusb.c      |  29 +++++++++++++-
 hw/usb/redirect.c         |  31 +++++++++++++-
 hw/virtio/virtio-pci.c    |  23 +++++++++++
 include/hw/block/block.h  |   1 -
 include/hw/nvram/fw_cfg.h |   2 +
 include/hw/scsi/scsi.h    |   1 +
 include/net/net.h         |   4 +-
 include/sysemu/sysemu.h   |   6 +++
 vl.c                      | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 30 files changed, 759 insertions(+), 50 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v6 01/27] bootindex: add check bootindex function

[arei.gonglei]

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

Determine whether a given bootindex exists or not.
If exists, we report an error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 include/sysemu/sysemu.h |  1 +
 vl.c                    | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..b343df5 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -207,6 +207,7 @@ void do_usb_add(Monitor *mon, const QDict *qdict);
 void do_usb_del(Monitor *mon, const QDict *qdict);
 void usb_info(Monitor *mon, const QDict *qdict);
 
+void check_boot_index(int32_t bootindex, Error **errp);
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
 char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
diff --git a/vl.c b/vl.c
index 95be92d..76fb008 100644
--- a/vl.c
+++ b/vl.c
@@ -1237,6 +1237,21 @@ static void restore_boot_order(void *opaque)
     g_free(normal_boot_order);
 }
 
+void check_boot_index(int32_t bootindex, Error **errp)
+{
+    FWBootEntry *i;
+
+    if (bootindex >= 0) {
+        QTAILQ_FOREACH(i, &fw_boot_order, link) {
+            if (i->bootindex == bootindex) {
+                error_setg(errp, "The bootindex %d has already been used",
+                           bootindex);
+                return;
+            }
+        }
+    }
+}
+
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix)
 {
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[arei.gonglei]

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

Introduce del_boot_device_path() to clean up fw_cfg content when
hot-unplugging a device that refers to a bootindex.

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

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b343df5..672984c 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -208,6 +208,7 @@ void do_usb_del(Monitor *mon, const QDict *qdict);
 void usb_info(Monitor *mon, const QDict *qdict);
 
 void check_boot_index(int32_t bootindex, Error **errp);
+void del_boot_device_path(DeviceState *dev);
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
 char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
diff --git a/vl.c b/vl.c
index 76fb008..5e57541 100644
--- a/vl.c
+++ b/vl.c
@@ -1252,6 +1252,42 @@ void check_boot_index(int32_t bootindex, Error **errp)
     }
 }
 
+static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
+{
+    bool ret = false;
+    char *devpath_src = qdev_get_fw_dev_path(src);
+    char *devpath_dst = qdev_get_fw_dev_path(dst);
+
+    if (!strcmp(devpath_src, devpath_dst)) {
+        ret = true;
+    }
+
+    g_free(devpath_src);
+    g_free(devpath_dst);
+    return ret;
+}
+
+void del_boot_device_path(DeviceState *dev)
+{
+    FWBootEntry *i;
+
+    assert(dev != NULL);
+
+    /* remove all entries of the assigned device */
+    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        if (i->dev == NULL) {
+            continue;
+        }
+        if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) {
+            QTAILQ_REMOVE(&fw_boot_order, i, link);
+            g_free(i->suffix);
+            g_free(i);
+
+            break;
+        }
+    }
+}
+
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix)
 {
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 03/27] 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         | 55 ++++++++++++++++++++++++++++++++++++++++++++---
 include/hw/nvram/fw_cfg.h |  2 ++
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b71d251..e7ed27e 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -402,6 +402,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 +519,42 @@ 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);
+}
+
+static void fw_cfg_machine_ready(struct Notifier *n, void *data)
+{
+    FWCfgState *s = container_of(n, FWCfgState, machine_ready);
+    qemu_register_reset(fw_cfg_machine_reset, s);
 }
 
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
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 v6 04/27] bootindex: rework add_boot_device_path function

[arei.gonglei]

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

Add the function of updating bootindex about fw_boot_order list
in add_boot_device_path(). We should delete the old one if a
device has existed in global fw_boot_order list.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 vl.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/vl.c b/vl.c
index 5e57541..4d5a889 100644
--- a/vl.c
+++ b/vl.c
@@ -1288,6 +1288,25 @@ void del_boot_device_path(DeviceState *dev)
     }
 }
 
+static void del_original_boot_device(DeviceState *dev, const char *suffix)
+{
+    FWBootEntry *i;
+
+    if (dev == NULL) {
+        return;
+    }
+
+    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        if (i->dev == dev && !strcmp(i->suffix, suffix)) {
+            QTAILQ_REMOVE(&fw_boot_order, i, link);
+            g_free(i->suffix);
+            g_free(i);
+
+            break;
+        }
+    }
+}
+
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix)
 {
@@ -1299,6 +1318,8 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
 
     assert(dev != NULL || suffix != NULL);
 
+    del_original_boot_device(dev, suffix);
+
     node = g_malloc0(sizeof(FWBootEntry));
     node->bootindex = bootindex;
     node->suffix = g_strdup(suffix);
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 05/27] bootindex: support to set a existent device's bootindex to -1

[arei.gonglei]

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

When set a device's bootindex to -1, we remove it from global
fw_boot_order list.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 vl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vl.c b/vl.c
index 4d5a889..f2c3b2d 100644
--- a/vl.c
+++ b/vl.c
@@ -1313,6 +1313,7 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
     FWBootEntry *node, *i;
 
     if (bootindex < 0) {
+        del_original_boot_device(dev, suffix);
         return;
     }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 06/27] bootindex: move setting bootindex on reset() instead of realize/init()

[arei.gonglei]

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

Only in this way, we can assure the new bootindex take effect
during vm rebooting.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/block/fdc.c           |  6 +++---
 hw/block/virtio-blk.c    |  4 ++--
 hw/i386/kvm/pci-assign.c |  4 ++--
 hw/ide/qdev.c            | 14 +++++++++++---
 hw/misc/vfio.c           |  3 ++-
 hw/net/e1000.c           |  4 ++--
 hw/net/eepro100.c        | 10 ++++++++--
 hw/net/lance.c           |  2 ++
 hw/net/ne2000.c          | 11 +++++++++--
 hw/net/pcnet-pci.c       |  3 +++
 hw/net/pcnet.c           |  7 +++++--
 hw/net/pcnet.h           |  1 +
 hw/net/rtl8139.c         |  4 ++--
 hw/net/spapr_llan.c      |  4 ++--
 hw/net/virtio-net.c      |  5 +++--
 hw/net/vmxnet3.c         |  4 ++--
 hw/scsi/scsi-disk.c      |  3 ++-
 hw/scsi/scsi-generic.c   |  7 ++++---
 hw/usb/dev-network.c     |  4 +++-
 hw/usb/host-libusb.c     |  3 ++-
 hw/usb/redirect.c        |  3 ++-
 21 files changed, 72 insertions(+), 34 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 490d127..8add4a1 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -787,6 +787,9 @@ static void fdctrl_external_reset_isa(DeviceState *d)
     FDCtrl *s = &isa->state;
 
     fdctrl_reset(s, 0);
+
+    add_boot_device_path(isa->bootindexA, d, "/floppy@0");
+    add_boot_device_path(isa->bootindexB, d, "/floppy@1");
 }
 
 static void fdctrl_handle_tc(void *opaque, int irq, int level)
@@ -2142,9 +2145,6 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
-
-    add_boot_device_path(isa->bootindexA, dev, "/floppy@0");
-    add_boot_device_path(isa->bootindexB, dev, "/floppy@1");
 }
 
 static void sysbus_fdc_initfn(Object *obj)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index a7f2827..e74a4c5 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -509,6 +509,8 @@ static void virtio_blk_reset(VirtIODevice *vdev)
      */
     bdrv_drain_all();
     bdrv_set_enable_write_cache(s->bs, s->original_wce);
+
+    add_boot_device_path(s->conf->bootindex, DEVICE(vdev), "/disk@0,0");
 }
 
 /* coalesce internal state, copy to pci i/o region 0
@@ -777,8 +779,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     bdrv_set_guest_block_size(s->bs, s->conf->logical_block_size);
 
     bdrv_iostatus_enable(s->bs);
-
-    add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
 }
 
 static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 17c7d6dc..0e6aa8a 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1740,6 +1740,8 @@ static void reset_assigned_device(DeviceState *dev)
      * disconnected from the PCI bus. This avoids further DMA transfers.
      */
     assigned_dev_pci_write_config(pci_dev, PCI_COMMAND, 0, 1);
+
+    add_boot_device_path(adev->bootindex, dev, NULL);
 }
 
 static int assigned_initfn(struct PCIDevice *pci_dev)
@@ -1825,8 +1827,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 
     assigned_dev_load_option_rom(dev);
 
-    add_boot_device_path(dev->bootindex, &pci_dev->qdev, NULL);
-
     return 0;
 
 assigned_out:
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b4a4671..83bdb97 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -185,12 +185,17 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         dev->serial = g_strdup(s->drive_serial_str);
     }
 
-    add_boot_device_path(dev->conf.bootindex, &dev->qdev,
-                         dev->unit ? "/disk@1" : "/disk@0");
-
     return 0;
 }
 
+static void ide_dev_reset(DeviceState *dev)
+{
+    IDEDevice *d = DO_UPCAST(IDEDevice, qdev, dev);
+
+    add_boot_device_path(d->conf.bootindex, dev,
+                         d->unit ? "/disk@1" : "/disk@0");
+}
+
 static int ide_hd_initfn(IDEDevice *dev)
 {
     return ide_dev_initfn(dev, IDE_HD);
@@ -231,6 +236,7 @@ static void ide_hd_class_init(ObjectClass *klass, void *data)
     dc->fw_name = "drive";
     dc->desc = "virtual IDE disk";
     dc->props = ide_hd_properties;
+    dc->reset = ide_dev_reset;
 }
 
 static const TypeInfo ide_hd_info = {
@@ -253,6 +259,7 @@ static void ide_cd_class_init(ObjectClass *klass, void *data)
     dc->fw_name = "drive";
     dc->desc = "virtual IDE CD-ROM";
     dc->props = ide_cd_properties;
+    dc->reset = ide_dev_reset;
 }
 
 static const TypeInfo ide_cd_info = {
@@ -275,6 +282,7 @@ static void ide_drive_class_init(ObjectClass *klass, void *data)
     dc->fw_name = "drive";
     dc->desc = "virtual IDE disk or CD-ROM (legacy)";
     dc->props = ide_drive_properties;
+    dc->reset = ide_dev_reset;
 }
 
 static const TypeInfo ide_drive_info = {
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 40dcaa6..625598a 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4296,7 +4296,6 @@ static int vfio_initfn(PCIDevice *pdev)
         }
     }
 
-    add_boot_device_path(vdev->bootindex, &pdev->qdev, NULL);
     vfio_register_err_notifier(vdev);
 
     return 0;
@@ -4341,6 +4340,8 @@ static void vfio_pci_reset(DeviceState *dev)
 
     vfio_pci_pre_reset(vdev);
 
+    add_boot_device_path(vdev->bootindex, dev, NULL);
+
     if (vdev->reset_works && (vdev->has_flr || !vdev->has_pm_reset) &&
         !ioctl(vdev->fd, VFIO_DEVICE_RESET)) {
         DPRINTF("%04x:%02x:%02x.%x FLR/VFIO_DEVICE_RESET\n", vdev->host.domain,
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 272df00..afdfaa3 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1569,8 +1569,6 @@ static int pci_e1000_init(PCIDevice *pci_dev)
 
     qemu_format_nic_info_str(qemu_get_queue(d->nic), macaddr);
 
-    add_boot_device_path(d->conf.bootindex, dev, "/ethernet-phy@0");
-
     d->autoneg_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, e1000_autoneg_timer, d);
     d->mit_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, e1000_mit_timer, d);
 
@@ -1581,6 +1579,8 @@ static void qdev_e1000_reset(DeviceState *dev)
 {
     E1000State *d = E1000(dev);
     e1000_reset(d);
+
+    add_boot_device_path(d->conf.bootindex, dev, "/ethernet-phy@0");
 }
 
 static Property e1000_properties[] = {
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 3cd826a..dcfaa48 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1901,11 +1901,16 @@ static int e100_nic_init(PCIDevice *pci_dev)
     s->vmstate->name = qemu_get_queue(s->nic)->model;
     vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
 
-    add_boot_device_path(s->conf.bootindex, &pci_dev->qdev, "/ethernet-phy@0");
-
     return 0;
 }
 
+static void eepro100_reset(DeviceState *dev)
+{
+    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, PCI_DEVICE(dev));
+
+    add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
+}
+
 static E100PCIDeviceInfo e100_devices[] = {
     {
         .name = "i82550",
@@ -2082,6 +2087,7 @@ static void eepro100_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
     dc->props = e100_properties;
     dc->desc = info->desc;
+    dc->reset = eepro100_reset;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
     k->romfile = "pxe-eepro100.rom";
diff --git a/hw/net/lance.c b/hw/net/lance.c
index 7811a9e..3d921b0 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -141,6 +141,8 @@ static void lance_reset(DeviceState *dev)
     SysBusPCNetState *d = SYSBUS_PCNET(dev);
 
     pcnet_h_reset(&d->state);
+
+    pcnet_common_reset(dev, s);
 }
 
 static Property lance_properties[] = {
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index a62d12d..a6fc3c0 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -738,11 +738,17 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
                           object_get_typename(OBJECT(pci_dev)), pci_dev->qdev.id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->c.macaddr.a);
 
-    add_boot_device_path(s->c.bootindex, &pci_dev->qdev, "/ethernet-phy@0");
-
     return 0;
 }
 
+static void pci_ne2000_reset(DeviceState *dev)
+{
+    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, PCI_DEVICE(dev));
+    NE2000State *s = &d->ne2000;
+
+    add_boot_device_path(s->c.bootindex, dev, "/ethernet-phy@0");
+}
+
 static void pci_ne2000_exit(PCIDevice *pci_dev)
 {
     PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
@@ -770,6 +776,7 @@ static void ne2000_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
     dc->vmsd = &vmstate_pci_ne2000;
     dc->props = ne2000_properties;
+    dc->reset = pci_ne2000_reset;
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
 }
 
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index 50ffe91..e90344e 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -340,8 +340,11 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
 static void pci_reset(DeviceState *dev)
 {
     PCIPCNetState *d = PCI_PCNET(dev);
+    PCNetState *s = &d->state;
 
     pcnet_h_reset(&d->state);
+
+    pcnet_common_reset(dev, s);
 }
 
 static Property pcnet_properties[] = {
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 5299d52..4fd2e52 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1735,8 +1735,6 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
     s->nic = qemu_new_nic(info, &s->conf, object_get_typename(OBJECT(dev)), dev->id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
-    add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
-
     /* Initialize the PROM */
 
     /*
@@ -1768,3 +1766,8 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
 
     return 0;
 }
+
+void pcnet_common_reset(DeviceState *dev, PCNetState *s)
+{
+    add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
+}
diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
index 9dee6f3..ce9b9cb 100644
--- a/hw/net/pcnet.h
+++ b/hw/net/pcnet.h
@@ -65,6 +65,7 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_);
 void pcnet_set_link_status(NetClientState *nc);
 void pcnet_common_cleanup(PCNetState *d);
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
+void pcnet_common_reset(DeviceState *dev, PCNetState *s);
 extern const VMStateDescription vmstate_pcnet;
 
 #endif
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 6e59f38..b6d3930 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -1285,6 +1285,8 @@ static void rtl8139_reset(DeviceState *d)
 
     /* reset tally counters */
     RTL8139TallyCounters_clear(&s->tally_counters);
+
+    add_boot_device_path(s->conf.bootindex, d, "/ethernet-phy@0");
 }
 
 static void RTL8139TallyCounters_clear(RTL8139TallyCounters* counters)
@@ -3538,8 +3540,6 @@ static int pci_rtl8139_init(PCIDevice *dev)
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, rtl8139_timer, s);
     rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
 
-    add_boot_device_path(s->conf.bootindex, d, "/ethernet-phy@0");
-
     return 0;
 }
 
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 2d47df6..62c0249 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -202,6 +202,8 @@ static void spapr_vlan_reset(VIOsPAPRDevice *sdev)
     dev->buf_list = 0;
     dev->rx_bufs = 0;
     dev->isopen = 0;
+
+    add_boot_device_path(dev->nicconf.bootindex, DEVICE(dev), "");
 }
 
 static int spapr_vlan_init(VIOsPAPRDevice *sdev)
@@ -214,8 +216,6 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
                             object_get_typename(OBJECT(sdev)), sdev->qdev.id, dev);
     qemu_format_nic_info_str(qemu_get_queue(dev->nic), dev->nicconf.macaddr.a);
 
-    add_boot_device_path(dev->nicconf.bootindex, DEVICE(dev), "");
-
     return 0;
 }
 
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 268eff9..09ecb24 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -332,6 +332,9 @@ static void virtio_net_reset(VirtIODevice *vdev)
     memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
     qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
     memset(n->vlans, 0, MAX_VLAN >> 3);
+
+    add_boot_device_path(n->nic_conf.bootindex, DEVICE(vdev),
+                         "/ethernet-phy@0");
 }
 
 static void peer_test_vnet_hdr(VirtIONet *n)
@@ -1640,8 +1643,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     n->qdev = dev;
     register_savevm(dev, "virtio-net", -1, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
-
-    add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
 }
 
 static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index f246fa1..e82ef94 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2172,8 +2172,6 @@ static int vmxnet3_pci_init(PCIDevice *pci_dev)
     register_savevm(dev, "vmxnet3-msix", -1, 1,
                     vmxnet3_msix_save, vmxnet3_msix_load, s);
 
-    add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
-
     return 0;
 }
 
@@ -2201,6 +2199,8 @@ static void vmxnet3_qdev_reset(DeviceState *dev)
 
     VMW_CBPRN("Starting QDEV reset...");
     vmxnet3_reset(s);
+
+    add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 }
 
 static bool vmxnet3_mc_list_needed(void *opaque)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e34a544..03d1384 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2149,6 +2149,8 @@ static void scsi_disk_reset(DeviceState *dev)
     /* reset tray statuses */
     s->tray_locked = 0;
     s->tray_open = 0;
+
+    add_boot_device_path(s->qdev.conf.bootindex, dev, NULL);
 }
 
 static void scsi_unrealize(SCSIDevice *dev, Error **errp)
@@ -2285,7 +2287,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     bdrv_set_guest_block_size(s->qdev.conf.bs, s->qdev.blocksize);
 
     bdrv_iostatus_enable(s->qdev.conf.bs);
-    add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, NULL);
 }
 
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 20587b4..db6a80d 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -381,6 +381,10 @@ static void scsi_generic_reset(DeviceState *dev)
     SCSIDevice *s = SCSI_DEVICE(dev);
 
     scsi_device_purge_requests(s, SENSE_CODE(RESET));
+
+    if (s->type == TYPE_DISK || s->type == TYPE_ROM) {
+        add_boot_device_path(s->conf.bootindex, dev, NULL);
+    }
 }
 
 static void scsi_unrealize(SCSIDevice *s, Error **errp)
@@ -431,9 +435,6 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp)
     /* define device state */
     s->type = scsiid.scsi_type;
     DPRINTF("device type %d\n", s->type);
-    if (s->type == TYPE_DISK || s->type == TYPE_ROM) {
-        add_boot_device_path(s->conf.bootindex, &s->qdev, NULL);
-    }
 
     switch (s->type) {
     case TYPE_TAPE:
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 518d536..ea038ab 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1053,6 +1053,9 @@ static int rndis_parse(USBNetState *s, uint8_t *data, int length)
 
 static void usb_net_handle_reset(USBDevice *dev)
 {
+    USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
+
+    add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0");
 }
 
 static void usb_net_handle_control(USBDevice *dev, USBPacket *p,
@@ -1372,7 +1375,6 @@ static int usb_net_initfn(USBDevice *dev)
              s->conf.macaddr.a[5]);
     usb_desc_set_string(dev, STRING_ETHADDR, s->usbstring_mac);
 
-    add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0");
     return 0;
 }
 
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index c189147..f37708d 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -975,7 +975,6 @@ static int usb_host_initfn(USBDevice *udev)
     qemu_add_exit_notifier(&s->exit);
 
     QTAILQ_INSERT_TAIL(&hostdevs, s, next);
-    add_boot_device_path(s->bootindex, &udev->qdev, NULL);
     usb_host_auto_check(NULL);
     return 0;
 }
@@ -1354,6 +1353,8 @@ static void usb_host_handle_reset(USBDevice *udev)
     if (rc != 0) {
         usb_host_nodev(s);
     }
+
+    add_boot_device_path(s->bootindex, &udev->qdev, NULL);
 }
 
 static int usb_host_alloc_streams(USBDevice *udev, USBEndpoint **eps,
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 44522d9..92dd353 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -505,6 +505,8 @@ static void usbredir_handle_reset(USBDevice *udev)
     DPRINTF("reset device\n");
     usbredirparser_send_reset(dev->parser);
     usbredirparser_do_write(dev->parser);
+
+    add_boot_device_path(dev->bootindex, &udev->qdev, NULL);
 }
 
 static void usbredir_handle_iso_data(USBRedirDevice *dev, USBPacket *p,
@@ -1397,7 +1399,6 @@ static int usbredir_initfn(USBDevice *udev)
                           usbredir_chardev_read, usbredir_chardev_event, dev);
 
     qemu_add_vm_change_state_handler(usbredir_vm_state_change, dev);
-    add_boot_device_path(dev->bootindex, &udev->qdev, NULL);
     return 0;
 }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[arei.gonglei]

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

when we remove bootindex form qdev.property to qom.property,
we can use those functions set/get bootindex property for all
correlative devices.

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

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 672984c..ca231e4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict *qdict);
 void usb_info(Monitor *mon, const QDict *qdict);
 
 void check_boot_index(int32_t bootindex, Error **errp);
+void get_bootindex(int32_t *bootindex, Visitor *v,
+                   const char *name, Error **errp);
+void set_bootindex(int32_t *bootindex, Visitor *v,
+                   const char *name, Error **errp);
 void del_boot_device_path(DeviceState *dev);
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
diff --git a/vl.c b/vl.c
index f2c3b2d..4363185 100644
--- a/vl.c
+++ b/vl.c
@@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error **errp)
     }
 }
 
+void get_bootindex(int32_t *bootindex, Visitor *v,
+                   const char *name, Error **errp)
+{
+    visit_type_int32(v, bootindex, name, errp);
+}
+
+void set_bootindex(int32_t *bootindex, Visitor *v,
+                   const char *name, Error **errp)
+{
+    int32_t boot_index;
+    Error *local_err = NULL;
+
+    visit_type_int32(v, &boot_index, name, &local_err);
+
+    if (local_err == NULL) {
+        /* check the bootindex existes or not in fw_boot_order list  */
+        check_boot_index(boot_index, &local_err);
+    }
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    /* change bootindex to a new one */
+    *bootindex = boot_index;
+}
+
 static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
 {
     bool ret = false;
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 08/27] virtio-net: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/virtio-net.c    | 20 ++++++++++++++++++++
 hw/virtio/virtio-pci.c | 22 ++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 09ecb24..8b409ad 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1685,6 +1685,22 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
+static void virtio_net_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    VirtIONet *n = VIRTIO_NET(obj);
+
+    get_bootindex(&n->nic_conf.bootindex, v, name, errp);
+}
+
+static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    VirtIONet *n = VIRTIO_NET(obj);
+
+    set_bootindex(&n->nic_conf.bootindex, v, name, errp);
+}
+
 static void virtio_net_instance_init(Object *obj)
 {
     VirtIONet *n = VIRTIO_NET(obj);
@@ -1694,6 +1710,10 @@ static void virtio_net_instance_init(Object *obj)
      * Can be overriden with virtio_net_set_config_size.
      */
     n->config_size = sizeof(struct virtio_net_config);
+
+    object_property_add(obj, "bootindex", "int",
+                        virtio_net_get_bootindex,
+                        virtio_net_set_bootindex, NULL, NULL, NULL);
 }
 
 static Property virtio_net_properties[] = {
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ddb5da1..ca8bdfd 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1451,11 +1451,33 @@ static void virtio_net_pci_class_init(ObjectClass *klass, void *data)
     vpciklass->init = virtio_net_pci_init;
 }
 
+static void virtio_net_pci_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                         const char *name, Error **errp)
+{
+    VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
+    VirtIONet *n = &dev->vdev;
+
+     get_bootindex(&n->nic_conf.bootindex, v, name, errp);
+}
+
+static void virtio_net_pci_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                         const char *name, Error **errp)
+{
+    VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
+    VirtIONet *n = &dev->vdev;
+
+     set_bootindex(&n->nic_conf.bootindex, v, name, errp);
+}
+
 static void virtio_net_pci_instance_init(Object *obj)
 {
     VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
     object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_NET);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+
+    object_property_add(obj, "bootindex", "int",
+                        virtio_net_pci_get_bootindex,
+                        virtio_net_pci_set_bootindex, NULL, NULL, NULL);
 }
 
 static const TypeInfo virtio_net_pci_info = {
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 09/27] e1000: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/e1000.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index afdfaa3..ed87b74 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1583,6 +1583,30 @@ static void qdev_e1000_reset(DeviceState *dev)
     add_boot_device_path(d->conf.bootindex, dev, "/ethernet-phy@0");
 }
 
+static void e1000_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                const char *name, Error **errp)
+{
+    E1000State *n = E1000(obj);
+
+    get_bootindex(&n->conf.bootindex, v, name, errp);
+}
+
+static void e1000_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                const char *name, Error **errp)
+{
+    E1000State *n = E1000(obj);
+
+    set_bootindex(&n->conf.bootindex, v, name, errp);
+}
+
+
+static void e1000_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        e1000_get_bootindex,
+                        e1000_set_bootindex, NULL, NULL, NULL);
+}
+
 static Property e1000_properties[] = {
     DEFINE_NIC_PROPERTIES(E1000State, conf),
     DEFINE_PROP_BIT("autonegotiation", E1000State,
@@ -1625,6 +1649,7 @@ static const TypeInfo e1000_base_info = {
     .name          = TYPE_E1000_BASE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(E1000State),
+    .instance_init = e1000_instance_init,
     .class_size    = sizeof(E1000BaseClass),
     .abstract      = true,
 };
@@ -1668,6 +1693,7 @@ static void e1000_register_types(void)
         type_info.parent = TYPE_E1000_BASE;
         type_info.class_data = (void *)info;
         type_info.class_init = e1000_class_init;
+        type_info.instance_init = e1000_instance_init;
 
         type_register(&type_info);
     }
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 10/27] eepro100: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-ooff-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/eepro100.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index dcfaa48..5a29f20 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1911,6 +1911,29 @@ static void eepro100_reset(DeviceState *dev)
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 }
 
+static void eepro100_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, PCI_DEVICE(obj));
+
+    get_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void eepro100_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, PCI_DEVICE(obj));
+
+    set_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void eepro100_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        eepro100_get_bootindex,
+                        eepro100_set_bootindex, NULL, NULL, NULL);
+}
+
 static E100PCIDeviceInfo e100_devices[] = {
     {
         .name = "i82550",
@@ -2110,7 +2133,8 @@ static void eepro100_register_types(void)
         type_info.parent = TYPE_PCI_DEVICE;
         type_info.class_init = eepro100_class_init;
         type_info.instance_size = sizeof(EEPRO100State);
-        
+        type_info.instance_init = eepro100_instance_init;
+
         type_register(&type_info);
     }
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 11/27] ne2000: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/ne2000.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index a6fc3c0..3ddb494 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -758,6 +758,31 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
     qemu_free_irq(s->irq);
 }
 
+static void ne2000_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                 const char *name, Error **errp)
+{
+    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, PCI_DEVICE(obj));
+    NE2000State *s = &d->ne2000;
+
+    get_bootindex(&s->c.bootindex, v, name, errp);
+}
+
+static void ne2000_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                 const char *name, Error **errp)
+{
+    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, PCI_DEVICE(obj));
+    NE2000State *s = &d->ne2000;
+
+    set_bootindex(&s->c.bootindex, v, name, errp);
+}
+
+static void ne2000_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        ne2000_get_bootindex,
+                        ne2000_set_bootindex, NULL, NULL, NULL);
+}
+
 static Property ne2000_properties[] = {
     DEFINE_NIC_PROPERTIES(PCINE2000State, ne2000.c),
     DEFINE_PROP_END_OF_LIST(),
@@ -785,6 +810,7 @@ static const TypeInfo ne2000_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCINE2000State),
     .class_init    = ne2000_class_init,
+    .instance_init = ne2000_instance_init,
 };
 
 static void ne2000_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 12/27] pcnet: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/lance.c     | 26 ++++++++++++++++++++++++++
 hw/net/pcnet-pci.c | 27 +++++++++++++++++++++++++++
 hw/net/pcnet.c     | 12 ++++++++++++
 hw/net/pcnet.h     |  4 ++++
 4 files changed, 69 insertions(+)

diff --git a/hw/net/lance.c b/hw/net/lance.c
index 3d921b0..3071b3f 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -145,6 +145,31 @@ static void lance_reset(DeviceState *dev)
     pcnet_common_reset(dev, s);
 }
 
+static void lance_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                const char *name, Error **errp)
+{
+    SysBusPCNetState *d = SYSBUS_PCNET(obj);
+    PCNetState *s = &d->state;
+
+    pcnet_common_set_bootindex(s, v, name, errp);
+}
+
+static void lance_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                const char *name, Error **errp)
+{
+    SysBusPCNetState *d = SYSBUS_PCNET(dev);
+    PCNetState *s = &d->state;
+
+    pcnet_common_set_bootindex(s, v, name, errp);
+}
+
+static void lance_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        lance_get_bootindex,
+                        lance_set_bootindex, NULL, NULL, NULL);
+}
+
 static Property lance_properties[] = {
     DEFINE_PROP_PTR("dma", SysBusPCNetState, state.dma_opaque),
     DEFINE_NIC_PROPERTIES(SysBusPCNetState, state.conf),
@@ -171,6 +196,7 @@ static const TypeInfo lance_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusPCNetState),
     .class_init    = lance_class_init,
+    .instance_init = lance_instance_init,
 };
 
 static void lance_register_types(void)
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index e90344e..ca744c8 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -33,6 +33,7 @@
 #include "qemu/timer.h"
 #include "sysemu/dma.h"
 
+
 #include "pcnet.h"
 
 //#define PCNET_DEBUG
@@ -347,6 +348,31 @@ static void pci_reset(DeviceState *dev)
     pcnet_common_reset(dev, s);
 }
 
+static void pcnet_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                const char *name, Error **errp)
+{
+    PCIPCNetState *d = PCI_PCNET(obj);
+    PCNetState *s = &d->state;
+
+    pcnet_common_set_bootindex(s, v, name, errp);
+}
+
+static void pcnet_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                const char *name, Error **errp)
+{
+    PCIPCNetState *d = PCI_PCNET(obj);
+    PCNetState *s = &d->state;
+
+    pcnet_common_set_bootindex(s, v, name, errp);
+}
+
+static void pcnet_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        pcnet_get_bootindex,
+                        pcnet_set_bootindex, NULL, NULL, NULL);
+}
+
 static Property pcnet_properties[] = {
     DEFINE_NIC_PROPERTIES(PCIPCNetState, state.conf),
     DEFINE_PROP_END_OF_LIST(),
@@ -375,6 +401,7 @@ static const TypeInfo pcnet_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCIPCNetState),
     .class_init    = pcnet_class_init,
+    .instance_init = pcnet_instance_init,
 };
 
 static void pci_pcnet_register_types(void)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 4fd2e52..6273abc 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1771,3 +1771,15 @@ void pcnet_common_reset(DeviceState *dev, PCNetState *s)
 {
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 }
+
+void pcnet_common_get_bootindex(PCNetState *s, Visitor *v,
+                                const char *name, Error **errp)
+{
+    get_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+void pcnet_common_set_bootindex(PCNetState *s, Visitor *v,
+                                const char *name, Error **errp)
+{
+    set_bootindex(&s->conf.bootindex, v, name, errp);
+}
diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
index ce9b9cb..6c2baf8 100644
--- a/hw/net/pcnet.h
+++ b/hw/net/pcnet.h
@@ -66,6 +66,10 @@ void pcnet_set_link_status(NetClientState *nc);
 void pcnet_common_cleanup(PCNetState *d);
 int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
 void pcnet_common_reset(DeviceState *dev, PCNetState *s);
+void pcnet_common_get_bootindex(PCNetState *s, Visitor *v,
+                                const char *name, Error **errp);
+void pcnet_common_set_bootindex(PCNetState *s, Visitor *v,
+                                const char *name, Error **errp);
 extern const VMStateDescription vmstate_pcnet;
 
 #endif
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 13/27] rtl8139: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/rtl8139.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index b6d3930..eba60d1 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3543,6 +3543,29 @@ static int pci_rtl8139_init(PCIDevice *dev)
     return 0;
 }
 
+static void rtl8139_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    RTL8139State *s = RTL8139(obj);
+
+    get_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void rtl8139_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    RTL8139State *s = RTL8139(obj);
+
+    set_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void rtl8139_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        rtl8139_get_bootindex,
+                        rtl8139_set_bootindex, NULL, NULL, NULL);
+}
+
 static Property rtl8139_properties[] = {
     DEFINE_NIC_PROPERTIES(RTL8139State, conf),
     DEFINE_PROP_END_OF_LIST(),
@@ -3571,6 +3594,7 @@ static const TypeInfo rtl8139_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(RTL8139State),
     .class_init    = rtl8139_class_init,
+    .instance_init = rtl8139_instance_init,
 };
 
 static void rtl8139_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 14/27] spapr_lian: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/spapr_llan.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 62c0249..773404a 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -219,6 +219,29 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
     return 0;
 }
 
+static void spapr_vlan_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    VIOsPAPRVLANDevice *dev = VIO_SPAPR_VLAN_DEVICE(obj);
+
+    get_bootindex(&dev->nicconf.bootindex, v, name, errp);
+}
+
+static void spapr_vlan_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    VIOsPAPRVLANDevice *dev = VIO_SPAPR_VLAN_DEVICE(obj);
+
+    set_bootindex(&dev->nicconf.bootindex, v, name, errp);
+}
+
+static void spapr_vlan_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        spapr_vlan_get_bootindex,
+                        spapr_vlan_set_bootindex, NULL, NULL, NULL);
+}
+
 void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd)
 {
     DeviceState *dev;
@@ -546,6 +569,7 @@ static const TypeInfo spapr_vlan_info = {
     .parent        = TYPE_VIO_SPAPR_DEVICE,
     .instance_size = sizeof(VIOsPAPRVLANDevice),
     .class_init    = spapr_vlan_class_init,
+    .instance_init = spapr_vlan_instance_init,
 };
 
 static void spapr_vlan_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 15/27] vmxnet3: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/vmxnet3.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index e82ef94..a9ed593 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2203,6 +2203,29 @@ static void vmxnet3_qdev_reset(DeviceState *dev)
     add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0");
 }
 
+static void vmxnet3_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    VMXNET3State *s = VMXNET3(obj);
+
+    get_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void vmxnet3_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    VMXNET3State *s = VMXNET3(obj);
+
+    set_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void vmxnet3_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        vmxnet3_get_bootindex,
+                        vmxnet3_set_bootindex, NULL, NULL, NULL);
+}
+
 static bool vmxnet3_mc_list_needed(void *opaque)
 {
     return true;
@@ -2524,6 +2547,7 @@ static const TypeInfo vmxnet3_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(VMXNET3State),
     .class_init    = vmxnet3_class_init,
+    .instance_init = vmxnet3_instance_init,
 };
 
 static void vmxnet3_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 16/27] usb-net: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/dev-network.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index ea038ab..7a34368 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1378,6 +1378,29 @@ static int usb_net_initfn(USBDevice *dev)
     return 0;
 }
 
+static void usb_net_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    USBNetState *s = DO_UPCAST(USBNetState, dev, USB_DEVICE(obj));
+
+    get_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void usb_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    USBNetState *s = DO_UPCAST(USBNetState, dev, USB_DEVICE(obj));
+
+    set_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void usb_net_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        usb_net_get_bootindex,
+                        usb_net_set_bootindex, NULL, NULL, NULL);
+}
+
 static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
 {
     Error *local_err = NULL;
@@ -1441,6 +1464,7 @@ static const TypeInfo net_info = {
     .parent        = TYPE_USB_DEVICE,
     .instance_size = sizeof(USBNetState),
     .class_init    = usb_net_class_initfn,
+    .instance_init = usb_net_instance_init,
 };
 
 static void usb_net_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 17/27] net: remove bootindex property from qdev to qom

[arei.gonglei]

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

Remove bootindex form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Meanwhile set the initial value of bootindex to -1.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/net/e1000.c         | 1 +
 hw/net/eepro100.c      | 1 +
 hw/net/lance.c         | 1 +
 hw/net/ne2000.c        | 1 +
 hw/net/pcnet-pci.c     | 1 +
 hw/net/rtl8139.c       | 1 +
 hw/net/spapr_llan.c    | 1 +
 hw/net/virtio-net.c    | 1 +
 hw/net/vmxnet3.c       | 1 +
 hw/usb/dev-network.c   | 1 +
 hw/virtio/virtio-pci.c | 1 +
 include/net/net.h      | 4 +---
 12 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index ed87b74..d1d66c5 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1605,6 +1605,7 @@ static void e1000_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         e1000_get_bootindex,
                         e1000_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property e1000_properties[] = {
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 5a29f20..503f04f 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1932,6 +1932,7 @@ static void eepro100_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         eepro100_get_bootindex,
                         eepro100_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static E100PCIDeviceInfo e100_devices[] = {
diff --git a/hw/net/lance.c b/hw/net/lance.c
index 3071b3f..4ca0340 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -168,6 +168,7 @@ static void lance_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         lance_get_bootindex,
                         lance_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property lance_properties[] = {
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 3ddb494..ecdb663 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -781,6 +781,7 @@ static void ne2000_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         ne2000_get_bootindex,
                         ne2000_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property ne2000_properties[] = {
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index ca744c8..fb00f4f 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -371,6 +371,7 @@ static void pcnet_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         pcnet_get_bootindex,
                         pcnet_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property pcnet_properties[] = {
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index eba60d1..59e8729 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3564,6 +3564,7 @@ static void rtl8139_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         rtl8139_get_bootindex,
                         rtl8139_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property rtl8139_properties[] = {
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 773404a..856f320 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -240,6 +240,7 @@ static void spapr_vlan_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         spapr_vlan_get_bootindex,
                         spapr_vlan_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 8b409ad..7dce688 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1714,6 +1714,7 @@ static void virtio_net_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         virtio_net_get_bootindex,
                         virtio_net_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property virtio_net_properties[] = {
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index a9ed593..e6f10bb 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2224,6 +2224,7 @@ static void vmxnet3_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         vmxnet3_get_bootindex,
                         vmxnet3_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static bool vmxnet3_mc_list_needed(void *opaque)
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 7a34368..a85866b 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1399,6 +1399,7 @@ static void usb_net_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         usb_net_get_bootindex,
                         usb_net_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ca8bdfd..704c83b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1478,6 +1478,7 @@ static void virtio_net_pci_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         virtio_net_pci_get_bootindex,
                         virtio_net_pci_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static const TypeInfo virtio_net_pci_info = {
diff --git a/include/net/net.h b/include/net/net.h
index ed594f9..e482550 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -36,9 +36,7 @@ typedef struct NICConf {
 #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
     DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr),                \
     DEFINE_PROP_VLAN("vlan",     _state, _conf.peers),                   \
-    DEFINE_PROP_NETDEV("netdev", _state, _conf.peers),                   \
-    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)
-
+    DEFINE_PROP_NETDEV("netdev", _state, _conf.peers)
 
 /* Net clients */
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 18/27] host-libusb: remove bootindex property from qdev to qom

[arei.gonglei]

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

Remove bootindex form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/host-libusb.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index f37708d..c3c91f3 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -979,6 +979,30 @@ static int usb_host_initfn(USBDevice *udev)
     return 0;
 }
 
+static void usb_host_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    USBHostDevice *s = USB_HOST_DEVICE(obj);
+
+    get_bootindex(&s->bootindex, v, name, errp);
+}
+
+static void usb_host_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    USBHostDevice *s = USB_HOST_DEVICE(obj);
+
+    set_bootindex(&s->bootindex, v, name, errp);
+}
+
+static void usb_host_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        usb_host_get_bootindex,
+                        usb_host_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
+}
+
 static void usb_host_handle_destroy(USBDevice *udev)
 {
     USBHostDevice *s = USB_HOST_DEVICE(udev);
@@ -1465,7 +1489,6 @@ static Property usb_host_dev_properties[] = {
     DEFINE_PROP_UINT32("productid", USBHostDevice, match.product_id, 0),
     DEFINE_PROP_UINT32("isobufs",  USBHostDevice, iso_urb_count,    4),
     DEFINE_PROP_UINT32("isobsize", USBHostDevice, iso_urb_frames,   32),
-    DEFINE_PROP_INT32("bootindex", USBHostDevice, bootindex,        -1),
     DEFINE_PROP_UINT32("loglevel",  USBHostDevice, loglevel,
                        LIBUSB_LOG_LEVEL_WARNING),
     DEFINE_PROP_BIT("pipeline",    USBHostDevice, options,
@@ -1498,6 +1521,7 @@ static TypeInfo usb_host_dev_info = {
     .parent        = TYPE_USB_DEVICE,
     .instance_size = sizeof(USBHostDevice),
     .class_init    = usb_host_class_initfn,
+    .instance_init = usb_host_instance_init,
 };
 
 static void usb_host_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 19/27] pci-assign: remove bootindex property from qdev to qom

[arei.gonglei]

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

Remove bootindex form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/i386/kvm/pci-assign.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 0e6aa8a..a91c23d 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1850,13 +1850,35 @@ static void assigned_exitfn(struct PCIDevice *pci_dev)
     free_assigned_device(dev);
 }
 
+static void assigned_dev_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    AssignedDevice *d = DO_UPCAST(AssignedDevice, dev, PCI_DEVICE(obj));
+
+    get_bootindex(&d->bootindex, v, name, errp);
+}
+
+static void assigned_dev_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    AssignedDevice *d = DO_UPCAST(AssignedDevice, dev, PCI_DEVICE(obj));
+    set_bootindex(&d->bootindex, v, name, errp);
+}
+
+static void assigned_dev_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        assigned_dev_get_bootindex,
+                        assigned_dev_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
+}
+
 static Property assigned_dev_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("host", AssignedDevice, host),
     DEFINE_PROP_BIT("prefer_msi", AssignedDevice, features,
                     ASSIGNED_DEVICE_PREFER_MSI_BIT, false),
     DEFINE_PROP_BIT("share_intx", AssignedDevice, features,
                     ASSIGNED_DEVICE_SHARE_INTX_BIT, true),
-    DEFINE_PROP_INT32("bootindex", AssignedDevice, bootindex, -1),
     DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_name),
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -1882,6 +1904,7 @@ static const TypeInfo assign_info = {
     .parent             = TYPE_PCI_DEVICE,
     .instance_size      = sizeof(AssignedDevice),
     .class_init         = assign_class_init,
+    .instance_init      = assigned_dev_instance_init,
 };
 
 static void assign_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 20/27] vfio: remove bootindex property from qdev to qom

[arei.gonglei]

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

Remove bootindex form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/misc/vfio.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 625598a..465d934 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4366,13 +4366,36 @@ post_reset:
     vfio_pci_post_reset(vdev);
 }
 
+static void vfio_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                               const char *name, Error **errp)
+{
+    VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, PCI_DEVICE(obj));
+
+    get_bootindex(&vdev->bootindex, v, name, errp);
+}
+
+static void vfio_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                               const char *name, Error **errp)
+{
+    VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, PCI_DEVICE(obj));
+
+    get_bootindex(&vdev->bootindex, v, name, errp);
+}
+
+static void vfio_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        vfio_get_bootindex,
+                        vfio_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
+}
+
 static Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIODevice, host),
     DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIODevice,
                        intx.mmap_timeout, 1100),
     DEFINE_PROP_BIT("x-vga", VFIODevice, features,
                     VFIO_FEATURE_ENABLE_VGA_BIT, false),
-    DEFINE_PROP_INT32("bootindex", VFIODevice, bootindex, -1),
     /*
      * TODO - support passed fds... is this necessary?
      * DEFINE_PROP_STRING("vfiofd", VFIODevice, vfiofd_name),
@@ -4408,6 +4431,7 @@ static const TypeInfo vfio_pci_dev_info = {
     .parent = TYPE_PCI_DEVICE,
     .instance_size = sizeof(VFIODevice),
     .class_init = vfio_pci_dev_class_init,
+    .instance_init = vfio_instance_init,
 };
 
 static void register_vfio_pci_dev_type(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 21/27] redirect: remove bootindex property from qdev to qom

[arei.gonglei]

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

Remove bootindex form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/redirect.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 92dd353..ad97834 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2469,7 +2469,6 @@ static Property usbredir_properties[] = {
     DEFINE_PROP_CHR("chardev", USBRedirDevice, cs),
     DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, usbredirparser_warning),
     DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str),
-    DEFINE_PROP_INT32("bootindex", USBRedirDevice, bootindex, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2494,11 +2493,38 @@ static void usbredir_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 }
 
+static void usbredir_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    USBDevice *udev = USB_DEVICE(obj);
+    USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
+
+    get_bootindex(&dev->bootindex, v, name, errp);
+}
+
+static void usbredir_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    USBDevice *udev = USB_DEVICE(obj);
+    USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
+
+    set_bootindex(&dev->bootindex, v, name, errp);
+}
+
+static void usbredir_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        usbredir_get_bootindex,
+                        usbredir_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
+}
+
 static const TypeInfo usbredir_dev_info = {
     .name          = "usb-redir",
     .parent        = TYPE_USB_DEVICE,
     .instance_size = sizeof(USBRedirDevice),
     .class_init    = usbredir_class_initfn,
+    .instance_init = usbredir_instance_init,
 };
 
 static void usbredir_register_types(void)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 22/27] isa-fdc: remove bootindexA/B property from qdev to qom

[arei.gonglei]

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

Remove bootindexA/B form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/block/fdc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 8add4a1..169a8f0 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2217,8 +2217,6 @@ static Property isa_fdc_properties[] = {
     DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
     DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
     DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
-    DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
-    DEFINE_PROP_INT32("bootindexB", FDCtrlISABus, bootindexB, -1),
     DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
                     0, true),
     DEFINE_PROP_END_OF_LIST(),
@@ -2236,11 +2234,56 @@ static void isabus_fdc_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
+static void isabus_fdc_get_bootindexA(Object *obj, Visitor *v, void *opaque,
+                                      const char *name, Error **errp)
+{
+    FDCtrlISABus *isa = ISA_FDC(obj);
+
+    get_bootindex(&isa->bootindexA, v, name, errp);
+}
+
+static void isabus_fdc_set_bootindexA(Object *obj, Visitor *v, void *opaque,
+                                      const char *name, Error **errp)
+{
+    FDCtrlISABus *isa = ISA_FDC(obj);
+
+    set_bootindex(&isa->bootindexA, v, name, errp);
+}
+
+static void isabus_fdc_get_bootindexB(Object *obj, Visitor *v, void *opaque,
+                                      const char *name, Error **errp)
+{
+    FDCtrlISABus *isa = ISA_FDC(obj);
+
+    get_bootindex(&isa->bootindexB, v, name, errp);
+}
+
+static void isabus_fdc_set_bootindexB(Object *obj, Visitor *v, void *opaque,
+                                      const char *name, Error **errp)
+{
+    FDCtrlISABus *isa = ISA_FDC(obj);
+
+    set_bootindex(&isa->bootindexB, v, name, errp);
+}
+
+static void isabus_fdc_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindexA", "int",
+                        isabus_fdc_get_bootindexA,
+                        isabus_fdc_set_bootindexA, NULL, NULL, NULL);
+    object_property_add(obj, "bootindexB", "int",
+                        isabus_fdc_get_bootindexB,
+                        isabus_fdc_set_bootindexB, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindexA", NULL);
+    object_property_set_int(obj, -1, "bootindexB", NULL);
+}
+
 static const TypeInfo isa_fdc_info = {
     .name          = TYPE_ISA_FDC,
     .parent        = TYPE_ISA_DEVICE,
     .instance_size = sizeof(FDCtrlISABus),
     .class_init    = isabus_fdc_class_init,
+    .instance_init = isabus_fdc_instance_init,
 };
 
 static const VMStateDescription vmstate_sysbus_fdc ={
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 23/27] ide: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/ide/qdev.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 83bdb97..67ee098 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -196,6 +196,31 @@ static void ide_dev_reset(DeviceState *dev)
                          d->unit ? "/disk@1" : "/disk@0");
 }
 
+static void ide_dev_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    IDEDevice *d = DO_UPCAST(IDEDevice, qdev, dev);
+
+    get_bootindex(&d->conf.bootindex, v, name, errp);
+}
+
+static void ide_dev_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    IDEDevice *d = DO_UPCAST(IDEDevice, qdev, dev);
+
+    set_bootindex(&d->conf.bootindex, v, name, errp);
+}
+
+static void ide_dev_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        ide_dev_get_bootindex,
+                        ide_dev_set_bootindex, NULL, NULL, NULL);
+}
+
 static int ide_hd_initfn(IDEDevice *dev)
 {
     return ide_dev_initfn(dev, IDE_HD);
@@ -244,6 +269,7 @@ static const TypeInfo ide_hd_info = {
     .parent        = TYPE_IDE_DEVICE,
     .instance_size = sizeof(IDEDrive),
     .class_init    = ide_hd_class_init,
+    .instance_init = ide_dev_instance_init,
 };
 
 static Property ide_cd_properties[] = {
@@ -267,6 +293,7 @@ static const TypeInfo ide_cd_info = {
     .parent        = TYPE_IDE_DEVICE,
     .instance_size = sizeof(IDEDrive),
     .class_init    = ide_cd_class_init,
+    .instance_init = ide_dev_instance_init,
 };
 
 static Property ide_drive_properties[] = {
@@ -290,6 +317,7 @@ static const TypeInfo ide_drive_info = {
     .parent        = TYPE_IDE_DEVICE,
     .instance_size = sizeof(IDEDrive),
     .class_init    = ide_drive_class_init,
+    .instance_init = ide_dev_instance_init,
 };
 
 static void ide_device_class_init(ObjectClass *klass, void *data)
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 24/27] scsi: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/scsi/scsi-disk.c    | 29 +++++++++++++++++++++++++++++
 hw/scsi/scsi-generic.c |  1 +
 include/hw/scsi/scsi.h |  1 +
 3 files changed, 31 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 03d1384..bc2deb9 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2332,6 +2332,31 @@ static void scsi_disk_realize(SCSIDevice *dev, Error **errp)
     }
 }
 
+static void scsi_dev_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    SCSIDevice *s = DO_UPCAST(SCSIDevice, qdev, dev);
+
+    get_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+static void scsi_dev_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    SCSIDevice *s = DO_UPCAST(SCSIDevice, qdev, dev);
+
+    set_bootindex(&s->conf.bootindex, v, name, errp);
+}
+
+void scsi_dev_instance_init(Object *obj)
+{
+    object_property_add(obj, "bootindex", "int",
+                        scsi_dev_get_bootindex,
+                        scsi_dev_set_bootindex, NULL, NULL, NULL);
+}
+
 static const SCSIReqOps scsi_disk_emulate_reqops = {
     .size         = sizeof(SCSIDiskReq),
     .free_req     = scsi_free_request,
@@ -2645,6 +2670,7 @@ static const TypeInfo scsi_hd_info = {
     .parent        = TYPE_SCSI_DEVICE,
     .instance_size = sizeof(SCSIDiskState),
     .class_init    = scsi_hd_class_initfn,
+    .instance_init = scsi_dev_instance_init,
 };
 
 static Property scsi_cd_properties[] = {
@@ -2676,6 +2702,7 @@ static const TypeInfo scsi_cd_info = {
     .parent        = TYPE_SCSI_DEVICE,
     .instance_size = sizeof(SCSIDiskState),
     .class_init    = scsi_cd_class_initfn,
+    .instance_init = scsi_dev_instance_init,
 };
 
 #ifdef __linux__
@@ -2706,6 +2733,7 @@ static const TypeInfo scsi_block_info = {
     .parent        = TYPE_SCSI_DEVICE,
     .instance_size = sizeof(SCSIDiskState),
     .class_init    = scsi_block_class_initfn,
+    .instance_init = scsi_dev_instance_init,
 };
 #endif
 
@@ -2744,6 +2772,7 @@ static const TypeInfo scsi_disk_info = {
     .parent        = TYPE_SCSI_DEVICE,
     .instance_size = sizeof(SCSIDiskState),
     .class_init    = scsi_disk_class_initfn,
+    .instance_init = scsi_dev_instance_init,
 };
 
 static void scsi_disk_register_types(void)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index db6a80d..34f192b 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -514,6 +514,7 @@ static const TypeInfo scsi_generic_info = {
     .parent        = TYPE_SCSI_DEVICE,
     .instance_size = sizeof(SCSIDevice),
     .class_init    = scsi_generic_class_initfn,
+    .instance_init = scsi_dev_instance_init,
 };
 
 static void scsi_generic_register_types(void)
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 2e3a8f9..99c1c57 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -270,4 +270,5 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun);
 /* scsi-generic.c. */
 extern const SCSIReqOps scsi_generic_req_ops;
 
+void scsi_dev_instance_init(Object *obj);
 #endif
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 25/27] virtio-blk: add bootindex to qom property

[arei.gonglei]

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

Add a qom property with the same name 'bootindex',
when we remove it form qdev property, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/block/virtio-blk.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e74a4c5..ccc1e1f 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -797,6 +797,22 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
+static void virtio_blk_get_bootindex(Object *obj, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    VirtIOBlock *s = VIRTIO_BLK(obj);
+
+    get_bootindex(&s->conf->bootindex, v, name, errp);
+}
+
+static void virtio_blk_set_bootindex(Object *obj, Visitor *v, void *opaque,
+                                     const char *name, Error **errp)
+{
+    VirtIOBlock *s = VIRTIO_BLK(obj);
+    
+    set_bootindex(&s->conf->bootindex, v, name, errp);
+}
+
 static void virtio_blk_instance_init(Object *obj)
 {
     VirtIOBlock *s = VIRTIO_BLK(obj);
@@ -805,6 +821,9 @@ static void virtio_blk_instance_init(Object *obj)
                              (Object **)&s->blk.iothread,
                              qdev_prop_allow_set_link_before_realize,
                              OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
+    object_property_add(obj, "bootindex", "int",
+                        virtio_blk_get_bootindex, 
+                        virtio_blk_set_bootindex, NULL, NULL, NULL);
 }
 
 static Property virtio_blk_properties[] = {
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 26/27] block: remove bootindex property from qdev to qom

[arei.gonglei]

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

Remove bootindex form qdev property to qom, things will
continue to work just fine, and we can use qom features
which are not supported by qdev property.

Meanwhile set the initial value of bootindex to -1.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/block/virtio-blk.c    | 1 +
 hw/ide/qdev.c            | 1 +
 hw/scsi/scsi-disk.c      | 2 +-
 hw/scsi/scsi-generic.c   | 1 -
 include/hw/block/block.h | 1 -
 5 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ccc1e1f..4fdae36 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -824,6 +824,7 @@ static void virtio_blk_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         virtio_blk_get_bootindex, 
                         virtio_blk_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static Property virtio_blk_properties[] = {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 67ee098..af63b0a 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -219,6 +219,7 @@ static void ide_dev_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         ide_dev_get_bootindex,
                         ide_dev_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static int ide_hd_initfn(IDEDevice *dev)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index bc2deb9..cd236cb 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2355,6 +2355,7 @@ void scsi_dev_instance_init(Object *obj)
     object_property_add(obj, "bootindex", "int",
                         scsi_dev_get_bootindex,
                         scsi_dev_set_bootindex, NULL, NULL, NULL);
+    object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
 static const SCSIReqOps scsi_disk_emulate_reqops = {
@@ -2708,7 +2709,6 @@ static const TypeInfo scsi_cd_info = {
 #ifdef __linux__
 static Property scsi_block_properties[] = {
     DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.bs),
-    DEFINE_PROP_INT32("bootindex", SCSIDiskState, qdev.conf.bootindex, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 34f192b..4e3164f 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -483,7 +483,6 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
 
 static Property scsi_generic_properties[] = {
     DEFINE_PROP_DRIVE("drive", SCSIDevice, conf.bs),
-    DEFINE_PROP_INT32("bootindex", SCSIDevice, conf.bootindex, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 3a01488..867a226 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -49,7 +49,6 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
                           _conf.physical_block_size, 512),              \
     DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
     DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
-    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1),        \
     DEFINE_PROP_UINT32("discard_granularity", _state, \
                        _conf.discard_granularity, -1)
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 27/27] 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: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Chenliang <chenliang88@huawei.com>
---
 hw/core/qdev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index da1ba48..70294ad 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -972,6 +972,10 @@ static void device_finalize(Object *obj)
     NamedGPIOList *ngl, *next;
 
     DeviceState *dev = DEVICE(obj);
+
+    /* remove device's bootindex from global boot order list */
+    del_boot_device_path(dev);
+
     if (dev->opts) {
         qemu_opts_del(dev->opts);
     }
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Michael S. Tsirkin]

On Sat, Aug 30, 2014 at 06:00:07PM +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> when we remove bootindex form qdev.property to qom.property,
> we can use those functions set/get bootindex property for all
> correlative devices.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  include/sysemu/sysemu.h |  4 ++++
>  vl.c                    | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 672984c..ca231e4 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict *qdict);
>  void usb_info(Monitor *mon, const QDict *qdict);
>  
>  void check_boot_index(int32_t bootindex, Error **errp);
> +void get_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp);
> +void set_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp);
>  void del_boot_device_path(DeviceState *dev);
>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>                            const char *suffix);
> diff --git a/vl.c b/vl.c
> index f2c3b2d..4363185 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error **errp)
>      }
>  }
>  
> +void get_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp)
> +{
> +    visit_type_int32(v, bootindex, name, errp);
> +}
> +
> +void set_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp)
> +{
> +    int32_t boot_index;
> +    Error *local_err = NULL;
> +
> +    visit_type_int32(v, &boot_index, name, &local_err);
> +
> +    if (local_err == NULL) {
> +        /* check the bootindex existes or not in fw_boot_order list  */

should be:
check whether bootindex is present in fw_boot_order list

> +        check_boot_index(boot_index, &local_err);
> +    }
> +
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    /* change bootindex to a new one */
> +    *bootindex = boot_index;
> +}
> +
>  static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
>  {
>      bool ret = false;
> -- 
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Sunday, August 31, 2014 5:58 PM
> Subject: Re: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> property
> 
> On Sat, Aug 30, 2014 at 06:00:07PM +0800, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > when we remove bootindex form qdev.property to qom.property,
> > we can use those functions set/get bootindex property for all
> > correlative devices.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  include/sysemu/sysemu.h |  4 ++++
> >  vl.c                    | 27 +++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 672984c..ca231e4 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict
> *qdict);
> >  void usb_info(Monitor *mon, const QDict *qdict);
> >
> >  void check_boot_index(int32_t bootindex, Error **errp);
> > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp);
> > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp);
> >  void del_boot_device_path(DeviceState *dev);
> >  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> >                            const char *suffix);
> > diff --git a/vl.c b/vl.c
> > index f2c3b2d..4363185 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error
> **errp)
> >      }
> >  }
> >
> > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp)
> > +{
> > +    visit_type_int32(v, bootindex, name, errp);
> > +}
> > +
> > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp)
> > +{
> > +    int32_t boot_index;
> > +    Error *local_err = NULL;
> > +
> > +    visit_type_int32(v, &boot_index, name, &local_err);
> > +
> > +    if (local_err == NULL) {
> > +        /* check the bootindex existes or not in fw_boot_order list  */
> 
> should be:
> check whether bootindex is present in fw_boot_order list
> 
OK, will fix this. Thanks!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gerd Hoffmann]

  Hi,

> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
> +{
> +    bool ret = false;
> +    char *devpath_src = qdev_get_fw_dev_path(src);
> +    char *devpath_dst = qdev_get_fw_dev_path(dst);
> +
> +    if (!strcmp(devpath_src, devpath_dst)) {
> +        ret = true;
> +    }
> +
> +    g_free(devpath_src);
> +    g_free(devpath_dst);
> +    return ret;
> +}
> +
> +void del_boot_device_path(DeviceState *dev)
> +{
> +    FWBootEntry *i;
> +
> +    assert(dev != NULL);
> +
> +    /* remove all entries of the assigned device */
> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +        if (i->dev == NULL) {
> +            continue;
> +        }
> +        if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) {

Why this is needed?  Is there any case where i-->dev != dev but
is_same_fw_dev_path() returns true?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Monday, September 01, 2014 2:43 PM
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> Importance: High
> 
>   Hi,
> 
> > +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
> > +{
> > +    bool ret = false;
> > +    char *devpath_src = qdev_get_fw_dev_path(src);
> > +    char *devpath_dst = qdev_get_fw_dev_path(dst);
> > +
> > +    if (!strcmp(devpath_src, devpath_dst)) {
> > +        ret = true;
> > +    }
> > +
> > +    g_free(devpath_src);
> > +    g_free(devpath_dst);
> > +    return ret;
> > +}
> > +
> > +void del_boot_device_path(DeviceState *dev)
> > +{
> > +    FWBootEntry *i;
> > +
> > +    assert(dev != NULL);
> > +
> > +    /* remove all entries of the assigned device */
> > +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > +        if (i->dev == NULL) {
> > +            continue;
> > +        }
> > +        if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) {
> 
> Why this is needed?  Is there any case where i-->dev != dev but
> is_same_fw_dev_path() returns true?
> 
Yes, it is needed. At present, the virito-net-pci device 
compliance with this situation.

Please see the qom path about virtio-net-pci and virtio-net device:

id: null, /machine/peripheral/nic1/virtio-backend
id: nic1, /machine/peripheral/nic1

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Mon, Sep 01, 2014 at 06:47:13AM +0000, Gonglei (Arei) wrote:
> > From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> > Sent: Monday, September 01, 2014 2:43 PM
> > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> > Importance: High
> > 
> >   Hi,
> > 
> > > +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
> > > +{
> > > +    bool ret = false;
> > > +    char *devpath_src = qdev_get_fw_dev_path(src);
> > > +    char *devpath_dst = qdev_get_fw_dev_path(dst);
> > > +
> > > +    if (!strcmp(devpath_src, devpath_dst)) {
> > > +        ret = true;
> > > +    }
> > > +
> > > +    g_free(devpath_src);
> > > +    g_free(devpath_dst);
> > > +    return ret;
> > > +}
> > > +
> > > +void del_boot_device_path(DeviceState *dev)
> > > +{
> > > +    FWBootEntry *i;
> > > +
> > > +    assert(dev != NULL);
> > > +
> > > +    /* remove all entries of the assigned device */
> > > +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > > +        if (i->dev == NULL) {
> > > +            continue;
> > > +        }
> > > +        if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) {
> > 
> > Why this is needed?  Is there any case where i-->dev != dev but
> > is_same_fw_dev_path() returns true?
> > 
> Yes, it is needed. At present, the virito-net-pci device 
> compliance with this situation.
> 
> Please see the qom path about virtio-net-pci and virtio-net device:
> 
> id: null, /machine/peripheral/nic1/virtio-backend
> id: nic1, /machine/peripheral/nic1

And why exactly is the caller passing a different pointer to
del_boot_device_path()? Why not simply change the caller to pass the
same pointer to add_boot_device_path() and del_boot_device_path()?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

Hi,

> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Wednesday, September 03, 2014 2:00 AM
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> Importance: High
> 
> On Mon, Sep 01, 2014 at 06:47:13AM +0000, Gonglei (Arei) wrote:
> > > From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> > > Sent: Monday, September 01, 2014 2:43 PM
> > > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path
> function
> > > Importance: High
> > >
> > >   Hi,
> > >
> > > > +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
> > > > +{
> > > > +    bool ret = false;
> > > > +    char *devpath_src = qdev_get_fw_dev_path(src);
> > > > +    char *devpath_dst = qdev_get_fw_dev_path(dst);
> > > > +
> > > > +    if (!strcmp(devpath_src, devpath_dst)) {
> > > > +        ret = true;
> > > > +    }
> > > > +
> > > > +    g_free(devpath_src);
> > > > +    g_free(devpath_dst);
> > > > +    return ret;
> > > > +}
> > > > +
> > > > +void del_boot_device_path(DeviceState *dev)
> > > > +{
> > > > +    FWBootEntry *i;
> > > > +
> > > > +    assert(dev != NULL);
> > > > +
> > > > +    /* remove all entries of the assigned device */
> > > > +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > > > +        if (i->dev == NULL) {
> > > > +            continue;
> > > > +        }
> > > > +        if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) {
> > >
> > > Why this is needed?  Is there any case where i-->dev != dev but
> > > is_same_fw_dev_path() returns true?
> > >
> > Yes, it is needed. At present, the virito-net-pci device
> > compliance with this situation.
> >
> > Please see the qom path about virtio-net-pci and virtio-net device:
> >
> > id: null, /machine/peripheral/nic1/virtio-backend
> > id: nic1, /machine/peripheral/nic1
> 
> And why exactly is the caller passing a different pointer to
> del_boot_device_path()? Why not simply change the caller to pass the
> same pointer to add_boot_device_path() and del_boot_device_path()?
> 
1. When we want to create a virtio-net device, we must use '-device virtio-net-pci'.
This device is a pci device, and virtio-net-device is its child:
 object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
2. Both virtio-net-pci and virtio-net-device own bootindex property because they
include "DEFINE_NIC_PROPERTIES(VirtIONetPCI, vdev.nic_conf)".
3. Only virtio-net-device will call add_boot_device_path() in virtio_net_device_realize(),
which use its own pointer, but not virtio-net-pci's pointer:
 add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer to 
del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a function
named is_same_fw_dev_path() to handle this situation. 


Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

> Subject: RE: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> 
> Hi,
> 
> > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > Sent: Wednesday, September 03, 2014 2:00 AM
> > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> > Importance: High
> >
> > On Mon, Sep 01, 2014 at 06:47:13AM +0000, Gonglei (Arei) wrote:
> > > > From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> > > > Sent: Monday, September 01, 2014 2:43 PM
> > > > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path
> > function
> > > > Importance: High
> > > >
> > > >   Hi,
> > > >
> > > > > +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
> > > > > +{
> > > > > +    bool ret = false;
> > > > > +    char *devpath_src = qdev_get_fw_dev_path(src);
> > > > > +    char *devpath_dst = qdev_get_fw_dev_path(dst);
> > > > > +
> > > > > +    if (!strcmp(devpath_src, devpath_dst)) {
> > > > > +        ret = true;
> > > > > +    }
> > > > > +
> > > > > +    g_free(devpath_src);
> > > > > +    g_free(devpath_dst);
> > > > > +    return ret;
> > > > > +}
> > > > > +
> > > > > +void del_boot_device_path(DeviceState *dev)
> > > > > +{
> > > > > +    FWBootEntry *i;
> > > > > +
> > > > > +    assert(dev != NULL);
> > > > > +
> > > > > +    /* remove all entries of the assigned device */
> > > > > +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > > > > +        if (i->dev == NULL) {
> > > > > +            continue;
> > > > > +        }
> > > > > +        if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) {
> > > >
> > > > Why this is needed?  Is there any case where i-->dev != dev but
> > > > is_same_fw_dev_path() returns true?
> > > >
> > > Yes, it is needed. At present, the virito-net-pci device
> > > compliance with this situation.
> > >
> > > Please see the qom path about virtio-net-pci and virtio-net device:
> > >
> > > id: null, /machine/peripheral/nic1/virtio-backend
> > > id: nic1, /machine/peripheral/nic1
> >
> > And why exactly is the caller passing a different pointer to
> > del_boot_device_path()? Why not simply change the caller to pass the
> > same pointer to add_boot_device_path() and del_boot_device_path()?
> >
> 1. When we want to create a virtio-net device, we must use '-device
> virtio-net-pci'.
> This device is a pci device, and virtio-net-device is its child:
>  object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev),
> NULL);
> 2. Both virtio-net-pci and virtio-net-device own bootindex property because
> they
> include "DEFINE_NIC_PROPERTIES(VirtIONetPCI, vdev.nic_conf)".
> 3. Only virtio-net-device will call add_boot_device_path() in
> virtio_net_device_realize(),
> which use its own pointer, but not virtio-net-pci's pointer:
>  add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
> 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer

Sorry, my typo, s/hotplug/hot-unplug/

> to del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a
> function named is_same_fw_dev_path() to handle this situation.
> 
> 
> Best regards,
> -Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gerd Hoffmann]

  Hi,

> 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer to 
> del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a function
> named is_same_fw_dev_path() to handle this situation. 

When hot-unplugging virtio-net-pci I'd expect we free both
virtio-net-pci and virtio-net-device (and therefore call
del_boot_device_path twice, once for each device).  Can you check that?

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

Best regards,
-Gonglei


> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Wednesday, September 03, 2014 2:25 PM
> To: Gonglei (Arei)
> Cc: Eduardo Habkost; qemu-devel@nongnu.org; aliguori@amazon.com;
> mst@redhat.com; pbonzini@redhat.com; akong@redhat.com;
> hutao@cn.fujitsu.com; eblake@redhat.com; afaerber@suse.de;
> armbru@redhat.com; imammedo@redhat.com; aik@ozlabs.ru;
> peter.crosthwaite@xilinx.com; lcapitulino@redhat.com; hani@linux.com;
> stefanha@redhat.com; agraf@suse.de; chenliang (T); Huangweidong (C);
> Luonengjun; Huangpeng (Peter); kwolf@redhat.com
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> Importance: High
> 
> 
>   Hi,
> 
> > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer
> to
> > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a
> function
> > named is_same_fw_dev_path() to handle this situation.
> 
> When hot-unplugging virtio-net-pci I'd expect we free both
> virtio-net-pci and virtio-net-device (and therefore call
> del_boot_device_path twice, once for each device).  Can you check that?
> 
Yes, I can. 

The del_boot_device_path() is called only once, just for virtio-net-pci.
For its child, virtio-net-devcie's resource is cleaned by qbus->child unrealizing 
process, will not call device_finalize().

Command line:

# ./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive file=/mnt/sdb/gonglei/image/win7_32_2U,if=none,id=drive-ide0-0-0 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -drive file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-0-1 -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 -vnc 0.0.0.0:10 -netdev type=user,id=net0 -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1
QEMU 2.1.50 monitor - type 'help' for more information
(qemu) device_del nic1
(qemu)

The below is the backtrace by gdb:

Breakpoint 1, virtio_net_device_unrealize (dev=0x7f1b1ea87d58, errp=0x7f1b17d8c350)
    at /mnt/sdb/gonglei/qemu.git/qemu/hw/net/virtio-net.c:1649
1649    {
(gdb) bt
#0  virtio_net_device_unrealize (dev=0x7f1b1ea87d58, errp=0x7f1b17d8c350)
    at /mnt/sdb/gonglei/qemu.git/qemu/hw/net/virtio-net.c:1649
#1  0x00007f1b1dd69b71 in virtio_device_unrealize (dev=0x7f1b1ea87d58, errp=0x7f1b17d8c3c8)
    at /mnt/sdb/gonglei/qemu.git/qemu/hw/virtio/virtio.c:1312
#2  0x00007f1b1de8a1a4 in device_set_realized (obj=0x7f1b1ea87d58, value=false, errp=0x7f1b17d8c568) at hw/core/qdev.c:885
#3  0x00007f1b1dfa01f7 in property_set_bool (obj=0x7f1b1ea87d58, v=0x7f1b1eb29de0, opaque=0x7f1b1eaa72c0, name=
    0x7f1b1e0c2b69 "realized", errp=0x7f1b17d8c568) at qom/object.c:1467
#4  0x00007f1b1df9e82f in object_property_set (obj=0x7f1b1ea87d58, v=0x7f1b1eb29de0, name=0x7f1b1e0c2b69 "realized", errp=
    0x7f1b17d8c568) at qom/object.c:814
#5  0x00007f1b1dfa0c45 in object_property_set_qobject (obj=0x7f1b1ea87d58, value=0x7f1b1eb09c80, name=0x7f1b1e0c2b69 "realized", 
    errp=0x7f1b17d8c568) at qom/qom-qobject.c:24
#6  0x00007f1b1df9ebd7 in object_property_set_bool (obj=0x7f1b1ea87d58, value=false, name=0x7f1b1e0c2b69 "realized", errp=
    0x7f1b17d8c568) at qom/object.c:878
#7  0x00007f1b1de893fa in bus_set_realized (obj=0x7f1b1ea87ce0, value=false, errp=0x7f1b17d8c718) at hw/core/qdev.c:583
#8  0x00007f1b1dfa01f7 in property_set_bool (obj=0x7f1b1ea87ce0, v=0x7f1b1ead5a10, opaque=0x7f1b1eaa1640, name=
    0x7f1b1e0c2b69 "realized", errp=0x7f1b17d8c718) at qom/object.c:1467
#9  0x00007f1b1df9e82f in object_property_set (obj=0x7f1b1ea87ce0, v=0x7f1b1ead5a10, name=0x7f1b1e0c2b69 "realized", errp=
    0x7f1b17d8c718) at qom/object.c:814
#10 0x00007f1b1dfa0c45 in object_property_set_qobject (obj=0x7f1b1ea87ce0, value=0x7f1b1eb09c60, name=0x7f1b1e0c2b69 "realized", 
    errp=0x7f1b17d8c718) at qom/qom-qobject.c:24
#11 0x00007f1b1df9ebd7 in object_property_set_bool (obj=0x7f1b1ea87ce0, value=false, name=0x7f1b1e0c2b69 "realized", errp=
    0x7f1b17d8c718) at qom/object.c:878
#12 0x00007f1b1de8a127 in device_set_realized (obj=0x7f1b1ea87380, value=false, errp=0x0) at hw/core/qdev.c:875
#13 0x00007f1b1dfa01f7 in property_set_bool (obj=0x7f1b1ea87380, v=0x7f1b1eacf940, opaque=0x7f1b1ea89f20, name=
    0x7f1b1e0c2b69 "realized", errp=0x0) at qom/object.c:1467
#14 0x00007f1b1df9e82f in object_property_set (obj=0x7f1b1ea87380, v=0x7f1b1eacf940, name=0x7f1b1e0c2b69 "realized", errp=0x0)
    at qom/object.c:814
#15 0x00007f1b1dfa0c45 in object_property_set_qobject (obj=0x7f1b1ea87380, value=0x7f1b1eb085e0, name=0x7f1b1e0c2b69 "realized", 
    errp=0x0) at qom/qom-qobject.c:24
#16 0x00007f1b1df9ebd7 in object_property_set_bool (obj=0x7f1b1ea87380, value=false, name=0x7f1b1e0c2b69 "realized", errp=0x0)
    at qom/object.c:878
#17 0x00007f1b1de8a7d0 in device_unparent (obj=0x7f1b1ea87380) at hw/core/qdev.c:1010
#18 0x00007f1b1df9f26b in object_finalize_child_property (obj=0x7f1b1e9f4f30, name=0x7f1b1eaa7ba0 "nic1", opaque=0x7f1b1ea87380)
    at qom/object.c:1036
#19 0x00007f1b1df9e671 in object_property_del (obj=0x7f1b1e9f4f30, name=0x7f1b1eaa7ba0 "nic1", errp=0x0) at qom/object.c:778
#20 0x00007f1b1df9d692 in object_property_del_child (obj=0x7f1b1e9f4f30, child=0x7f1b1ea87380, errp=0x0) at qom/object.c:382
#21 0x00007f1b1df9d74b in object_unparent (obj=0x7f1b1ea87380) at qom/object.c:391
#22 0x00007f1b1de52317 in acpi_pcihp_eject_slot (s=0x7f1b1eaf3438, bsel=0, slots=8) at hw/acpi/pcihp.c:139
#23 0x00007f1b1de529b5 in pci_write (opaque=0x7f1b1eaf3438, addr=8, data=8, size=4) at hw/acpi/pcihp.c:277
#24 0x00007f1b1dd1e958 in memory_region_write_accessor (mr=0x7f1b1eaf4048, addr=8, value=0x7f1b17d8cb38, size=4, shift=0, mask=
    4294967295) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:443
#25 0x00007f1b1dd1ea94 in access_with_adjusted_size (addr=8, value=0x7f1b17d8cb38, size=4, access_size_min=1, access_size_max=4, 
    access=0x7f1b1dd1e8cb <memory_region_write_accessor>, mr=0x7f1b1eaf4048) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:480
#26 0x00007f1b1dd22061 in memory_region_dispatch_write (mr=0x7f1b1eaf4048, addr=8, data=8, size=4)
    at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1137
#27 0x00007f1b1dd25c10 in io_mem_write (mr=0x7f1b1eaf4048, addr=8, val=8, size=4) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1973
#28 0x00007f1b1dcd01a9 in address_space_rw (as=0x7f1b1e4ed180 <address_space_io>, addr=44552, buf=0x7f1b1dc1f000 "\b", len=4, 
    is_write=true) at /mnt/sdb/gonglei/qemu.git/qemu/exec.c:2042
#29 0x00007f1b1dd1b186 in kvm_handle_io (port=44552, data=0x7f1b1dc1f000, direction=1, size=4, count=1)
    at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1597
#30 0x00007f1b1dd1b7c4 in kvm_cpu_exec (cpu=0x7f1b1e9f85b0) at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1748
#31 0x00007f1b1dd02c88 in qemu_kvm_cpu_thread_fn (arg=0x7f1b1e9f85b0) at /mnt/sdb/gonglei/qemu.git/qemu/cpus.c:940
#32 0x00007f1b1af3b7f6 in start_thread () from /lib64/libpthread.so.0
#33 0x00007f1b1ac9709d in clone () from /lib64/libc.so.6
#34 0x0000000000000000 in ?? ()
(gdb) c
Continuing.

Breakpoint 2, device_finalize (obj=0x7f1b1ea87380) at hw/core/qdev.c:971
971     {
(gdb) bt
#0  device_finalize (obj=0x7f1b1ea87380) at hw/core/qdev.c:971
#1  0x00007f1b1df9d79b in object_deinit (obj=0x7f1b1ea87380, type=0x7f1b1e99a970) at qom/object.c:398
#2  0x00007f1b1df9d7bd in object_deinit (obj=0x7f1b1ea87380, type=0x7f1b1e990a40) at qom/object.c:402
#3  0x00007f1b1df9d7bd in object_deinit (obj=0x7f1b1ea87380, type=0x7f1b1e987700) at qom/object.c:402
#4  0x00007f1b1df9d7bd in object_deinit (obj=0x7f1b1ea87380, type=0x7f1b1e987fc0) at qom/object.c:402
#5  0x00007f1b1df9d81a in object_finalize (data=0x7f1b1ea87380) at qom/object.c:412
#6  0x00007f1b1df9e3d9 in object_unref (obj=0x7f1b1ea87380) at qom/object.c:719
#7  0x00007f1b1df9f280 in object_finalize_child_property (obj=0x7f1b1e9f4f30, name=0x7f1b1eaa7ba0 "nic1", opaque=0x7f1b1ea87380)
    at qom/object.c:1039
#8  0x00007f1b1df9e671 in object_property_del (obj=0x7f1b1e9f4f30, name=0x7f1b1eaa7ba0 "nic1", errp=0x0) at qom/object.c:778
#9  0x00007f1b1df9d692 in object_property_del_child (obj=0x7f1b1e9f4f30, child=0x7f1b1ea87380, errp=0x0) at qom/object.c:382
#10 0x00007f1b1df9d74b in object_unparent (obj=0x7f1b1ea87380) at qom/object.c:391
#11 0x00007f1b1de52317 in acpi_pcihp_eject_slot (s=0x7f1b1eaf3438, bsel=0, slots=8) at hw/acpi/pcihp.c:139
#12 0x00007f1b1de529b5 in pci_write (opaque=0x7f1b1eaf3438, addr=8, data=8, size=4) at hw/acpi/pcihp.c:277
#13 0x00007f1b1dd1e958 in memory_region_write_accessor (mr=0x7f1b1eaf4048, addr=8, value=0x7f1b17d8cb38, size=4, shift=0, mask=
    4294967295) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:443
#14 0x00007f1b1dd1ea94 in access_with_adjusted_size (addr=8, value=0x7f1b17d8cb38, size=4, access_size_min=1, access_size_max=4, 
    access=0x7f1b1dd1e8cb <memory_region_write_accessor>, mr=0x7f1b1eaf4048) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:480
#15 0x00007f1b1dd22061 in memory_region_dispatch_write (mr=0x7f1b1eaf4048, addr=8, data=8, size=4)
    at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1137
#16 0x00007f1b1dd25c10 in io_mem_write (mr=0x7f1b1eaf4048, addr=8, val=8, size=4) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1973
#17 0x00007f1b1dcd01a9 in address_space_rw (as=0x7f1b1e4ed180 <address_space_io>, addr=44552, buf=0x7f1b1dc1f000 "\b", len=4, 
    is_write=true) at /mnt/sdb/gonglei/qemu.git/qemu/exec.c:2042
#18 0x00007f1b1dd1b186 in kvm_handle_io (port=44552, data=0x7f1b1dc1f000, direction=1, size=4, count=1)
    at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1597
#19 0x00007f1b1dd1b7c4 in kvm_cpu_exec (cpu=0x7f1b1e9f85b0) at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1748
#20 0x00007f1b1dd02c88 in qemu_kvm_cpu_thread_fn (arg=0x7f1b1e9f85b0) at /mnt/sdb/gonglei/qemu.git/qemu/cpus.c:940
#21 0x00007f1b1af3b7f6 in start_thread () from /lib64/libpthread.so.0
#22 0x00007f1b1ac9709d in clone () from /lib64/libc.so.6
#23 0x0000000000000000 in ?? ()
(gdb) n
974         DeviceState *dev = DEVICE(obj);
(gdb) 
977         del_boot_device_path(dev);
(gdb) p *dev
$1 = {parent_obj = {class = 0x7f1b1e9b7270, free = 0x7f1b1caf1a30 <g_free>, properties = {tqh_first = 0x0, tqh_last = 
    0x7f1b1ea87390}, ref = 0, parent = 0x0}, id = 0x7f1b1e9cbb30 "nic1", realized = false, pending_deleted_event = true, opts = 
    0x7f1b1e9cb2f0, hotplugged = 0, parent_bus = 0x0, gpios = {lh_first = 0x0}, child_bus = {lh_first = 0x0}, num_child_bus = 0, 
  instance_id_alias = -1, alias_required_for_version = 0}
(gdb) n
[Thread 0x7f1b18770700 (LWP 13982) exited]
979         if (dev->opts) {
(gdb) info b
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x00007f1b1dd5def2 in virtio_net_device_unrealize 
                                                   at /mnt/sdb/gonglei/qemu.git/qemu/hw/net/virtio-net.c:1649
        breakpoint already hit 1 time
2       breakpoint     keep y   0x00007f1b1de8a608 in device_finalize at hw/core/qdev.c:971
        breakpoint already hit 1 time
(gdb) c
Continuing.
[New Thread 0x7f1b18770700 (LWP 15096)]
[Thread 0x7f1b18770700 (LWP 15096) exited]
[New Thread 0x7f1b18770700 (LWP 16434)]
[Thread 0x7f1b18770700 (LWP 16434) exited]

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

Hi, 

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Saturday, August 30, 2014 6:00 PM
> Subject: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> property
> 
> From: Gonglei <arei.gonglei@huawei.com>
> 
> when we remove bootindex form qdev.property to qom.property,
> we can use those functions set/get bootindex property for all
> correlative devices.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  include/sysemu/sysemu.h |  4 ++++
>  vl.c                    | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 672984c..ca231e4 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict *qdict);
>  void usb_info(Monitor *mon, const QDict *qdict);
> 
>  void check_boot_index(int32_t bootindex, Error **errp);
> +void get_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp);
> +void set_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp);
>  void del_boot_device_path(DeviceState *dev);
>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>                            const char *suffix);
> diff --git a/vl.c b/vl.c
> index f2c3b2d..4363185 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error
> **errp)
>      }
>  }
> 
> +void get_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp)
> +{
> +    visit_type_int32(v, bootindex, name, errp);
> +}
> +
> +void set_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp)
> +{
> +    int32_t boot_index;
> +    Error *local_err = NULL;
> +
> +    visit_type_int32(v, &boot_index, name, &local_err);
> +
> +    if (local_err == NULL) {
> +        /* check the bootindex existes or not in fw_boot_order list  */
> +        check_boot_index(boot_index, &local_err);
> +    }
> +
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    /* change bootindex to a new one */
> +    *bootindex = boot_index;
> +}
> +

In this version, I just change devices' bootindex value, but not update fw_boot_order
immediately. When we reboot the vm, we will call add_boot_device_path to update
fw_boot_order list. Do you think it is a good method? This method will cause a problem
that when we want set a existed bootindex which has been changed, we will get failure.
Only after vm rebooting, we can set the same bootindex again.

I have listed the example in PATCH 00/27 cover-letter:

(qemu) qom-get /machine/peripheral/nic1 bootindex
3 (0x3)
(qemu) qom-set /machine/peripheral/nic1 bootindex 3
The bootindex 3 has already been used
(qemu) qom-set /machine/peripheral/nic1 bootindex 0   --->change nic1's bootindex to 0, successful
(qemu) qom-get /machine/peripheral/nic1 bootindex
0 (0x0)
(qemu) qom-set /machine/peripheral/floppy1 bootindexA 3 -->set floppy1's bootindex to 3, failed
The bootindex 3 has already been used
(qemu) system_reset                             --> reboot vm
(qemu) qom-get /machine/peripheral/nic1 bootindex
0 (0x0)
(qemu) qom-get /machine/peripheral/floppy1 bootindexA
5 (0x5)
(qemu) qom-set /machine/peripheral/floppy1 bootindexA 3  -->set floppy1's bootindex to 3, successful
(qemu) qom-get /machine/peripheral/floppy1 bootindexA
3 (0x3)
(qemu)

Any ideas will be appreciated!

Best regards,
-Gonglei

>  static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
>  {
>      bool ret = false;
> --
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gerd Hoffmann]

  Hi,

> +void set_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp)
> > +{
> > +    int32_t boot_index;
> > +    Error *local_err = NULL;
> > +
> > +    visit_type_int32(v, &boot_index, name, &local_err);
> > +
> > +    if (local_err == NULL) {
> > +        /* check the bootindex existes or not in fw_boot_order list  */
> > +        check_boot_index(boot_index, &local_err);
> > +    }
> > +
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +    /* change bootindex to a new one */
> > +    *bootindex = boot_index;
> > +}
> > +
> 
> In this version, I just change devices' bootindex value, but not update fw_boot_order
> immediately. When we reboot the vm, we will call add_boot_device_path to update
> fw_boot_order list. Do you think it is a good method? This method will cause a problem
> that when we want set a existed bootindex which has been changed, we will get failure.
> Only after vm rebooting, we can set the same bootindex again.

Good point.  check_boot_index() doing the verification against stale
data is pointless and may even throw an error in cases where it should
not.

I guess we should add a add_boot_device_path() call to the
${device}_set_bootindex functions.  Which also means we don't need to do
it on reset (patch #6 can be dropped).  And I think we can also drop the
add_boot_device_path() calls from ${device}_realize then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> 
>   Hi,
> 
> > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > > +                   const char *name, Error **errp)
> > > +{
> > > +    int32_t boot_index;
> > > +    Error *local_err = NULL;
> > > +
> > > +    visit_type_int32(v, &boot_index, name, &local_err);
> > > +
> > > +    if (local_err == NULL) {
> > > +        /* check the bootindex existes or not in fw_boot_order list  */
> > > +        check_boot_index(boot_index, &local_err);
> > > +    }
> > > +
> > > +    if (local_err) {
> > > +        error_propagate(errp, local_err);
> > > +        return;
> > > +    }
> > > +    /* change bootindex to a new one */
> > > +    *bootindex = boot_index;
> > > +}
> > > +
> >
> > In this version, I just change devices' bootindex value, but not update
> fw_boot_order
> > immediately. When we reboot the vm, we will call add_boot_device_path to
> update
> > fw_boot_order list. Do you think it is a good method? This method will cause
> a problem
> > that when we want set a existed bootindex which has been changed, we will
> get failure.
> > Only after vm rebooting, we can set the same bootindex again.
> 
> Good point.  check_boot_index() doing the verification against stale
> data is pointless and may even throw an error in cases where it should
> not.
> 
Yes.

> I guess we should add a add_boot_device_path() call to the
> ${device}_set_bootindex functions.  Which also means we don't need to do
> it on reset (patch #6 can be dropped).  And I think we can also drop the
> add_boot_device_path() calls from ${device}_realize then.
> 
Bravo! I can't agree more with you. Thanks, Gerd.

Will do it.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
[...]
> > > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer
> > to
> > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a
> > function
> > > named is_same_fw_dev_path() to handle this situation.
> > 
> > When hot-unplugging virtio-net-pci I'd expect we free both
> > virtio-net-pci and virtio-net-device (and therefore call
> > del_boot_device_path twice, once for each device).  Can you check that?
> > 
> Yes, I can. 
> 
> The del_boot_device_path() is called only once, just for virtio-net-pci.
> For its child, virtio-net-devcie's resource is cleaned by qbus->child unrealizing 
> process, will not call device_finalize().

Then we need to fix this to make sure there's a corresponding
del_boot_device_path() call (with the same pointer) to every
add_boot_device_path() call, instead of adding a hack to
del_boot_device_path().

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

Hi,

> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Thursday, September 04, 2014 2:13 AM
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> 
> On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> [...]
> > > > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's
> pointer
> > > to
> > > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a
> > > function
> > > > named is_same_fw_dev_path() to handle this situation.
> > >
> > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > virtio-net-pci and virtio-net-device (and therefore call
> > > del_boot_device_path twice, once for each device).  Can you check that?
> > >
> > Yes, I can.
> >
> > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > For its child, virtio-net-devcie's resource is cleaned by qbus->child unrealizing
> > process, will not call device_finalize().
> 
> Then we need to fix this to make sure there's a corresponding
> del_boot_device_path() call (with the same pointer) to every
> add_boot_device_path() call, instead of adding a hack to
> del_boot_device_path().
> 
Good idea. We can add functions named $device_finalize_bootindex(), such as:

static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
                                     const char *name, Error **errp)
{
    VirtIONet *n = VIRTIO_NET(obj);
    
    set_bootindex(&n->nic_conf, v, name, errp);
    add_boot_device_path(n->nic_conf.bootindex,
                         DEVICE(obj), "/ethernet-phy@0"); 
}

static void virtio_net_finalize_bootindex(Object *obj, const char *name,
                                          void *opaque)
{
    del_boot_device_path(DEVICE(obj));
}
...
object_property_add(obj, "bootindex", "int",
                        virtio_net_get_bootindex, 
                        virtio_net_set_bootindex,
                        virtio_net_finalize_bootindex, dev, NULL);

as the previous email, we lay add_boot_device_path() in $device_set_bootindex,
and lay del_boot_device_path() in $device_finalize_bootindex is a good idea IMO.

Do you like it? Thanks.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

Hi,

> > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> >
> > On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> > [...]
> > > > > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's
> > pointer
> > > > to
> > > > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a
> > > > function
> > > > > named is_same_fw_dev_path() to handle this situation.
> > > >
> > > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > > virtio-net-pci and virtio-net-device (and therefore call
> > > > del_boot_device_path twice, once for each device).  Can you check that?
> > > >
> > > Yes, I can.
> > >
> > > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > > For its child, virtio-net-devcie's resource is cleaned by qbus->child
> unrealizing
> > > process, will not call device_finalize().
> >
> > Then we need to fix this to make sure there's a corresponding
> > del_boot_device_path() call (with the same pointer) to every
> > add_boot_device_path() call, instead of adding a hack to
> > del_boot_device_path().
> >
> Good idea. We can add functions named $device_finalize_bootindex(), such as:
> 
> static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
>                                      const char *name, Error **errp)
> {
>     VirtIONet *n = VIRTIO_NET(obj);
> 
>     set_bootindex(&n->nic_conf, v, name, errp);
>     add_boot_device_path(n->nic_conf.bootindex,
>                          DEVICE(obj), "/ethernet-phy@0");
> }
> 
> static void virtio_net_finalize_bootindex(Object *obj, const char *name,
>                                           void *opaque)
> {
>     del_boot_device_path(DEVICE(obj));
> }
> ...

Oops, I found an issue that when we hot-unplug a virtio-net-pci device,
the virtio_net_finalize_bootindex function will not be called too.
Maybe this is a potential *bug* which will cause the virtio-net-device's property
resource leak. 

Paolo, would you have a look at it? Thanks a lot!

Backtrace as below:

#0  object_unref (obj=0x7f207c640468) at qom/object.c:711
#1  0x00007f207b9850da in bus_remove_child (bus=0x7f207c6403f0, child=0x7f207c640468) at hw/core/qdev.c:76
#2  0x00007f207b987cbc in device_unparent (obj=0x7f207c640468) at hw/core/qdev.c:1013
#3  0x00007f207ba9c5a3 in object_finalize_child_property (obj=0x7f207c63fa90, name=0x7f207c6027c0 "virtio-backend", opaque=
    0x7f207c640468) at qom/object.c:1036
#4  0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c63fa90, name=0x7f207c6027c0 "virtio-backend", errp=0x0)
    at qom/object.c:778
#5  0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c63fa90, child=0x7f207c640468, errp=0x0) at qom/object.c:382
#6  0x00007f207ba9aa83 in object_unparent (obj=0x7f207c640468) at qom/object.c:391
#7  0x00007f207b986666 in bus_unparent (obj=0x7f207c6403f0) at hw/core/qdev.c:548
#8  0x00007f207ba9c5a3 in object_finalize_child_property (obj=0x7f207c63fa90, name=0x7f207c5db7a0 "virtio-bus", opaque=
    0x7f207c6403f0) at qom/object.c:1036
#9  0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c63fa90, name=0x7f207c5db7a0 "virtio-bus", errp=0x0) at qom/object.c:778
#10 0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c63fa90, child=0x7f207c6403f0, errp=0x0) at qom/object.c:382
#11 0x00007f207ba9aa83 in object_unparent (obj=0x7f207c6403f0) at qom/object.c:391
#12 0x00007f207b987c8f in device_unparent (obj=0x7f207c63fa90) at hw/core/qdev.c:1010
#13 0x00007f207ba9c5a3 in object_finalize_child_property (obj=0x7f207c4f2f90, name=0x7f207c604580 "nic1", opaque=0x7f207c63fa90)
    at qom/object.c:1036
#14 0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c4f2f90, name=0x7f207c604580 "nic1", errp=0x0) at qom/object.c:778
#15 0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c4f2f90, child=0x7f207c63fa90, errp=0x0) at qom/object.c:382
#16 0x00007f207ba9aa83 in object_unparent (obj=0x7f207c63fa90) at qom/object.c:391
#17 0x00007f207b94f7c7 in acpi_pcihp_eject_slot (s=0x7f207c5801e8, bsel=0, slots=8) at hw/acpi/pcihp.c:139
#18 0x00007f207b94fe65 in pci_write (opaque=0x7f207c5801e8, addr=8, data=8, size=4) at hw/acpi/pcihp.c:277
#19 0x00007f207b818a38 in memory_region_write_accessor (mr=0x7f207c580df8, addr=8, value=0x7f2075886b38, size=4, shift=0, mask=
    4294967295) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:443
#20 0x00007f207b818b74 in access_with_adjusted_size (addr=8, value=0x7f2075886b38, size=4, access_size_min=1, access_size_max=4, 
    access=0x7f207b8189ab <memory_region_write_accessor>, mr=0x7f207c580df8) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:480
#21 0x00007f207b81c141 in memory_region_dispatch_write (mr=0x7f207c580df8, addr=8, data=8, size=4)
    at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1137
#22 0x00007f207b81fcf0 in io_mem_write (mr=0x7f207c580df8, addr=8, val=8, size=4) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1973
#23 0x00007f207b7ca289 in address_space_rw (as=0x7f207bfeb1e0 <address_space_io>, addr=44552, buf=0x7f207b719000 "\b", len=4, 
    is_write=true) at /mnt/sdb/gonglei/qemu.git/qemu/exec.c:2042
#24 0x00007f207b815266 in kvm_handle_io (port=44552, data=0x7f207b719000, direction=1, size=4, count=1)
    at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1597
#25 0x00007f207b8158a4 in kvm_cpu_exec (cpu=0x7f207c4f6610) at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1748
#26 0x00007f207b7fcd68 in qemu_kvm_cpu_thread_fn (arg=0x7f207c4f6610) at /mnt/sdb/gonglei/qemu.git/qemu/cpus.c:940
#27 0x00007f2078a357f6 in start_thread () from /lib64/libpthread.so.0
#28 0x00007f207879109d in clone () from /lib64/libc.so.6
#29 0x0000000000000000 in ?? ()
(gdb) p *obj
$64 = {class = 0x7f207c4bc5f0, free = 0x0, properties = {tqh_first = 0x7f207c591fd0, tqh_last = 0x7f207c6027a8}, ref = 3, parent = 
    0x7f207c63fa90}
(gdb) n
712         if (!obj) {
(gdb) 
715         g_assert(obj->ref > 0);
(gdb) 
718         if (atomic_fetch_dec(&obj->ref) == 1) {
(gdb) 
721     }
(gdb) p *obj
$65 = {class = 0x7f207c4bc5f0, free = 0x0, properties = {tqh_first = 0x7f207c591fd0, tqh_last = 0x7f207c6027a8}, ref = 2, parent = 
    0x7f207c63fa90}

Because of the virtio-net-device object's ref is 3, will not enter object_finailze(), *leak resource*. Am I wrong?

Best regards,
-Gonglei

> object_property_add(obj, "bootindex", "int",
>                         virtio_net_get_bootindex,
>                         virtio_net_set_bootindex,
>                         virtio_net_finalize_bootindex, dev, NULL);
> 
> as the previous email, we lay add_boot_device_path() in
> $device_set_bootindex,
> and lay del_boot_device_path() in $device_finalize_bootindex is a good idea
> IMO.
> 
> Do you like it? Thanks.
> 
> Best regards,
> -Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

> Subject: RE: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> 
> Hi,
> 
> > > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path
> function
> > >
> > > On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> > > [...]
> > > > > > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's
> > > pointer
> > > > > to
> > > > > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add
> a
> > > > > function
> > > > > > named is_same_fw_dev_path() to handle this situation.
> > > > >
> > > > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > > > virtio-net-pci and virtio-net-device (and therefore call
> > > > > del_boot_device_path twice, once for each device).  Can you check
> that?
> > > > >
> > > > Yes, I can.
> > > >
> > > > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > > > For its child, virtio-net-devcie's resource is cleaned by qbus->child
> > unrealizing
> > > > process, will not call device_finalize().
> > >
> > > Then we need to fix this to make sure there's a corresponding
> > > del_boot_device_path() call (with the same pointer) to every
> > > add_boot_device_path() call, instead of adding a hack to
> > > del_boot_device_path().
> > >
> > Good idea. We can add functions named $device_finalize_bootindex(), such
> as:
> >
> > static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
> >                                      const char *name, Error
> **errp)
> > {
> >     VirtIONet *n = VIRTIO_NET(obj);
> >
> >     set_bootindex(&n->nic_conf, v, name, errp);
> >     add_boot_device_path(n->nic_conf.bootindex,
> >                          DEVICE(obj), "/ethernet-phy@0");
> > }
> >
> > static void virtio_net_finalize_bootindex(Object *obj, const char *name,
> >                                           void *opaque)
> > {
> >     del_boot_device_path(DEVICE(obj));
> > }
> > ...
> 
> Oops, I found an issue that when we hot-unplug a virtio-net-pci device,
> the virtio_net_finalize_bootindex function will not be called too.
> Maybe this is a potential *bug* which will cause the virtio-net-device's property
> resource leak.
> 
> Paolo, would you have a look at it? Thanks a lot!
> 
> Backtrace as below:
> 
> #0  object_unref (obj=0x7f207c640468) at qom/object.c:711
> #1  0x00007f207b9850da in bus_remove_child (bus=0x7f207c6403f0,
> child=0x7f207c640468) at hw/core/qdev.c:76
> #2  0x00007f207b987cbc in device_unparent (obj=0x7f207c640468) at
> hw/core/qdev.c:1013
> #3  0x00007f207ba9c5a3 in object_finalize_child_property
> (obj=0x7f207c63fa90, name=0x7f207c6027c0 "virtio-backend", opaque=
>     0x7f207c640468) at qom/object.c:1036
> #4  0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c63fa90,
> name=0x7f207c6027c0 "virtio-backend", errp=0x0)
>     at qom/object.c:778
> #5  0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c63fa90,
> child=0x7f207c640468, errp=0x0) at qom/object.c:382
> #6  0x00007f207ba9aa83 in object_unparent (obj=0x7f207c640468) at
> qom/object.c:391
> #7  0x00007f207b986666 in bus_unparent (obj=0x7f207c6403f0) at
> hw/core/qdev.c:548
> #8  0x00007f207ba9c5a3 in object_finalize_child_property
> (obj=0x7f207c63fa90, name=0x7f207c5db7a0 "virtio-bus", opaque=
>     0x7f207c6403f0) at qom/object.c:1036
> #9  0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c63fa90,
> name=0x7f207c5db7a0 "virtio-bus", errp=0x0) at qom/object.c:778
> #10 0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c63fa90,
> child=0x7f207c6403f0, errp=0x0) at qom/object.c:382
> #11 0x00007f207ba9aa83 in object_unparent (obj=0x7f207c6403f0) at
> qom/object.c:391
> #12 0x00007f207b987c8f in device_unparent (obj=0x7f207c63fa90) at
> hw/core/qdev.c:1010
> #13 0x00007f207ba9c5a3 in object_finalize_child_property
> (obj=0x7f207c4f2f90, name=0x7f207c604580 "nic1", opaque=0x7f207c63fa90)
>     at qom/object.c:1036
> #14 0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c4f2f90,
> name=0x7f207c604580 "nic1", errp=0x0) at qom/object.c:778
> #15 0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c4f2f90,
> child=0x7f207c63fa90, errp=0x0) at qom/object.c:382
> #16 0x00007f207ba9aa83 in object_unparent (obj=0x7f207c63fa90) at
> qom/object.c:391
> #17 0x00007f207b94f7c7 in acpi_pcihp_eject_slot (s=0x7f207c5801e8, bsel=0,
> slots=8) at hw/acpi/pcihp.c:139
> #18 0x00007f207b94fe65 in pci_write (opaque=0x7f207c5801e8, addr=8,
> data=8, size=4) at hw/acpi/pcihp.c:277
> #19 0x00007f207b818a38 in memory_region_write_accessor
> (mr=0x7f207c580df8, addr=8, value=0x7f2075886b38, size=4, shift=0, mask=
>     4294967295) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:443
> #20 0x00007f207b818b74 in access_with_adjusted_size (addr=8,
> value=0x7f2075886b38, size=4, access_size_min=1, access_size_max=4,
>     access=0x7f207b8189ab <memory_region_write_accessor>,
> mr=0x7f207c580df8) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:480
> #21 0x00007f207b81c141 in memory_region_dispatch_write
> (mr=0x7f207c580df8, addr=8, data=8, size=4)
>     at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1137
> #22 0x00007f207b81fcf0 in io_mem_write (mr=0x7f207c580df8, addr=8, val=8,
> size=4) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1973
> #23 0x00007f207b7ca289 in address_space_rw (as=0x7f207bfeb1e0
> <address_space_io>, addr=44552, buf=0x7f207b719000 "\b", len=4,
>     is_write=true) at /mnt/sdb/gonglei/qemu.git/qemu/exec.c:2042
> #24 0x00007f207b815266 in kvm_handle_io (port=44552,
> data=0x7f207b719000, direction=1, size=4, count=1)
>     at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1597
> #25 0x00007f207b8158a4 in kvm_cpu_exec (cpu=0x7f207c4f6610) at
> /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1748
> #26 0x00007f207b7fcd68 in qemu_kvm_cpu_thread_fn (arg=0x7f207c4f6610)
> at /mnt/sdb/gonglei/qemu.git/qemu/cpus.c:940
> #27 0x00007f2078a357f6 in start_thread () from /lib64/libpthread.so.0
> #28 0x00007f207879109d in clone () from /lib64/libc.so.6
> #29 0x0000000000000000 in ?? ()
> (gdb) p *obj
> $64 = {class = 0x7f207c4bc5f0, free = 0x0, properties = {tqh_first =
> 0x7f207c591fd0, tqh_last = 0x7f207c6027a8}, ref = 3, parent =
>     0x7f207c63fa90}
> (gdb) n
> 712         if (!obj) {
> (gdb)
> 715         g_assert(obj->ref > 0);
> (gdb)
> 718         if (atomic_fetch_dec(&obj->ref) == 1) {
> (gdb)
> 721     }
> (gdb) p *obj
> $65 = {class = 0x7f207c4bc5f0, free = 0x0, properties = {tqh_first =
> 0x7f207c591fd0, tqh_last = 0x7f207c6027a8}, ref = 2, parent =
>     0x7f207c63fa90}
> 
> Because of the virtio-net-device object's ref is 3, will not enter object_finailze(),
> *leak resource*. Am I wrong?
> 

I've confirmed that this is a bug, and have posted a patch fix it. 

Please review, thanks!

[PATCH] virtio-pci: fix virtio-net child refcount in transports

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

Hi,

> 
> I've confirmed that this is a bug, and have posted a patch fix it.
> 
> Please review, thanks!
> 
> [PATCH] virtio-pci: fix virtio-net child refcount in transports
> 
> Best regards,
> -Gonglei

So, about Gerd's previous question:

> When hot-unplugging virtio-net-pci I'd expect we free both
> virtio-net-pci and virtio-net-device (and therefore call
> del_boot_device_path twice, once for each device).  Can you check that?
>  
After fix that bug, you are right. The del_boot_device_path will be called twice.
I can rework del_boot_device_path(), remove is_same_fw_dev_path().

The new del_boot_device_path function as below:

void del_boot_device_path(DeviceState *dev)
{
    FWBootEntry *i;

    assert(dev != NULL);

    /* remove all entries of the assigned device */
    QTAILQ_FOREACH(i, &fw_boot_order, link) {
        if (i->dev == NULL) {
            continue;
        }
        if (i->dev == dev) {
            QTAILQ_REMOVE(&fw_boot_order, i, link);
            g_free(i->suffix);
            g_free(i);

            break;
        }
    }
}

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Thu, Sep 04, 2014 at 03:01:41AM +0000, Gonglei (Arei) wrote:
> Hi,
> 
> > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > Sent: Thursday, September 04, 2014 2:13 AM
> > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> > 
> > On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> > [...]
> > > > > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's
> > pointer
> > > > to
> > > > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a
> > > > function
> > > > > named is_same_fw_dev_path() to handle this situation.
> > > >
> > > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > > virtio-net-pci and virtio-net-device (and therefore call
> > > > del_boot_device_path twice, once for each device).  Can you check that?
> > > >
> > > Yes, I can.
> > >
> > > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > > For its child, virtio-net-devcie's resource is cleaned by qbus->child unrealizing
> > > process, will not call device_finalize().
> > 
> > Then we need to fix this to make sure there's a corresponding
> > del_boot_device_path() call (with the same pointer) to every
> > add_boot_device_path() call, instead of adding a hack to
> > del_boot_device_path().
> > 
> Good idea. We can add functions named $device_finalize_bootindex(), such as:
> 
> static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
>                                      const char *name, Error **errp)
> {
>     VirtIONet *n = VIRTIO_NET(obj);
>     
>     set_bootindex(&n->nic_conf, v, name, errp);
>     add_boot_device_path(n->nic_conf.bootindex,
>                          DEVICE(obj), "/ethernet-phy@0"); 
> }
> 
> static void virtio_net_finalize_bootindex(Object *obj, const char *name,
>                                           void *opaque)
> {
>     del_boot_device_path(DEVICE(obj));
> }
> ...
> object_property_add(obj, "bootindex", "int",
>                         virtio_net_get_bootindex, 
>                         virtio_net_set_bootindex,
>                         virtio_net_finalize_bootindex, dev, NULL);
> 
> as the previous email, we lay add_boot_device_path() in $device_set_bootindex,
> and lay del_boot_device_path() in $device_finalize_bootindex is a good idea IMO.

Whatever the approach we use, can we have a wrapper to reduce code
duplication? e.g. a:
  void device_add_bootindex_property(DeviceState *dev, int32_t *field, const char *suffix)
function.

Then instead of reimplementing set/get/finalize functions, device code
could just call something like:
  device_add_bootindex_property(dev, &n->nic_conf.bootindex,
                                "/ethernet-phy@0");

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 06/27] bootindex: move setting bootindex on reset() instead of realize/init()

[Eduardo Habkost]

On Sat, Aug 30, 2014 at 06:00:06PM +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Only in this way, we can assure the new bootindex take effect
> during vm rebooting.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

An API that doesn't work unless it is called at a very specific moment
(during reset) is very easy to misuse.

Why not simply make the core bootindex/reset code smart enough to update
the tables on reset if add_boot_device_path() was called at any moment,
instead of requiring all devices to call add_boot_device_path() on
reset?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Eduardo Habkost]

On Sat, Aug 30, 2014 at 06:00:07PM +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> when we remove bootindex form qdev.property to qom.property,
> we can use those functions set/get bootindex property for all
> correlative devices.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  include/sysemu/sysemu.h |  4 ++++
>  vl.c                    | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 672984c..ca231e4 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict *qdict);
>  void usb_info(Monitor *mon, const QDict *qdict);
>  
>  void check_boot_index(int32_t bootindex, Error **errp);
> +void get_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp);
> +void set_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp);
>  void del_boot_device_path(DeviceState *dev);
>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>                            const char *suffix);
> diff --git a/vl.c b/vl.c
> index f2c3b2d..4363185 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error **errp)
>      }
>  }
>  
> +void get_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp)
> +{
> +    visit_type_int32(v, bootindex, name, errp);
> +}
> +
> +void set_bootindex(int32_t *bootindex, Visitor *v,
> +                   const char *name, Error **errp)
> +{
> +    int32_t boot_index;
> +    Error *local_err = NULL;
> +
> +    visit_type_int32(v, &boot_index, name, &local_err);
> +
> +    if (local_err == NULL) {
> +        /* check the bootindex existes or not in fw_boot_order list  */
> +        check_boot_index(boot_index, &local_err);
> +    }
> +
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    /* change bootindex to a new one */
> +    *bootindex = boot_index;

I believe the following pattern is more usual (and I find it easier to
read):

    visit_type_int32(v, &boot_index, name, &local_err);
    if (local_err) {
        goto out;
    }

    check_boot_index(boot_index, &local_err);
    if (local_err) {
        goto out;
    }

    *bootindex = boot_index;

out:
    if (local_err) {
        error_propagate(errp, local_err);
    }

Also, this function (the property setter) is where I expected
add_boot_device_path() to be called, instead of requiring every device
to add a reset handler and call add_boot_device_path() manually.

I suggest creating a new file for all the bootindex/boot-device code
(maybe call it bootdevice.c?), instead of adding more code to vl.c.

> +}
> +
>  static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
>  {
>      bool ret = false;
> -- 
> 1.7.12.4
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 06/27] bootindex: move setting bootindex on reset() instead of realize/init()

[Gonglei (Arei)]

> Subject: Re: [PATCH v6 06/27] bootindex: move setting bootindex on reset()
> instead of realize/init()
> 
> On Sat, Aug 30, 2014 at 06:00:06PM +0800, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > Only in this way, we can assure the new bootindex take effect
> > during vm rebooting.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> 
> An API that doesn't work unless it is called at a very specific moment
> (during reset) is very easy to misuse.
> 
> Why not simply make the core bootindex/reset code smart enough to update
> the tables on reset if add_boot_device_path() was called at any moment,
> instead of requiring all devices to call add_boot_device_path() on
> reset?
> 
Yes. I have moved this call to $device_set_bootindex() by Gerd's suggestion.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

> Subject: Re: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> property
> 
> On Sat, Aug 30, 2014 at 06:00:07PM +0800, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > when we remove bootindex form qdev.property to qom.property,
> > we can use those functions set/get bootindex property for all
> > correlative devices.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  include/sysemu/sysemu.h |  4 ++++
> >  vl.c                    | 27 +++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 672984c..ca231e4 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict
> *qdict);
> >  void usb_info(Monitor *mon, const QDict *qdict);
> >
> >  void check_boot_index(int32_t bootindex, Error **errp);
> > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp);
> > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp);
> >  void del_boot_device_path(DeviceState *dev);
> >  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> >                            const char *suffix);
> > diff --git a/vl.c b/vl.c
> > index f2c3b2d..4363185 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error
> **errp)
> >      }
> >  }
> >
> > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp)
> > +{
> > +    visit_type_int32(v, bootindex, name, errp);
> > +}
> > +
> > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > +                   const char *name, Error **errp)
> > +{
> > +    int32_t boot_index;
> > +    Error *local_err = NULL;
> > +
> > +    visit_type_int32(v, &boot_index, name, &local_err);
> > +
> > +    if (local_err == NULL) {
> > +        /* check the bootindex existes or not in fw_boot_order list  */
> > +        check_boot_index(boot_index, &local_err);
> > +    }
> > +
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +    /* change bootindex to a new one */
> > +    *bootindex = boot_index;
> 
> I believe the following pattern is more usual (and I find it easier to
> read):
> 
>     visit_type_int32(v, &boot_index, name, &local_err);
>     if (local_err) {
>         goto out;
>     }
> 
>     check_boot_index(boot_index, &local_err);
>     if (local_err) {
>         goto out;
>     }
> 
>     *bootindex = boot_index;
> 
> out:
>     if (local_err) {
>         error_propagate(errp, local_err);
>     }
> 
OK, agreed. Thanks!

> Also, this function (the property setter) is where I expected
> add_boot_device_path() to be called, instead of requiring every device
> to add a reset handler and call add_boot_device_path() manually.
> 
Optional, I have said in previous mail. I think we should lay call 
add_boot_device_path() in $device_set_bootindex(), not the common
function set_bootindex(). We can save the unitariness of a function, right?

> I suggest creating a new file for all the bootindex/boot-device code
> (maybe call it bootdevice.c?), instead of adding more code to vl.c.
> 
Agreed. But how do we do for the original code in vl.c, move them to
new file too? Maybe vl.c is need a big reconstruction, but is out of scope
of this series IMHO.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> 
> On Thu, Sep 04, 2014 at 03:01:41AM +0000, Gonglei (Arei) wrote:
> > Hi,
> >
> > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > Sent: Thursday, September 04, 2014 2:13 AM
> > > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path
> function
> > >
> > > On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> > > [...]
> > > > > > 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's
> > > pointer
> > > > > to
> > > > > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add
> a
> > > > > function
> > > > > > named is_same_fw_dev_path() to handle this situation.
> > > > >
> > > > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > > > virtio-net-pci and virtio-net-device (and therefore call
> > > > > del_boot_device_path twice, once for each device).  Can you check
> that?
> > > > >
> > > > Yes, I can.
> > > >
> > > > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > > > For its child, virtio-net-devcie's resource is cleaned by qbus->child
> unrealizing
> > > > process, will not call device_finalize().
> > >
> > > Then we need to fix this to make sure there's a corresponding
> > > del_boot_device_path() call (with the same pointer) to every
> > > add_boot_device_path() call, instead of adding a hack to
> > > del_boot_device_path().
> > >
> > Good idea. We can add functions named $device_finalize_bootindex(), such
> as:
> >
> > static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
> >                                      const char *name, Error
> **errp)
> > {
> >     VirtIONet *n = VIRTIO_NET(obj);
> >
> >     set_bootindex(&n->nic_conf, v, name, errp);
> >     add_boot_device_path(n->nic_conf.bootindex,
> >                          DEVICE(obj), "/ethernet-phy@0");
> > }
> >
> > static void virtio_net_finalize_bootindex(Object *obj, const char *name,
> >                                           void *opaque)
> > {
> >     del_boot_device_path(DEVICE(obj));
> > }
> > ...
> > object_property_add(obj, "bootindex", "int",
> >                         virtio_net_get_bootindex,
> >                         virtio_net_set_bootindex,
> >                         virtio_net_finalize_bootindex, dev, NULL);
> >
> > as the previous email, we lay add_boot_device_path() in
> $device_set_bootindex,
> > and lay del_boot_device_path() in $device_finalize_bootindex is a good idea
> IMO.
> 
> Whatever the approach we use, 

Hmm... we just call this function in device_finalize() as I have used.
My above explanation is wrong because of a virtio-net's bug. Thanks.

> can we have a wrapper to reduce code
> duplication? e.g. a:
>   void device_add_bootindex_property(DeviceState *dev, int32_t *field, const
> char *suffix)
> function.
> 
> Then instead of reimplementing set/get/finalize functions, device code
> could just call something like:
>   device_add_bootindex_property(dev, &n->nic_conf.bootindex,
>                                 "/ethernet-phy@0");
> 

This way we cannot attach our target that changing bootindex and take effect
during vm rebooting.

> --
> Eduardo

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Eduardo Habkost]

On Fri, Sep 05, 2014 at 12:37:35AM +0000, Gonglei (Arei) wrote:
> > Subject: Re: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> > property
> > 
> > On Sat, Aug 30, 2014 at 06:00:07PM +0800, arei.gonglei@huawei.com wrote:
> > > From: Gonglei <arei.gonglei@huawei.com>
> > >
> > > when we remove bootindex form qdev.property to qom.property,
> > > we can use those functions set/get bootindex property for all
> > > correlative devices.
> > >
> > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > ---
> > >  include/sysemu/sysemu.h |  4 ++++
> > >  vl.c                    | 27 +++++++++++++++++++++++++++
> > >  2 files changed, 31 insertions(+)
> > >
> > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > index 672984c..ca231e4 100644
> > > --- a/include/sysemu/sysemu.h
> > > +++ b/include/sysemu/sysemu.h
> > > @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict
> > *qdict);
> > >  void usb_info(Monitor *mon, const QDict *qdict);
> > >
> > >  void check_boot_index(int32_t bootindex, Error **errp);
> > > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > > +                   const char *name, Error **errp);
> > > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > > +                   const char *name, Error **errp);
> > >  void del_boot_device_path(DeviceState *dev);
> > >  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> > >                            const char *suffix);
> > > diff --git a/vl.c b/vl.c
> > > index f2c3b2d..4363185 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error
> > **errp)
> > >      }
> > >  }
> > >
> > > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > > +                   const char *name, Error **errp)
> > > +{
> > > +    visit_type_int32(v, bootindex, name, errp);
> > > +}
> > > +
> > > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > > +                   const char *name, Error **errp)
> > > +{
> > > +    int32_t boot_index;
> > > +    Error *local_err = NULL;
> > > +
> > > +    visit_type_int32(v, &boot_index, name, &local_err);
> > > +
> > > +    if (local_err == NULL) {
> > > +        /* check the bootindex existes or not in fw_boot_order list  */
> > > +        check_boot_index(boot_index, &local_err);
> > > +    }
> > > +
> > > +    if (local_err) {
> > > +        error_propagate(errp, local_err);
> > > +        return;
> > > +    }
> > > +    /* change bootindex to a new one */
> > > +    *bootindex = boot_index;
> > 
> > I believe the following pattern is more usual (and I find it easier to
> > read):
> > 
> >     visit_type_int32(v, &boot_index, name, &local_err);
> >     if (local_err) {
> >         goto out;
> >     }
> > 
> >     check_boot_index(boot_index, &local_err);
> >     if (local_err) {
> >         goto out;
> >     }
> > 
> >     *bootindex = boot_index;
> > 
> > out:
> >     if (local_err) {
> >         error_propagate(errp, local_err);
> >     }
> > 
> OK, agreed. Thanks!
> 
> > Also, this function (the property setter) is where I expected
> > add_boot_device_path() to be called, instead of requiring every device
> > to add a reset handler and call add_boot_device_path() manually.
> > 
> Optional, I have said in previous mail. I think we should lay call 
> add_boot_device_path() in $device_set_bootindex(), not the common
> function set_bootindex(). We can save the unitariness of a function, right?

If all (or most) $device_set_bootindex() functions you are adding look
exactly the same (with just a different struct field), we can have a
device_add_bootindex_property() wrapper like I suggested on my reply to
02/27, instead of copying/pasting the same setter/getter code on every
device.

> 
> > I suggest creating a new file for all the bootindex/boot-device code
> > (maybe call it bootdevice.c?), instead of adding more code to vl.c.
> > 
> Agreed. But how do we do for the original code in vl.c, move them to
> new file too? Maybe vl.c is need a big reconstruction, but is out of scope
> of this series IMHO.

Yes, I would move the existing bootindex code (fw_boot_order,
add_boot_device_path(), get_boot_device(), get_boot_devices_list()) to
the new file as the first step, and then make the changes inside the new
file. See, for example, how we created numa.c.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Friday, September 05, 2014 9:55 AM
> Subject: Re: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> property
> 
> On Fri, Sep 05, 2014 at 12:37:35AM +0000, Gonglei (Arei) wrote:
> > > Subject: Re: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> > > property
> > >
> > > On Sat, Aug 30, 2014 at 06:00:07PM +0800, arei.gonglei@huawei.com
> wrote:
> > > > From: Gonglei <arei.gonglei@huawei.com>
> > > >
> > > > when we remove bootindex form qdev.property to qom.property,
> > > > we can use those functions set/get bootindex property for all
> > > > correlative devices.
> > > >
> > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > > ---
> > > >  include/sysemu/sysemu.h |  4 ++++
> > > >  vl.c                    | 27 +++++++++++++++++++++++++++
> > > >  2 files changed, 31 insertions(+)
> > > >
> > > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > > index 672984c..ca231e4 100644
> > > > --- a/include/sysemu/sysemu.h
> > > > +++ b/include/sysemu/sysemu.h
> > > > @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict
> > > *qdict);
> > > >  void usb_info(Monitor *mon, const QDict *qdict);
> > > >
> > > >  void check_boot_index(int32_t bootindex, Error **errp);
> > > > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > > > +                   const char *name, Error **errp);
> > > > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > > > +                   const char *name, Error **errp);
> > > >  void del_boot_device_path(DeviceState *dev);
> > > >  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
> > > >                            const char *suffix);
> > > > diff --git a/vl.c b/vl.c
> > > > index f2c3b2d..4363185 100644
> > > > --- a/vl.c
> > > > +++ b/vl.c
> > > > @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex,
> Error
> > > **errp)
> > > >      }
> > > >  }
> > > >
> > > > +void get_bootindex(int32_t *bootindex, Visitor *v,
> > > > +                   const char *name, Error **errp)
> > > > +{
> > > > +    visit_type_int32(v, bootindex, name, errp);
> > > > +}
> > > > +
> > > > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > > > +                   const char *name, Error **errp)
> > > > +{
> > > > +    int32_t boot_index;
> > > > +    Error *local_err = NULL;
> > > > +
> > > > +    visit_type_int32(v, &boot_index, name, &local_err);
> > > > +
> > > > +    if (local_err == NULL) {
> > > > +        /* check the bootindex existes or not in fw_boot_order list  */
> > > > +        check_boot_index(boot_index, &local_err);
> > > > +    }
> > > > +
> > > > +    if (local_err) {
> > > > +        error_propagate(errp, local_err);
> > > > +        return;
> > > > +    }
> > > > +    /* change bootindex to a new one */
> > > > +    *bootindex = boot_index;
> > >
> > > I believe the following pattern is more usual (and I find it easier to
> > > read):
> > >
> > >     visit_type_int32(v, &boot_index, name, &local_err);
> > >     if (local_err) {
> > >         goto out;
> > >     }
> > >
> > >     check_boot_index(boot_index, &local_err);
> > >     if (local_err) {
> > >         goto out;
> > >     }
> > >
> > >     *bootindex = boot_index;
> > >
> > > out:
> > >     if (local_err) {
> > >         error_propagate(errp, local_err);
> > >     }
> > >
> > OK, agreed. Thanks!
> >
> > > Also, this function (the property setter) is where I expected
> > > add_boot_device_path() to be called, instead of requiring every device
> > > to add a reset handler and call add_boot_device_path() manually.
> > >
> > Optional, I have said in previous mail. I think we should lay call
> > add_boot_device_path() in $device_set_bootindex(), not the common
> > function set_bootindex(). We can save the unitariness of a function, right?
> 
> If all (or most) $device_set_bootindex() functions you are adding look
> exactly the same (with just a different struct field), we can have a
> device_add_bootindex_property() wrapper like I suggested on my reply to
> 02/27, instead of copying/pasting the same setter/getter code on every
> device.
> 
I want to know where the device_add_bootindex_property() be called?
which assure that new bootindex take effect during vm rebooting?

> >
> > > I suggest creating a new file for all the bootindex/boot-device code
> > > (maybe call it bootdevice.c?), instead of adding more code to vl.c.
> > >
> > Agreed. But how do we do for the original code in vl.c, move them to
> > new file too? Maybe vl.c is need a big reconstruction, but is out of scope
> > of this series IMHO.
> 
> Yes, I would move the existing bootindex code (fw_boot_order,
> add_boot_device_path(), get_boot_device(), get_boot_devices_list()) to
> the new file as the first step, and then make the changes inside the new
> file. See, for example, how we created numa.c.
> 
Sounds good to me. Thanks!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Fri, Sep 05, 2014 at 12:44:56AM +0000, Gonglei (Arei) wrote:
[...]
> > can we have a wrapper to reduce code
> > duplication? e.g. a:
> >   void device_add_bootindex_property(DeviceState *dev, int32_t *field, const
> > char *suffix)
> > function.
> > 
> > Then instead of reimplementing set/get/finalize functions, device code
> > could just call something like:
> >   device_add_bootindex_property(dev, &n->nic_conf.bootindex,
> >                                 "/ethernet-phy@0");
> > 
> 
> This way we cannot attach our target that changing bootindex and take effect
> during vm rebooting.

I don't understand what you mean, here. Whatever you are planning to do on the
device-specific setter/getters, if they all look the same, you can write a
common getter/setter to do exactly the same steps, and register it inside
device_add_bootindex_property(). This way, patches 08/27 to 26/27 can become
one-liners, instead of adding more 20 lines each.

I mean something like this:

typedef struct BootIndexProperty
{
    int32_t *field;
    const char *suffix;
} BootIndexProperty;

static void device_get_bootindex(Object *obj, Visitor *v, void *opaque,
                                 const char *name, Error **errp)
{
    BootIndexProperty *prop = opaque;
    visit_type_int32(v, prop->field, name, errp);
}

static void device_set_bootindex(Object *obj, Visitor *v, void *opaque,
                                 const char *name, Error **errp)
{
    DeviceState *dev = DEVICE(obj);
    BootIndexProperty *prop = opaque;
    int32_t boot_index;
    Error *local_err = NULL;

    visit_type_int32(v, &boot_index, name, &local_err);
    if (local_err) {
        goto out;
    }

    check_boot_index(boot_index, &local_err);
    if (local_err) {
        goto out;
    }

    *prop->field = boot_index;
    add_boot_device_path(boot_index, dev, prop->suffix);

out:
    if (local_err) {
        error_propagate(errp, local_err);
    }
}

static void property_release_bootindex(Object *obj, const char *name,
                                       void *opaque)

{
    BootIndexProperty *prop = opaque;
    DeviceState *dev = DEVICE(obj);
    del_boot_device_path(dev);
    g_free(prop);
}

void device_add_bootindex_property(DeviceState *dev, int32_t *field,
                                   const char *suffix, Error **errp)
{
    Error *local_err = NULL;
    BootIndexProperty *prop = g_malloc0(sizeof(*prop));

    prop->field = field;
    prop->suffix = suffix;
    object_property_add(OBJECT(dev), "bootindex", "int32",
                        device_get_bootindex,
                        device_set_bootindex,
                        property_release_bootindex,
                        prop, &local_err);

    if (local_err) {
        error_propagate(errp, local_err);
        g_free(prop);
    }
}

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Eduardo Habkost]

On Fri, Sep 05, 2014 at 02:07:23AM +0000, Gonglei (Arei) wrote:
[...]
> > > > Also, this function (the property setter) is where I expected
> > > > add_boot_device_path() to be called, instead of requiring every device
> > > > to add a reset handler and call add_boot_device_path() manually.
> > > >
> > > Optional, I have said in previous mail. I think we should lay call
> > > add_boot_device_path() in $device_set_bootindex(), not the common
> > > function set_bootindex(). We can save the unitariness of a function, right?
> > 
> > If all (or most) $device_set_bootindex() functions you are adding look
> > exactly the same (with just a different struct field), we can have a
> > device_add_bootindex_property() wrapper like I suggested on my reply to
> > 02/27, instead of copying/pasting the same setter/getter code on every
> > device.
> > 
> I want to know where the device_add_bootindex_property() be called?
> which assure that new bootindex take effect during vm rebooting?

In exactly the same place you were going to call object_property_add()
(instance_init). It would be just a wrapper around object_property_add()
that won't require you to write new setter/getter functions for each
device.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Friday, September 05, 2014 10:20 AM
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> 
> On Fri, Sep 05, 2014 at 12:44:56AM +0000, Gonglei (Arei) wrote:
> [...]
> > > can we have a wrapper to reduce code
> > > duplication? e.g. a:
> > >   void device_add_bootindex_property(DeviceState *dev, int32_t *field,
> const
> > > char *suffix)
> > > function.
> > >
> > > Then instead of reimplementing set/get/finalize functions, device code
> > > could just call something like:
> > >   device_add_bootindex_property(dev, &n->nic_conf.bootindex,
> > >                                 "/ethernet-phy@0");
> > >
> >
> > This way we cannot attach our target that changing bootindex and take
> effect
> > during vm rebooting.
> 
> I don't understand what you mean, here. Whatever you are planning to do on
> the
> device-specific setter/getters, if they all look the same, you can write a
> common getter/setter to do exactly the same steps, and register it inside
> device_add_bootindex_property(). This way, patches 08/27 to 26/27 can
> become
> one-liners, instead of adding more 20 lines each.
> 
> I mean something like this:
> 
OK. Thanks for explanation. :)

I have two questions:

1) virtio-net-pci device do need special handle in device_set_bootindex(). 
2) isa-fdc' property is bootindexA and bootindexB, maybe we
can add more a parameter for device_add_bootindex_property()?

Best regards,
-Gonglei

> typedef struct BootIndexProperty
> {
>     int32_t *field;
>     const char *suffix;
> } BootIndexProperty;
> 
> static void device_get_bootindex(Object *obj, Visitor *v, void *opaque,
>                                  const char *name, Error **errp)
> {
>     BootIndexProperty *prop = opaque;
>     visit_type_int32(v, prop->field, name, errp);
> }
> 
> static void device_set_bootindex(Object *obj, Visitor *v, void *opaque,
>                                  const char *name, Error **errp)
> {
>     DeviceState *dev = DEVICE(obj);
>     BootIndexProperty *prop = opaque;
>     int32_t boot_index;
>     Error *local_err = NULL;
> 
>     visit_type_int32(v, &boot_index, name, &local_err);
>     if (local_err) {
>         goto out;
>     }
> 
>     check_boot_index(boot_index, &local_err);
>     if (local_err) {
>         goto out;
>     }
> 
>     *prop->field = boot_index;
>     add_boot_device_path(boot_index, dev, prop->suffix);
> 
> out:
>     if (local_err) {
>         error_propagate(errp, local_err);
>     }
> }
> 
> static void property_release_bootindex(Object *obj, const char *name,
>                                        void *opaque)
> 
> {
>     BootIndexProperty *prop = opaque;
>     DeviceState *dev = DEVICE(obj);
>     del_boot_device_path(dev);
>     g_free(prop);
> }
> 
> void device_add_bootindex_property(DeviceState *dev, int32_t *field,
>                                    const char *suffix, Error **errp)
> {
>     Error *local_err = NULL;
>     BootIndexProperty *prop = g_malloc0(sizeof(*prop));
> 
>     prop->field = field;
>     prop->suffix = suffix;
>     object_property_add(OBJECT(dev), "bootindex", "int32",
>                         device_get_bootindex,
>                         device_set_bootindex,
>                         property_release_bootindex,
>                         prop, &local_err);
> 
>     if (local_err) {
>         error_propagate(errp, local_err);
>         g_free(prop);
>     }
> }
> 
> --
> Eduardo

Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Friday, September 05, 2014 10:22 AM
> Subject: Re: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex
> property
> 
> On Fri, Sep 05, 2014 at 02:07:23AM +0000, Gonglei (Arei) wrote:
> [...]
> > > > > Also, this function (the property setter) is where I expected
> > > > > add_boot_device_path() to be called, instead of requiring every device
> > > > > to add a reset handler and call add_boot_device_path() manually.
> > > > >
> > > > Optional, I have said in previous mail. I think we should lay call
> > > > add_boot_device_path() in $device_set_bootindex(), not the common
> > > > function set_bootindex(). We can save the unitariness of a function, right?
> > >
> > > If all (or most) $device_set_bootindex() functions you are adding look
> > > exactly the same (with just a different struct field), we can have a
> > > device_add_bootindex_property() wrapper like I suggested on my reply to
> > > 02/27, instead of copying/pasting the same setter/getter code on every
> > > device.
> > >
> > I want to know where the device_add_bootindex_property() be called?
> > which assure that new bootindex take effect during vm rebooting?
> 
> In exactly the same place you were going to call object_property_add()
> (instance_init). It would be just a wrapper around object_property_add()
> that won't require you to write new setter/getter functions for each
> device.
> 
Got it, thanks!

Best regards,
-Gonglei

> --
> Eduardo

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Fri, Sep 05, 2014 at 02:42:48AM +0000, Gonglei (Arei) wrote:
> > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > Sent: Friday, September 05, 2014 10:20 AM
> > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> > 
> > On Fri, Sep 05, 2014 at 12:44:56AM +0000, Gonglei (Arei) wrote:
> > [...]
> > > > can we have a wrapper to reduce code
> > > > duplication? e.g. a:
> > > >   void device_add_bootindex_property(DeviceState *dev, int32_t *field,
> > const
> > > > char *suffix)
> > > > function.
> > > >
> > > > Then instead of reimplementing set/get/finalize functions, device code
> > > > could just call something like:
> > > >   device_add_bootindex_property(dev, &n->nic_conf.bootindex,
> > > >                                 "/ethernet-phy@0");
> > > >
> > >
> > > This way we cannot attach our target that changing bootindex and take
> > effect
> > > during vm rebooting.
> > 
> > I don't understand what you mean, here. Whatever you are planning to do on
> > the
> > device-specific setter/getters, if they all look the same, you can write a
> > common getter/setter to do exactly the same steps, and register it inside
> > device_add_bootindex_property(). This way, patches 08/27 to 26/27 can
> > become
> > one-liners, instead of adding more 20 lines each.
> > 
> > I mean something like this:
> > 
> OK. Thanks for explanation. :)
> 
> I have two questions:
> 
> 1) virtio-net-pci device do need special handle in device_set_bootindex(). 
> 2) isa-fdc' property is bootindexA and bootindexB, maybe we
> can add more a parameter for device_add_bootindex_property()?

I see that you added two extra parameters to
device_add_bootindex_property(). Looks like a good solution, to me.

-- 
Eduardo