[Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

This series implements a "bootonceindex" property for setting the boot
source priorities for the next boot only. In principle it is supposed
to do the same thing as the '-boot once=' argument but in a compatible
way with the 'bootindex' mechanism.

The basic idea is to have a second list that is sorted by the values
of the "bootonceindex" properties. When the boot order is requested,
the new list is returned if it is not empty and the list is cleared.
The normal bootindex list is only used when there are no devices on
the "once" list.

So far I have only added the support for a couple of devices and
lightly tested it on x86_64 system emulation.

Some questions:

  - Is there already some (reasonable) way to accomplish the same
    effect in QEMU?

  - Does this approach make sense? Any better ideas?

  - Any suggestions for better function / property names?

  - Should the series be split or squashed differently? I tried to
    make it easy to review, but...

  - Are the object life times (no dangling pointers left behind)
    and (lack of) locking correct? As far as I can see, they should
    be, but someone with more experience with this codebase may see
    something I don't...

  - If this approach is going to be merged, any volunteers for
    converting the rest of the devices? Or can the conversion be
    left to be done one by one at some later date? I can try to
    convert more of them, but testing them all is likely not going
    to be possible.

  - Any other suggestions/ideas/comments?


Janne Huttunen (6):
  Re-factor bootdevice list handling, pt1.
  Re-factor bootdevice list handling, pt2.
  Add support for "bootonceindex" property.
  Clear the boot once list after it has been used.
  Support "bootonceindex" property for virtio-net interfaces.
  Support "bootonceindex" property for SCSI disks.

 bootdevice.c             | 112 ++++++++++++++++++++++++++++++++++++-----------
 hw/net/virtio-net.c      |   3 ++
 hw/nvram/fw_cfg.c        |   2 +
 hw/ppc/spapr.c           |   2 +
 hw/s390x/ipl.c           |   2 +
 hw/scsi/scsi-bus.c       |   3 ++
 hw/virtio/virtio-pci.c   |   2 +
 include/hw/block/block.h |   1 +
 include/net/net.h        |   1 +
 include/sysemu/sysemu.h  |   4 ++
 10 files changed, 106 insertions(+), 26 deletions(-)

-- 
2.7.4

[Qemu-devel] [RFC][PATCH 1/6] Re-factor bootdevice list handling, pt1.

[Janne Huttunen]

Pass the list as a parameter instead of referencing it directly. This
is done so that a second list can be added in a later patch.

Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
 bootdevice.c | 49 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index 33e3029..a29d43b 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -31,6 +31,7 @@
 #include "hw/qdev-core.h"
 
 typedef struct FWBootEntry FWBootEntry;
+typedef QTAILQ_HEAD(, FWBootEntry) FWBootList;
 
 struct FWBootEntry {
     QTAILQ_ENTRY(FWBootEntry) link;
@@ -39,8 +40,7 @@ struct FWBootEntry {
     char *suffix;
 };
 
-static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
-    QTAILQ_HEAD_INITIALIZER(fw_boot_order);
+static FWBootList fw_boot_order = QTAILQ_HEAD_INITIALIZER(fw_boot_order);
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
 
@@ -116,12 +116,13 @@ void restore_boot_order(void *opaque)
     g_free(normal_boot_order);
 }
 
-void check_boot_index(int32_t bootindex, Error **errp)
+static void do_check_boot_index(FWBootList *bootlist, int32_t bootindex,
+                                Error **errp)
 {
     FWBootEntry *i;
 
     if (bootindex >= 0) {
-        QTAILQ_FOREACH(i, &fw_boot_order, link) {
+        QTAILQ_FOREACH(i, bootlist, link) {
             if (i->bootindex == bootindex) {
                 error_setg(errp, "The bootindex %d has already been used",
                            bootindex);
@@ -131,7 +132,13 @@ void check_boot_index(int32_t bootindex, Error **errp)
     }
 }
 
-void del_boot_device_path(DeviceState *dev, const char *suffix)
+void check_boot_index(int32_t bootindex, Error **errp)
+{
+    do_check_boot_index(&fw_boot_order, bootindex, errp);
+}
+
+static void do_del_boot_device_path(FWBootList *bootlist, DeviceState *dev,
+                                    const char *suffix)
 {
     FWBootEntry *i;
 
@@ -139,10 +146,10 @@ void del_boot_device_path(DeviceState *dev, const char *suffix)
         return;
     }
 
-    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+    QTAILQ_FOREACH(i, bootlist, link) {
         if ((!suffix || !g_strcmp0(i->suffix, suffix)) &&
              i->dev == dev) {
-            QTAILQ_REMOVE(&fw_boot_order, i, link);
+            QTAILQ_REMOVE(bootlist, i, link);
             g_free(i->suffix);
             g_free(i);
 
@@ -151,26 +158,31 @@ void del_boot_device_path(DeviceState *dev, const char *suffix)
     }
 }
 
-void add_boot_device_path(int32_t bootindex, DeviceState *dev,
-                          const char *suffix)
+void del_boot_device_path(DeviceState *dev, const char *suffix)
+{
+    do_del_boot_device_path(&fw_boot_order, dev, suffix);
+}
+
+static void do_add_boot_device_path(FWBootList *bootlist, int32_t bootindex,
+                                    DeviceState *dev, const char *suffix)
 {
     FWBootEntry *node, *i;
 
     if (bootindex < 0) {
-        del_boot_device_path(dev, suffix);
+        do_del_boot_device_path(bootlist, dev, suffix);
         return;
     }
 
     assert(dev != NULL || suffix != NULL);
 
-    del_boot_device_path(dev, suffix);
+    do_del_boot_device_path(bootlist, dev, suffix);
 
     node = g_malloc0(sizeof(FWBootEntry));
     node->bootindex = bootindex;
     node->suffix = g_strdup(suffix);
     node->dev = dev;
 
-    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+    QTAILQ_FOREACH(i, bootlist, link) {
         if (i->bootindex == bootindex) {
             error_report("Two devices with same boot index %d", bootindex);
             exit(1);
@@ -180,7 +192,13 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
         QTAILQ_INSERT_BEFORE(i, node, link);
         return;
     }
-    QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
+    QTAILQ_INSERT_TAIL(bootlist, node, link);
+}
+
+void add_boot_device_path(int32_t bootindex, DeviceState *dev,
+                          const char *suffix)
+{
+    do_add_boot_device_path(&fw_boot_order, bootindex, dev, suffix);
 }
 
 DeviceState *get_boot_device(uint32_t position)
@@ -210,11 +228,14 @@ DeviceState *get_boot_device(uint32_t position)
  */
 char *get_boot_devices_list(size_t *size, bool ignore_suffixes)
 {
+    FWBootList *bootlist;
     FWBootEntry *i;
     size_t total = 0;
     char *list = NULL;
 
-    QTAILQ_FOREACH(i, &fw_boot_order, link) {
+    bootlist = &fw_boot_order;
+
+    QTAILQ_FOREACH(i, bootlist, link) {
         char *devpath = NULL,  *suffix = NULL;
         char *bootpath;
         char *d;
-- 
2.7.4

[Qemu-devel] [RFC][PATCH 2/6] Re-factor bootdevice list handling, pt2.

[Janne Huttunen]

Store the list head into the property structure. This allows the callback
functions to access the correct list even when there are multiple lists.

Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
 bootdevice.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index a29d43b..30e98ae 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -33,6 +33,13 @@
 typedef struct FWBootEntry FWBootEntry;
 typedef QTAILQ_HEAD(, FWBootEntry) FWBootList;
 
+typedef struct {
+    FWBootList *bootlist;
+    int32_t *bootindex;
+    const char *suffix;
+    DeviceState *dev;
+} BootIndexProperty;
+
 struct FWBootEntry {
     QTAILQ_ENTRY(FWBootEntry) link;
     int32_t bootindex;
@@ -288,12 +295,6 @@ char *get_boot_devices_list(size_t *size, bool ignore_suffixes)
     return list;
 }
 
-typedef struct {
-    int32_t *bootindex;
-    const char *suffix;
-    DeviceState *dev;
-} BootIndexProperty;
-
 static void device_get_bootindex(Object *obj, Visitor *v, const char *name,
                                  void *opaque, Error **errp)
 {
@@ -313,14 +314,15 @@ static void device_set_bootindex(Object *obj, Visitor *v, const char *name,
         goto out;
     }
     /* check whether bootindex is present in fw_boot_order list  */
-    check_boot_index(boot_index, &local_err);
+    do_check_boot_index(prop->bootlist, boot_index, &local_err);
     if (local_err) {
         goto out;
     }
     /* change bootindex to a new one */
     *prop->bootindex = boot_index;
 
-    add_boot_device_path(*prop->bootindex, prop->dev, prop->suffix);
+    do_add_boot_device_path(prop->bootlist, *prop->bootindex,
+                            prop->dev, prop->suffix);
 
 out:
     error_propagate(errp, local_err);
@@ -332,17 +334,19 @@ static void property_release_bootindex(Object *obj, const char *name,
 {
     BootIndexProperty *prop = opaque;
 
-    del_boot_device_path(prop->dev, prop->suffix);
+    do_del_boot_device_path(prop->bootlist, prop->dev, prop->suffix);
     g_free(prop);
 }
 
-void device_add_bootindex_property(Object *obj, int32_t *bootindex,
-                                   const char *name, const char *suffix,
-                                   DeviceState *dev, Error **errp)
+static void do_add_bootindex_prop(FWBootList *bootlist,
+                                  Object *obj, int32_t *bootindex,
+                                  const char *name, const char *suffix,
+                                  DeviceState *dev, Error **errp)
 {
     Error *local_err = NULL;
     BootIndexProperty *prop = g_malloc0(sizeof(*prop));
 
+    prop->bootlist = bootlist;
     prop->bootindex = bootindex;
     prop->suffix = suffix;
     prop->dev = dev;
@@ -361,3 +365,11 @@ void device_add_bootindex_property(Object *obj, int32_t *bootindex,
     /* initialize devices' bootindex property to -1 */
     object_property_set_int(obj, -1, name, NULL);
 }
+
+void device_add_bootindex_property(Object *obj, int32_t *bootindex,
+                                   const char *name, const char *suffix,
+                                   DeviceState *dev, Error **errp)
+{
+    do_add_bootindex_prop(&fw_boot_order, obj, bootindex,
+                          name, suffix, dev, errp);
+}
-- 
2.7.4

[Qemu-devel] [RFC][PATCH 3/6] Add support for "bootonceindex" property.

[Janne Huttunen]

The property works just like the "bootindex" property does, but uses
a separate list. When the list of boot devices is queried, the boot
once list is consulted first. The normal list is returned only if the
boot once list is empty.

Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
 bootdevice.c            | 11 ++++++++++-
 include/sysemu/sysemu.h |  3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/bootdevice.c b/bootdevice.c
index 30e98ae..b5515f8 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -48,6 +48,7 @@ struct FWBootEntry {
 };
 
 static FWBootList fw_boot_order = QTAILQ_HEAD_INITIALIZER(fw_boot_order);
+static FWBootList fw_boot_once = QTAILQ_HEAD_INITIALIZER(fw_boot_once);
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
 
@@ -240,7 +241,7 @@ char *get_boot_devices_list(size_t *size, bool ignore_suffixes)
     size_t total = 0;
     char *list = NULL;
 
-    bootlist = &fw_boot_order;
+    bootlist = QTAILQ_EMPTY(&fw_boot_once) ? &fw_boot_order : &fw_boot_once;
 
     QTAILQ_FOREACH(i, bootlist, link) {
         char *devpath = NULL,  *suffix = NULL;
@@ -373,3 +374,11 @@ void device_add_bootindex_property(Object *obj, int32_t *bootindex,
     do_add_bootindex_prop(&fw_boot_order, obj, bootindex,
                           name, suffix, dev, errp);
 }
+
+void device_add_bootonceindex_property(Object *obj, int32_t *bootindex,
+                                       const char *name, const char *suffix,
+                                       DeviceState *dev, Error **errp)
+{
+    do_add_bootindex_prop(&fw_boot_once, obj, bootindex,
+                          name, suffix, dev, errp);
+}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 576c7ce..9acf2d9 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -214,6 +214,9 @@ void del_boot_device_path(DeviceState *dev, const char *suffix);
 void device_add_bootindex_property(Object *obj, int32_t *bootindex,
                                    const char *name, const char *suffix,
                                    DeviceState *dev, Error **errp);
+void device_add_bootonceindex_property(Object *obj, int32_t *bootindex,
+                                       const char *name, const char *suffix,
+                                       DeviceState *dev, Error **errp);
 void restore_boot_order(void *opaque);
 void validate_bootdevices(const char *devices, Error **errp);
 
-- 
2.7.4

[Qemu-devel] [RFC][PATCH 4/6] Clear the boot once list after it has been used.

[Janne Huttunen]

When the list is cleared, any attached properties are set to -1.
To do this, a reference to the property needs to be stored in
the list element.

Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
 bootdevice.c            | 26 ++++++++++++++++++++++----
 hw/nvram/fw_cfg.c       |  2 ++
 hw/ppc/spapr.c          |  2 ++
 hw/s390x/ipl.c          |  2 ++
 include/sysemu/sysemu.h |  1 +
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index b5515f8..e94d78d 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -42,6 +42,7 @@ typedef struct {
 
 struct FWBootEntry {
     QTAILQ_ENTRY(FWBootEntry) link;
+    BootIndexProperty *prop;
     int32_t bootindex;
     DeviceState *dev;
     char *suffix;
@@ -171,8 +172,10 @@ void del_boot_device_path(DeviceState *dev, const char *suffix)
     do_del_boot_device_path(&fw_boot_order, dev, suffix);
 }
 
-static void do_add_boot_device_path(FWBootList *bootlist, int32_t bootindex,
-                                    DeviceState *dev, const char *suffix)
+static void do_add_boot_device_path(FWBootList *bootlist,
+                                    BootIndexProperty *prop,
+                                    int32_t bootindex, DeviceState *dev,
+                                    const char *suffix)
 {
     FWBootEntry *node, *i;
 
@@ -186,6 +189,7 @@ static void do_add_boot_device_path(FWBootList *bootlist, int32_t bootindex,
     do_del_boot_device_path(bootlist, dev, suffix);
 
     node = g_malloc0(sizeof(FWBootEntry));
+    node->prop = prop;
     node->bootindex = bootindex;
     node->suffix = g_strdup(suffix);
     node->dev = dev;
@@ -206,7 +210,7 @@ static void do_add_boot_device_path(FWBootList *bootlist, int32_t bootindex,
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix)
 {
-    do_add_boot_device_path(&fw_boot_order, bootindex, dev, suffix);
+    do_add_boot_device_path(&fw_boot_order, NULL, bootindex, dev, suffix);
 }
 
 DeviceState *get_boot_device(uint32_t position)
@@ -227,6 +231,20 @@ DeviceState *get_boot_device(uint32_t position)
     return res;
 }
 
+void clear_boot_once_list(void)
+{
+    FWBootEntry *i, *next;
+
+    QTAILQ_FOREACH_SAFE(i, &fw_boot_once, link, next) {
+        if (i->prop) {
+            *i->prop->bootindex = -1;
+        }
+        QTAILQ_REMOVE(&fw_boot_once, i, link);
+        g_free(i->suffix);
+        g_free(i);
+    }
+}
+
 /*
  * This function returns null terminated string that consist of new line
  * separated device paths.
@@ -322,7 +340,7 @@ static void device_set_bootindex(Object *obj, Visitor *v, const char *name,
     /* change bootindex to a new one */
     *prop->bootindex = boot_index;
 
-    do_add_boot_device_path(prop->bootlist, *prop->bootindex,
+    do_add_boot_device_path(prop->bootlist, prop, *prop->bootindex,
                             prop->dev, prop->suffix);
 
 out:
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 316fca9..7a707fa 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -898,6 +898,8 @@ static void fw_cfg_machine_reset(void *opaque)
     FWCfgState *s = opaque;
     char *bootindex = get_boot_devices_list(&len, false);
 
+    clear_boot_once_list();
+
     ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
     g_free(ptr);
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e465d7a..68ff980 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -803,6 +803,8 @@ static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
     size_t cb = 0;
     char *bootlist = get_boot_devices_list(&cb, true);
 
+    clear_boot_once_list();
+
     _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
 
     _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 2e2664f..0a59058 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -253,6 +253,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
             ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3;
             return true;
         }
+
+        clear_boot_once_list();
     }
 
     return false;
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 9acf2d9..bf3e618 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -207,6 +207,7 @@ void hmp_info_usb(Monitor *mon, const QDict *qdict);
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
 char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
+void clear_boot_once_list(void);
 
 DeviceState *get_boot_device(uint32_t position);
 void check_boot_index(int32_t bootindex, Error **errp);
-- 
2.7.4

[Qemu-devel] [RFC][PATCH 5/6] Support "bootonceindex" property for virtio-net interfaces.

[Janne Huttunen]

Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
 hw/net/virtio-net.c    | 3 +++
 hw/virtio/virtio-pci.c | 2 ++
 include/net/net.h      | 1 +
 3 files changed, 6 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c321680..928b60b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2016,6 +2016,9 @@ static void virtio_net_instance_init(Object *obj)
     device_add_bootindex_property(obj, &n->nic_conf.bootindex,
                                   "bootindex", "/ethernet-phy@0",
                                   DEVICE(n), NULL);
+    device_add_bootonceindex_property(obj, &n->nic_conf.bootonceindex,
+                                      "bootonceindex", "/ethernet-phy@0",
+                                      DEVICE(n), NULL);
 }
 
 static void virtio_net_pre_save(void *opaque)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 5ce42af..abfa5bd 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2322,6 +2322,8 @@ static void virtio_net_pci_instance_init(Object *obj)
                                 TYPE_VIRTIO_NET);
     object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
                               "bootindex", &error_abort);
+    object_property_add_alias(obj, "bootonceindex", OBJECT(&dev->vdev),
+                              "bootonceindex", &error_abort);
 }
 
 static const TypeInfo virtio_net_pci_info = {
diff --git a/include/net/net.h b/include/net/net.h
index 99b28d5..de98b883 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -36,6 +36,7 @@ typedef struct NICConf {
     MACAddr macaddr;
     NICPeers peers;
     int32_t bootindex;
+    int32_t bootonceindex;
 } NICConf;
 
 #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
-- 
2.7.4

[Qemu-devel] [RFC][PATCH 6/6] Support "bootonceindex" property for SCSI disks.

[Janne Huttunen]

Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
 hw/scsi/scsi-bus.c       | 3 +++
 include/hw/block/block.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 5940cb1..26e38ab 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -2084,6 +2084,9 @@ static void scsi_dev_instance_init(Object *obj)
     device_add_bootindex_property(obj, &s->conf.bootindex,
                                   "bootindex", NULL,
                                   &s->qdev, NULL);
+    device_add_bootonceindex_property(obj, &s->conf.bootonceindex,
+                                      "bootonceindex", NULL,
+                                      &s->qdev, NULL);
 }
 
 static const TypeInfo scsi_device_type_info = {
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index df9d207..f3e76c3 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -22,6 +22,7 @@ typedef struct BlockConf {
     uint16_t min_io_size;
     uint32_t opt_io_size;
     int32_t bootindex;
+    int32_t bootonceindex;
     uint32_t discard_granularity;
     /* geometry, not all devices use this */
     uint32_t cyls, heads, secs;
-- 
2.7.4

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Gerd Hoffmann]

  Hi,

>   - Does this approach make sense? Any better ideas?

What is the use case?

Sometimes I'm wondering why we actually need the "once" thing.  Looks
like people use it for installs, but I fail to see why.  I usually
configure my guests with hard disk as bootindex=1 and install media
(cdrom or net) as bootindex=2.  In case the hard disk is blank it
automatically falls back to the second and boots the installer, and when
the installer is finished the hard disk isn't blank any more and the
just installed system gets booted ...

cheers,
  Gerd

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

> >   - Does this approach make sense? Any better ideas?
>  
> What is the use case?

The short answer: emulating real hardware.

Since real HW has this capability, there exist certain
auxiliary systems that are built on it. Having similar
semantics available in QEMU allows me to build a virtual
machine that works with these systems without modifying
them in any way.

Also, the support I'm after already almost was in QEMU
('-boot once='). It just didn't quite provide the
functionality I wanted i.e. the 'bootindex' like operation.
This patch series tries to bridge that gap. But hey, I
wasn't sure if this was useful for other people too, and
that's why the RFC.

In principle I agree with you that in some specific cases
QEMU has other means of doing things that necessarily don't
exist in real HW. Obviously systems that are built for real
HW don't use any such QEMU specific methods. Again, in some
specific cases it might be possible to get close enough
semantics without real "boot once" support and in others it
probably won't be. Note that part of the semantics I'm after
is that the "boot once" order can be set while the system is
up and running. It may or may not be strictly necessary
depending on the specific use case, but that's the way the
real HW works.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Vasiliy Tolstov]

14 Мар 2017 г. 19:58 пользователь "Gerd Hoffmann" <kraxel@redhat.com>
написал:

  Hi,

>   - Does this approach make sense? Any better ideas?

What is the use case?

Sometimes I'm wondering why we actually need the "once" thing.  Looks
like people use it for installs, but I fail to see why.  I usually
configure my guests with hard disk as bootindex=1 and install media
(cdrom or net) as bootindex=2.  In case the hard disk is blank it
automatically falls back to the second and boots the installer, and when
the installer is finished the hard disk isn't blank any more and the
just installed system gets booted ...

cheers,
  Gerd

This is useful for reinstalls. In this case hard disk already have boot
data and it can be used as boot device.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Gerd Hoffmann]

   Hi,

> The short answer: emulating real hardware.
> 
> Since real HW has this capability, there exist certain
> auxiliary systems that are built on it. Having similar
> semantics available in QEMU allows me to build a virtual
> machine that works with these systems without modifying
> them in any way.

Ok, that is reason enough.

Adding bootonceindex everywhere doesn't look like the best plan to me
though.  Possibly we can pimp up bootindex in a backward-compatible way?
Something like bootindex=<normal>[.<once>] ?

cheers,
  Gerd

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Wed, 2017-03-15 at 08:24 +0100, Gerd Hoffmann wrote:
> >
> > The short answer: emulating real hardware.
> 
> Ok, that is reason enough.
> 
> Adding bootonceindex everywhere doesn't look like the best plan to me
> though.  Possibly we can pimp up bootindex in a backward-compatible
> way?
> Something like bootindex=<normal>[.<once>] ?

That would (likely) avoid modifying all devices, but wouldn't
that make the 'bootindex' property a string (now: 'int32')
and thus change the QOM API?

I did consider making device_add_bootindex_property() to
automatically add the new property too, but since that API
is currently such that the _caller_ provides the name of the
added property, it would mean that the function would need
to generate the second name using some magic mangling rule
and that didn't seem very nice to me. Of course the API could
be modified so that the caller provides two names, but then
we are already back to modifying all relevant devices.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Gerd Hoffmann]

On Do, 2017-03-16 at 11:46 +0200, Janne Huttunen wrote:
> On Wed, 2017-03-15 at 08:24 +0100, Gerd Hoffmann wrote:
> > >
> > > The short answer: emulating real hardware.
> > 
> > Ok, that is reason enough.
> > 
> > Adding bootonceindex everywhere doesn't look like the best plan to me
> > though.  Possibly we can pimp up bootindex in a backward-compatible
> > way?
> > Something like bootindex=<normal>[.<once>] ?
> 
> That would (likely) avoid modifying all devices, but wouldn't
> that make the 'bootindex' property a string (now: 'int32')
> and thus change the QOM API?

Good point.  I was thinking about the cmd line only where it is a string
anyway.

> I did consider making device_add_bootindex_property() to
> automatically add the new property too, but since that API
> is currently such that the _caller_ provides the name of the
> added property, it would mean that the function would need
> to generate the second name using some magic mangling rule
> and that didn't seem very nice to me.

I think the only case where this is something != "bootindex" is the
floppy controller, which has bootindexA and bootindexB for the two
drives.  So name mangling doesn't look too bad to me.  Maybe we could
just add a "first-" or "once-" prefix.  But the second bootindex still
needs to be stored somewhere in the device state struct ...

> Of course the API could
> be modified so that the caller provides two names, but then
> we are already back to modifying all relevant devices.

... so I guess there isn't really some way around that.

cheers,
  Gerd

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

On 03/16/2017 04:46 AM, Janne Huttunen wrote:
> On Wed, 2017-03-15 at 08:24 +0100, Gerd Hoffmann wrote:
>>>
>>> The short answer: emulating real hardware.
>>
>> Ok, that is reason enough.
>>
>> Adding bootonceindex everywhere doesn't look like the best plan to me
>> though.  Possibly we can pimp up bootindex in a backward-compatible
>> way?
>> Something like bootindex=<normal>[.<once>] ?
> 
> That would (likely) avoid modifying all devices, but wouldn't
> that make the 'bootindex' property a string (now: 'int32')
> and thus change the QOM API?

Is there any way to make use of an alternate that supports both int and
string simultaneously?  But see also Markus' recent addendum on the
difficulties of supporting multiple types on command-line vs. through QMP:

https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg04125.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Paolo Bonzini]

On 15/03/2017 07:58, Janne Huttunen wrote:
> 
> Since real HW has this capability, there exist certain
> auxiliary systems that are built on it. Having similar
> semantics available in QEMU allows me to build a virtual
> machine that works with these systems without modifying
> them in any way.

How does real hardware do it?  I suppose you'd do it with a firmware
setup menu or something like that; would it be enough to add a way to
modify bootindex during runtime?

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 03/16/2017 04:46 AM, Janne Huttunen wrote:
>> On Wed, 2017-03-15 at 08:24 +0100, Gerd Hoffmann wrote:
>>>>
>>>> The short answer: emulating real hardware.
>>>
>>> Ok, that is reason enough.
>>>
>>> Adding bootonceindex everywhere doesn't look like the best plan to me
>>> though.  Possibly we can pimp up bootindex in a backward-compatible
>>> way?
>>> Something like bootindex=<normal>[.<once>] ?
>> 
>> That would (likely) avoid modifying all devices, but wouldn't
>> that make the 'bootindex' property a string (now: 'int32')
>> and thus change the QOM API?
>
> Is there any way to make use of an alternate that supports both int and
> string simultaneously?  But see also Markus' recent addendum on the
> difficulties of supporting multiple types on command-line vs. through QMP:
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg04125.html

Making it a string that encodes two integers would be inappropriate for
QMP.  If you need two integers, QMP wants you to put two integers in
JSON.  That means either a new member next to bootindex, or turning
bootindex into an alternate of integer and object.

Not passing judgement on whether we actually need "once".

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Tue, 2017-03-21 at 18:55 +0100, Paolo Bonzini wrote:
> 
> > Since real HW has this capability, there exist certain
> > auxiliary systems that are built on it. Having similar
> > semantics available in QEMU allows me to build a virtual
> > machine that works with these systems without modifying
> > them in any way.
>
> How does real hardware do it?  I suppose you'd do it with a firmware
> setup menu or something like that; would it be enough to add a way to
> modify bootindex during runtime?

On the real hardware the "boot once" really means *once*
i.e. it only affects the next reboot regardless of how
the next boot is triggered (reset button, power button,
software, etc.). After the next boot the normal boot
order is automatically restored.

Theoretically it should be possible to get a close
approximation of this by changing the main boot order,
waiting for the boot to happen and then restoring the
original order back. This would require having some
process that constantly monitors what QEMU is doing so
that it can notice when the boot happens and then
restore the order. I'm trying to avoid having such
a process if possible, which in this case means that
QEMU would need to restore the order by itself.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Paolo Bonzini]

----- Original Message -----
> From: "Janne Huttunen" <janne.huttunen@nokia.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>, "Gerd Hoffmann" <kraxel@redhat.com>
> Cc: qemu-devel@nongnu.org
> Sent: Wednesday, March 22, 2017 7:36:54 AM
> Subject: Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property
> 
> On Tue, 2017-03-21 at 18:55 +0100, Paolo Bonzini wrote:
> > 
> > > Since real HW has this capability, there exist certain
> > > auxiliary systems that are built on it. Having similar
> > > semantics available in QEMU allows me to build a virtual
> > > machine that works with these systems without modifying
> > > them in any way.
> >
> > How does real hardware do it?  I suppose you'd do it with a firmware
> > setup menu or something like that; would it be enough to add a way to
> > modify bootindex during runtime?
> 
> On the real hardware the "boot once" really means *once*
> i.e. it only affects the next reboot regardless of how
> the next boot is triggered (reset button, power button,
> software, etc.). After the next boot the normal boot
> order is automatically restored.

Understood---my question is how you would set up the alternate
boot order: is it something like "keep a button pressed while
turning on", or something written in NVRAM, or something else
that is completely different?

Paolo

> Theoretically it should be possible to get a close
> approximation of this by changing the main boot order,
> waiting for the boot to happen and then restoring the
> original order back. This would require having some
> process that constantly monitors what QEMU is doing so
> that it can notice when the boot happens and then
> restore the order. I'm trying to avoid having such
> a process if possible, which in this case means that
> QEMU would need to restore the order by itself.
> 
> 
> 

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Huttunen, Janne (Nokia - FI/Espoo)]

On Wed, 2017-03-22 at 04:43 -0400, Paolo Bonzini wrote:
> 
> Understood---my question is how you would set up the alternate
> boot order: is it something like "keep a button pressed while
> turning on", or something written in NVRAM, or something else
> that is completely different?

In my case the real hardware has a management processor
on the board and the temporary boot source (and also the
permanent one for that matter) for the main processor can
be set from there. Since neither the BIOS nor the management
firmware are open source, I don't know how it technically
works, but I assume there either is some shared memory
between the main BIOS and the management processor or
alternatively the BIOS talks with the management processor
with some protocol during boot to get the order.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Laszlo Ersek]

On 03/22/17 10:00, Huttunen, Janne (Nokia - FI/Espoo) wrote:
> On Wed, 2017-03-22 at 04:43 -0400, Paolo Bonzini wrote:
>>
>> Understood---my question is how you would set up the alternate
>> boot order: is it something like "keep a button pressed while
>> turning on", or something written in NVRAM, or something else
>> that is completely different?
> 
> In my case the real hardware has a management processor
> on the board and the temporary boot source (and also the
> permanent one for that matter) for the main processor can
> be set from there. Since neither the BIOS nor the management
> firmware are open source, I don't know how it technically
> works, but I assume there either is some shared memory
> between the main BIOS and the management processor or
> alternatively the BIOS talks with the management processor
> with some protocol during boot to get the order.
> 

I'm generally opposed to the proposed implementation for this feature /
use case; that is, the new "bootonceindex" device property.

(1) My somewhat hand-waving counter-argument is simply the complexity /
confusion that it introduces. See for example recent QEMU commit
c0d9f7d0bced ("docs: Add a note about mixing bootindex with "-boot
order"", 2017-02-28).

Even if the proposed solution keeps the "bootorder" fw_cfg file intact,
and firmware wouldn't have to look at other fw_cfg files -- I can
already guarantee that OVMF will not look at other fw_cfg files --, the
command line changes look undesirable to me.

(2) My more technical counter-arguments are:

(2a) Exposing this in the libvirt domain XML would be a huge pain.
AFAICS, libvirt already doesn't expose "-boot once" in the domain XML,
which is a *good* thing.

(2b) With the proposed change, "having rebooted once" becomes explicit
runtime state that is guest-controlled. As such, it would have to be
migrated. Assume that you start the guest on the source host, using both
bootindex and bootonceindex properties. Then, for migration, libvirt (or
the user, manually) starts QEMU on the target host using the same
command line. After migration, if the guest reboots on the target host,
its behavior should depend on whether said reboot is its first reboot
since launching the domain, so the fact whether it rebooted on the
source host should reach the target host.


I think you must already have a means to massage the management
processor to change the boot order, for the next boot. Are you doing
that massaging in code that runs on the main processor? If so, that
means the "guest code" is highly privileged, as it can control outside
components in order to influence the boot order.

For that, I can offer the following analogy:

- use a guest with libvirt

- whenever you want to modify the boot order from within the guest,
  ssh back out to the host, and use virsh-dumpxml (--inactive),
  the xmlstarlet utility, and virsh-define, to dump, edit, and save the
  domain XML non-interactively. Xmlstarlet is extremely versatile for
  modifying domain XMLs (or any other kinds of XMLs), and virsh-define
  explicitly supports the case when the domain is already running.

  In a normal virtualization environment, this would be a huge security
  hole, of course, but you are already manipulating the management
  processor from code that runs on the main processor. Exact same
  privilege escalation.

- whenever you want to relaunch the domain fully (i.e., restart QEMU
  with a new command line), again ssh out to the host, and start a
  process (a shell script) in the background. The script should first
  initiate a domain shutdown (virsh-shutdown), then wait for domain
  termination (virsh-qemu-monitor-event, and see the SHUTDOWN event in
  "qapi/event.json"), then start the domain (virsh start). Which is
  when the modified boot order will take effect.


Alternatively, if you are fine using OVMF (as UEFI firmware) within the
guest, to run your payload, you can try the following commands, to set
the BootNext UEFI variable & to reboot:

  efibootmgr --bootnext XXXX
  reboot

While OVMF heavily massages the UEFI boot order (based on the
"bootorder" fw_cfg file), *if* you stick with a constant set of
bootindex properties (== constant boot order setting in the libvirt
domain XML), then most of the UEFI BootXXXX variables that you get to
see in the guest *should* be stable, and the above commands should
hopefully work (no guarantees though).

Thanks
Laszlo

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Wed, 2017-03-22 at 11:51 +0100, Laszlo Ersek wrote:
> 
> I'm generally opposed to the proposed implementation for this feature
> /
> use case; that is, the new "bootonceindex" device property.
> 
> (1) My somewhat hand-waving counter-argument is simply the complexity
> /
> confusion that it introduces. See for example recent QEMU commit
> c0d9f7d0bced ("docs: Add a note about mixing bootindex with "-boot
> order"", 2017-02-28).

I agree that it may be confusing, but I was under the
(possibly false?) impression that the "-boot order" was on
its way to being deprecated. I thought supporting the same
functionality on "bootindex" -based configuration would
actually _help_ eventually getting rid of "-boot order".

Of course it can be argued that the "-boot once" was
always a misfeature and should be removed at the same time,
but as it stands I find it even more confusing that there
exists this "bootindex" mechanism but "-boot once" isn't
compatible with it and no compatible alternative seems to
exist at all.

> Even if the proposed solution keeps the "bootorder" fw_cfg file
> intact,
> and firmware wouldn't have to look at other fw_cfg files -- I can
> already guarantee that OVMF will not look at other fw_cfg files --,
> the
> command line changes look undesirable to me.

The current implementation doesn't require any firmware
changes.

> (2) My more technical counter-arguments are:
> 
> (2a) Exposing this in the libvirt domain XML would be a huge pain.
> AFAICS, libvirt already doesn't expose "-boot once" in the domain
> XML,
> which is a *good* thing.

I'm nowhere nearly qualified to comment on that so I'll leave
it to others.

> (2b) With the proposed change, "having rebooted once" becomes
> explicit
> runtime state that is guest-controlled. As such, it would have to be
> migrated. Assume that you start the guest on the source host, using
> both
> bootindex and bootonceindex properties. Then, for migration, libvirt
> (or
> the user, manually) starts QEMU on the target host using the same
> command line. After migration, if the guest reboots on the target
> host,
> its behavior should depend on whether said reboot is its first reboot
> since launching the domain, so the fact whether it rebooted on the
> source host should reach the target host.

Whether the first boot has happened or not is reflected in
the values of the "bootonceindex" properties. The current
implementation resets them back to -1 when the boot happens.
I'm not qualified to say if that is sufficient or not, but
if it isn't don't you already have problems migrating other
things? If e.g. someone sets the "bootindex" property while
the system is running, is that going to get migrated? If it
is, migrating the "bootonceindex" values the same way should
be sufficient, no?

You can probably tell that I'm not very familiar with the
migration and I must admit that I did not think about it
at all when doing the implementation, but things like this
are exactly the reason I wanted to get comments on the idea.

> I think you must already have a means to massage the management
> processor to change the boot order, for the next boot. Are you doing
> that massaging in code that runs on the main processor?

No, the changes are made externally. My setup is such that
first there is a computer that has a main processor with
an attached management processor. Then there is a second
external system that remotely monitors and controls the
first one. Part of what the second system does requires
setting a temporary boot order for the next reboot of the
first system every now and then. Many of the used components
are beyond my control (like e.g. operating systems etc.)
meaning that I either cannot modify them or doing so would
make the whole exercise moot. All this exists in real
hardware, but since there's never enough hardware available
and there are better uses for it, I tried to set up a virtual
machine that could act as a cheap substitute.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Laszlo Ersek]

On 03/22/17 14:58, Janne Huttunen wrote:
> On Wed, 2017-03-22 at 11:51 +0100, Laszlo Ersek wrote:
>>  
>> I'm generally opposed to the proposed implementation for this feature
>> /
>> use case; that is, the new "bootonceindex" device property.
>>
>> (1) My somewhat hand-waving counter-argument is simply the complexity
>> /
>> confusion that it introduces. See for example recent QEMU commit
>> c0d9f7d0bced ("docs: Add a note about mixing bootindex with "-boot
>> order"", 2017-02-28).
> 
> I agree that it may be confusing, but I was under the
> (possibly false?) impression that the "-boot order" was on
> its way to being deprecated. I thought supporting the same
> functionality on "bootindex" -based configuration would
> actually _help_ eventually getting rid of "-boot order".
> 
> Of course it can be argued that the "-boot once" was
> always a misfeature and should be removed at the same time,

Yes, that's my argument.

> but as it stands I find it even more confusing that there
> exists this "bootindex" mechanism but "-boot once" isn't
> compatible with it and no compatible alternative seems to
> exist at all.

All of the "-boot" options have guest architecture and firmware-specific
behavior (they are even documented so, in both the libvirt domain XML
docs and the QEMU manual page).

For example, "-boot order" is translated to x86 CMOS settings that are
unusable in any firmware that isn't "legacy x86 BIOS".

> 
>> Even if the proposed solution keeps the "bootorder" fw_cfg file 
>> intact, and firmware wouldn't have to look at other fw_cfg files --
>> I can already guarantee that OVMF will not look at other fw_cfg
>> files --, the command line changes look undesirable to me.
> 
> The current implementation doesn't require any firmware
> changes.
> 
>> (2) My more technical counter-arguments are:
>>
>> (2a) Exposing this in the libvirt domain XML would be a huge pain. 
>> AFAICS, libvirt already doesn't expose "-boot once" in the domain 
>> XML, which is a *good* thing.
> 
> I'm nowhere nearly qualified to comment on that so I'll leave
> it to others.
> 
>> (2b) With the proposed change, "having rebooted once" becomes 
>> explicit runtime state that is guest-controlled. As such, it would
>> have to be migrated. Assume that you start the guest on the source
>> host, using both bootindex and bootonceindex properties. Then, for
>> migration, libvirt (or the user, manually) starts QEMU on the
>> target host using the same command line. After migration, if the
>> guest reboots on the target host, its behavior should depend on
>> whether said reboot is its first reboot since launching the domain,
>> so the fact whether it rebooted on the source host should reach the
>> target host.
> 
> Whether the first boot has happened or not is reflected in
> the values of the "bootonceindex" properties. The current
> implementation resets them back to -1 when the boot happens.
> I'm not qualified to say if that is sufficient or not, but
> if it isn't don't you already have problems migrating other
> things? If e.g. someone sets the "bootindex" property while
> the system is running, is that going to get migrated?

To my knowledge, currently the bootindex properties cannot be changed
dynamically (for example with monitor commands) after they have been
specified on the QEMU command line.

And, even if some settings can be changed, the question is what agent
changes them. If the management layer (libvirt etc) changes them using
monitor commands, then libvirt itself can keep track of that state, and
then start the QEMU process on the destination  host with a suitably
modified command line.

Whereas, if it is the guest that changes device state, memory state etc,
then those things have to be part of the migration stream.

> If it
> is, migrating the "bootonceindex" values the same way should
> be sufficient, no?

To my knowledge, "bootindex" properties are not migrated now, and they
also need not to, because they never change at runtime.

> You can probably tell that I'm not very familiar with the
> migration and I must admit that I did not think about it
> at all when doing the implementation, but things like this
> are exactly the reason I wanted to get comments on the idea.
> 
>> I think you must already have a means to massage the management
>> processor to change the boot order, for the next boot. Are you doing
>> that massaging in code that runs on the main processor?
> 
> No, the changes are made externally. My setup is such that
> first there is a computer that has a main processor with
> an attached management processor. Then there is a second
> external system that remotely monitors and controls the
> first one. Part of what the second system does requires
> setting a temporary boot order for the next reboot of the
> first system every now and then.

How does this work in more detail?

If the second system sets a temporary boot order for the first system
"every now and then", which I guess entails on-demand talking to the
first system's management processor, then how can you replace that with
a static QEMU command line (with the proposed bootonceindex property)?

It suggests that you are able to start QEMU processes on demand, if for
nothing else then to set the bootonceindex properties.

> Many of the used components
> are beyond my control (like e.g. operating systems etc.)
> meaning that I either cannot modify them or doing so would
> make the whole exercise moot. All this exists in real
> hardware, but since there's never enough hardware available
> and there are better uses for it, I tried to set up a virtual
> machine that could act as a cheap substitute.

I'm not saying that the use case is without merit. My point is that this
new property would introduce obscure interactions with other components
in the virt stack around QEMU, and it could have a significant
maintenance footprint, while the feature does look niche (to me anyway).
If there is another way to achieve what you need, possibly using other
tools as well, I suggest that we research that more (which is why we
keep inquiring about your setup I guess).

Thanks
Laszlo

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Wed, 2017-03-22 at 15:36 +0100, Laszlo Ersek wrote:
> 
> To my knowledge, currently the bootindex properties cannot be changed
> dynamically (for example with monitor commands) after they have been
> specified on the QEMU command line.

Yes they can, via QMP:

{'execute': 'qom-get', 'arguments': { 'path': 'scsi0.0/child[0]',
'property': 'bootindex' }}
{"return": 10}

{'execute': 'qom-set', 'arguments': { 'path': 'scsi0.0/child[0]',
'property': 'bootindex', 'value': 11 }}
{"return": {}}

{'execute': 'qom-get', 'arguments': { 'path': 'scsi0.0/child[0]',
'property': 'bootindex' }}
{"return": 11}


I have no idea if doing so breaks something, like e.g.
migration, but it definitely can be done.

> And, even if some settings can be changed, the question is what agent
> changes them. If the management layer (libvirt etc) changes them
> using
> monitor commands, then libvirt itself can keep track of that state,
> and
> then start the QEMU process on the destination  host with a suitably
> modified command line.
> 
> Whereas, if it is the guest that changes device state, memory state
> etc,
> then those things have to be part of the migration stream.

True.

> > If it
> > is, migrating the "bootonceindex" values the same way should
> > be sufficient, no?
> To my knowledge, "bootindex" properties are not migrated now, and
> they
> also need not to, because they never change at runtime.

Like demonstrated above, they can change, but of course
there might be an assumption that they won't change
"unexpectedly".

> If the second system sets a temporary boot order for the first system
> "every now and then", which I guess entails on-demand talking to the
> first system's management processor, then how can you replace that
> with
> a static QEMU command line (with the proposed bootonceindex
> property)?

I'm not. I'm using QMP to change the index dynamically.

> I'm not saying that the use case is without merit. My point is that
> this
> new property would introduce obscure interactions with other
> components
> in the virt stack around QEMU, and it could have a significant
> maintenance footprint, while the feature does look niche (to me
> anyway).

Maybe, and it is definitely good to analyze it. Like I
already admitted, I did not think about migration at all
and there may very well have been other things I have
overlooked too.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Laszlo Ersek]

On 03/22/17 16:19, Janne Huttunen wrote:
> On Wed, 2017-03-22 at 15:36 +0100, Laszlo Ersek wrote:
>>  
>> To my knowledge, currently the bootindex properties cannot be changed
>> dynamically (for example with monitor commands) after they have been
>> specified on the QEMU command line.
> 
> Yes they can, via QMP:
> 
> {'execute': 'qom-get', 'arguments': { 'path': 'scsi0.0/child[0]',
> 'property': 'bootindex' }}
> {"return": 10}
> 
> {'execute': 'qom-set', 'arguments': { 'path': 'scsi0.0/child[0]',
> 'property': 'bootindex', 'value': 11 }}
> {"return": {}}
> 
> {'execute': 'qom-get', 'arguments': { 'path': 'scsi0.0/child[0]',
> 'property': 'bootindex' }}
> {"return": 11}
> 
> 
> I have no idea if doing so breaks something, like e.g.
> migration, but it definitely can be done.
> 
>> And, even if some settings can be changed, the question is what agent
>> changes them. If the management layer (libvirt etc) changes them
>> using
>> monitor commands, then libvirt itself can keep track of that state,
>> and
>> then start the QEMU process on the destination  host with a suitably
>> modified command line.
>>
>> Whereas, if it is the guest that changes device state, memory state
>> etc,
>> then those things have to be part of the migration stream.
> 
> True.
> 
>>> If it
>>> is, migrating the "bootonceindex" values the same way should
>>> be sufficient, no?
>> To my knowledge, "bootindex" properties are not migrated now, and
>> they
>> also need not to, because they never change at runtime.
> 
> Like demonstrated above, they can change, but of course
> there might be an assumption that they won't change
> "unexpectedly".
> 
>> If the second system sets a temporary boot order for the first system
>> "every now and then", which I guess entails on-demand talking to the
>> first system's management processor, then how can you replace that
>> with
>> a static QEMU command line (with the proposed bootonceindex
>> property)?
> 
> I'm not. I'm using QMP to change the index dynamically.

Wait, if you are already changing the "bootindex" property dynamically
(do I understand that right?), and it even takes effect after guest
reboot, then why is "bootonceindex" necessary at all?

(I apologize, I think I'm confused.)

Thanks
Laszlo

> 
>> I'm not saying that the use case is without merit. My point is that
>> this
>> new property would introduce obscure interactions with other
>> components
>> in the virt stack around QEMU, and it could have a significant
>> maintenance footprint, while the feature does look niche (to me
>> anyway).
> 
> Maybe, and it is definitely good to analyze it. Like I
> already admitted, I did not think about migration at all
> and there may very well have been other things I have
> overlooked too.
> 
> 

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Wed, 2017-03-22 at 16:29 +0100, Laszlo Ersek wrote:
> > I'm not. I'm using QMP to change the index dynamically.
> > 
> Wait, if you are already changing the "bootindex" property
> dynamically (do I understand that right?)

No, I'm not changing "bootindex" dynamically. I'm changing
"bootonceindex" dynamically. The point is that whatever
change I'm making is supposed to affect only one boot, the
next one. Since the guest can trigger reboots by itself,
I don't necessarily know when they are going to happen.

Like I said earlier, I can get very close to the semantics
I need if set the "bootindex" and get an event when the
boot happens so that I know when to reset the bootindex
back to the original value. However doing it like that
is (at least in theory) racy if the event isn't synchronous
and it requires some process that actively monitors those
events which I'm trying to avoid.

> ...and it could have a significant maintenance footprint,
> while the feature does look niche (to me anyway).

Whatever I'm currently doing is definitely a niche, but very
similar setting of a one time boot source while the system
is running is e.g. part of the IPMI standard (see "Set
System Boot Options Command" in IPMI Specification), so
the concept itself is not that much of a niche.

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Laszlo Ersek]

On 03/23/17 07:53, Janne Huttunen wrote:
> On Wed, 2017-03-22 at 16:29 +0100, Laszlo Ersek wrote:
>>> I'm not. I'm using QMP to change the index dynamically.
>>>  
>> Wait, if you are already changing the "bootindex" property
>> dynamically (do I understand that right?)
> 
> No, I'm not changing "bootindex" dynamically. I'm changing
> "bootonceindex" dynamically. The point is that whatever
> change I'm making is supposed to affect only one boot, the
> next one. Since the guest can trigger reboots by itself,
> I don't necessarily know when they are going to happen.
> 
> Like I said earlier, I can get very close to the semantics
> I need if set the "bootindex" and get an event when the
> boot happens so that I know when to reset the bootindex
> back to the original value. However doing it like that
> is (at least in theory) racy if the event isn't synchronous
> and it requires some process that actively monitors those
> events which I'm trying to avoid.
> 
>> ...and it could have a significant maintenance footprint,
>> while the feature does look niche (to me anyway).
> 
> Whatever I'm currently doing is definitely a niche, but very
> similar setting of a one time boot source while the system
> is running is e.g. part of the IPMI standard (see "Set
> System Boot Options Command" in IPMI Specification), so
> the concept itself is not that much of a niche.
> 

... Okay, I've spoken my mind on this, and have nothing more to add. I'm
still not convinced, but that doesn't mean you can't convince others.
(And you need to convince others more than me, because I'm not a QEMU
maintainer -- I just wanted to voice my opinion, since this was an RFC.)

What really matters to me though is that the "bootorder" fw_cfg file
preserve
- both its current, exclusive role, for communicating the boot order to
the firmware,
- and its syntax & semantics.

Thanks
Laszlo