| Message ID | 87sixagysq.fsf@blackfin.pond.sub.org |
|---|---|
| State | New |
| Headers | show |
On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > > > Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > >> Qemu is expected to quit if the same boot index value is used by two devices. > >> However, hot-plugging a device with a bootindex value already used should > >> fail with a friendly message rather than quitting a running VM. > > > > I think the problem is right where QEMU exits, i.e. in > > add_boot_device_path. This function should return an error instead, via > > an Error ** argument. > > Agree. > > > Callers, typically a device's init or realize function, will either > > print the error before returning an error code (e.g. -EBUSY for init) or > > propagate the error up (for realize). > > > > Returning/propagating failure will still cause QEMU to exit when the > > duplicate bootindexes are found on the command line. > > I have an unfinished patch in my tree that does exactly that. It's > unfinished, because cleanup on error paths needs work. Current state > appended with FIXMEs and all. Beware, the FIXMEs may not be correct and > are almost certainly not complete. Thanks Markus, Should I use it as my starting point and finish it or you intend to? Marcel > > > diff --git a/hw/block/fdc.c b/hw/block/fdc.c > index c5a6c21..f8b2b27 100644 > --- a/hw/block/fdc.c > +++ b/hw/block/fdc.c > @@ -2147,8 +2147,16 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) > return; > } > > - add_boot_device_path(isa->bootindexA, dev, "/floppy@0"); > - add_boot_device_path(isa->bootindexB, dev, "/floppy@1"); > + add_boot_device_path_err(isa->bootindexA, dev, "/floppy@0", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + add_boot_device_path_err(isa->bootindexB, dev, "/floppy@1", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > } > > static void sysbus_fdc_initfn(Object *obj) > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index e2f55cc..de7ca31 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -678,6 +678,10 @@ static int virtio_blk_device_init(VirtIODevice *vdev) > return -1; > } > > + if (add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0") < 0) { > + return -1; > + } > + > virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, > sizeof(struct virtio_blk_config)); > > @@ -691,6 +695,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev) > #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) { > virtio_cleanup(vdev); > + /* FIXME rm_boot_device_path() */ > return -1; > } > s->migration_state_notifier.notify = virtio_blk_migration_state_changed; > @@ -705,7 +710,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev) > > bdrv_iostatus_enable(s->bs); > > - add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0"); > return 0; > } > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 7b3d3ee..c9568f5 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -687,12 +687,16 @@ int rom_add_file(const char *file, const char *fw_dir, > snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr); > } > > - add_boot_device_path(bootindex, NULL, devpath); > + if (add_boot_device_path(bootindex, NULL, devpath) < 0) { > + goto err_closed; > + /* FIXME undo rom_insert()? */ > + } > return 0; > > err: > if (fd != -1) > close(fd); > +err_closed: > g_free(rom->data); > g_free(rom->path); > g_free(rom->name); > diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c > index 011764f..d532b21 100644 > --- a/hw/i386/kvm/pci-assign.c > +++ b/hw/i386/kvm/pci-assign.c > @@ -1813,9 +1813,7 @@ 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; > + return add_boot_device_path(dev->bootindex, &pci_dev->qdev, NULL); > > assigned_out: > deassign_device(dev); > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c > index 18c4b7e..557be55 100644 > --- a/hw/ide/qdev.c > +++ b/hw/ide/qdev.c > @@ -166,10 +166,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) > return -1; > } > > + if (add_boot_device_path(dev->conf.bootindex, &dev->qdev, > + dev->unit ? "/disk@1" : "/disk@0") < 0) { > + return -1; > + } > + > if (ide_init_drive(s, dev->conf.bs, kind, > dev->version, dev->serial, dev->model, dev->wwn, > dev->conf.cyls, dev->conf.heads, dev->conf.secs, > dev->chs_trans) < 0) { > + /* FIXME rm_boot_device_path() */ > return -1; > } > > @@ -180,9 +186,6 @@ 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; > } > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index a1c08fb..9c064ce 100644 > --- a/hw/misc/vfio.c > +++ b/hw/misc/vfio.c > @@ -3185,7 +3185,10 @@ static int vfio_initfn(PCIDevice *pdev) > } > } > > - add_boot_device_path(vdev->bootindex, &pdev->qdev, NULL); > + if (add_boot_device_path(vdev->bootindex, &pdev->qdev, NULL) < 0) { > + // FIXME cleanup > + goto out_teardown; > + } > vfio_register_err_notifier(vdev); > > return 0; > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index d3f274c..ede2a35 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -1490,7 +1490,10 @@ 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"); > + if (add_boot_device_path(d->conf.bootindex, dev, "/ethernet-phy@0") < 0) { > + // FIXME cleanup > + return -1; > + } > > 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); > diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c > index ffa60d5..3cb986e 100644 > --- a/hw/net/eepro100.c > +++ b/hw/net/eepro100.c > @@ -1905,9 +1905,8 @@ 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; > + return add_boot_device_path(s->conf.bootindex, &pci_dev->qdev, > + "/ethernet-phy@0"); > } > > static E100PCIDeviceInfo e100_devices[] = { > diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c > index c961258..5541f2f 100644 > --- a/hw/net/ne2000.c > +++ b/hw/net/ne2000.c > @@ -740,9 +740,8 @@ 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; > + return add_boot_device_path(s->c.bootindex, &pci_dev->qdev, > + "/ethernet-phy@0"); > } > > static void pci_ne2000_exit(PCIDevice *pci_dev) > diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c > index 7cb47b3..66cac7c 100644 > --- a/hw/net/pcnet.c > +++ b/hw/net/pcnet.c > @@ -1737,7 +1737,9 @@ 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"); > + if (add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0") < 0) { > + return -1; > + } > > /* Initialize the PROM */ > > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > index c31199f..9e5b648 100644 > --- a/hw/net/rtl8139.c > +++ b/hw/net/rtl8139.c > @@ -3538,9 +3538,7 @@ 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; > + return add_boot_device_path(s->conf.bootindex, d, "/ethernet-phy@0"); > } > > static Property rtl8139_properties[] = { > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index dd41008..020a8c1 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1564,8 +1564,7 @@ static int virtio_net_device_init(VirtIODevice *vdev) > register_savevm(qdev, "virtio-net", -1, VIRTIO_NET_VM_VERSION, > virtio_net_save, virtio_net_load, n); > > - add_boot_device_path(n->nic_conf.bootindex, qdev, "/ethernet-phy@0"); > - return 0; > + return add_boot_device_path(n->nic_conf.bootindex, qdev, "/ethernet-phy@0"); > } > > static int virtio_net_device_exit(DeviceState *qdev) > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 49c2466..1da4c7b 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -2106,9 +2106,7 @@ 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; > + return add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0"); > } > > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index 74e6a14..3450782 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -2120,8 +2120,7 @@ static int scsi_initfn(SCSIDevice *dev) > bdrv_set_buffer_alignment(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); > - return 0; > + return add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, NULL); > } > > static int scsi_hd_initfn(SCSIDevice *dev) > diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c > index 8f195be..2e830e3 100644 > --- a/hw/scsi/scsi-generic.c > +++ b/hw/scsi/scsi-generic.c > @@ -433,7 +433,9 @@ static int scsi_generic_initfn(SCSIDevice *s) > 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); > + if (add_boot_device_path(s->conf.bootindex, &s->qdev, NULL) < 0) { > + return -1; > + } > } > > switch (s->type) { > diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c > index 660d774..07d75af 100644 > --- a/hw/usb/dev-network.c > +++ b/hw/usb/dev-network.c > @@ -1373,8 +1373,7 @@ 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; > + return add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0"); > } > > static USBDevice *usb_net_init(USBBus *bus, const char *cmdline) > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > index 128955d..5724bb5 100644 > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c > @@ -904,7 +904,9 @@ 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); > + if (add_boot_device_path(s->bootindex, &udev->qdev, NULL) < 0) { > + return -1; > + } > usb_host_auto_check(NULL); > return 0; > } > diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c > index 65cd3b4..c7a7bd0 100644 > --- a/hw/usb/host-linux.c > +++ b/hw/usb/host-linux.c > @@ -1488,8 +1488,7 @@ static int usb_host_initfn(USBDevice *dev) > if (s->match.bus_num != 0 && s->match.port != NULL) { > usb_host_claim_port(s); > } > - add_boot_device_path(s->bootindex, &dev->qdev, NULL); > - return 0; > + return add_boot_device_path(s->bootindex, &dev->qdev, NULL); > } > > static const VMStateDescription vmstate_usb_host = { > diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c > index 287a505..b4d01f6 100644 > --- a/hw/usb/redirect.c > +++ b/hw/usb/redirect.c > @@ -1314,7 +1314,9 @@ 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); > + if (add_boot_device_path(dev->bootindex, &udev->qdev, NULL) < 0) { > + return -1; > + } > return 0; > } > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index b1aa059..b033d97 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -178,8 +178,10 @@ void usb_info(Monitor *mon, const QDict *qdict); > > void rtc_change_mon_event(struct tm *tm); > > -void add_boot_device_path(int32_t bootindex, DeviceState *dev, > - const char *suffix); > +void add_boot_device_path_err(int32_t bootindex, DeviceState *dev, > + const char *suffix, Error **errp); > +int add_boot_device_path(int32_t bootindex, DeviceState *dev, > + const char *suffix) QEMU_WARN_UNUSED_RESULT; > char *get_boot_devices_list(size_t *size); > > DeviceState *get_boot_device(uint32_t position); > diff --git a/vl.c b/vl.c > index 4e709d5..18f9297 100644 > --- a/vl.c > +++ b/vl.c > @@ -1158,8 +1158,8 @@ static void restore_boot_order(void *opaque) > g_free(normal_boot_order); > } > > -void add_boot_device_path(int32_t bootindex, DeviceState *dev, > - const char *suffix) > +void add_boot_device_path_err(int32_t bootindex, DeviceState *dev, > + const char *suffix, Error **errp) > { > FWBootEntry *node, *i; > > @@ -1176,8 +1176,9 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev, > > QTAILQ_FOREACH(i, &fw_boot_order, link) { > if (i->bootindex == bootindex) { > - fprintf(stderr, "Two devices with same boot index %d\n", bootindex); > - exit(1); > + error_setg(errp, "Two devices with same boot index %d", > + bootindex); > + return; > } else if (i->bootindex < bootindex) { > continue; > } > @@ -1187,6 +1188,20 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev, > QTAILQ_INSERT_TAIL(&fw_boot_order, node, link); > } > > +int add_boot_device_path(int32_t bootindex, DeviceState *dev, > + const char *suffix) > +{ > + Error *local_err = NULL; > + > + add_boot_device_path_err(bootindex, dev, suffix, &local_err); > + if (local_err) { > + qerror_report_err(local_err); > + error_free(local_err); > + return -1; > + } > + return 0; > +} > + > DeviceState *get_boot_device(uint32_t position) > { > uint32_t counter = 0;
Marcel Apfelbaum <marcel.a@redhat.com> writes: > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >> > Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: >> >> Qemu is expected to quit if the same boot index value is used by >> >> two devices. >> >> However, hot-plugging a device with a bootindex value already used should >> >> fail with a friendly message rather than quitting a running VM. >> > >> > I think the problem is right where QEMU exits, i.e. in >> > add_boot_device_path. This function should return an error instead, via >> > an Error ** argument. >> >> Agree. >> >> > Callers, typically a device's init or realize function, will either >> > print the error before returning an error code (e.g. -EBUSY for init) or >> > propagate the error up (for realize). >> > >> > Returning/propagating failure will still cause QEMU to exit when the >> > duplicate bootindexes are found on the command line. >> >> I have an unfinished patch in my tree that does exactly that. It's >> unfinished, because cleanup on error paths needs work. Current state >> appended with FIXMEs and all. Beware, the FIXMEs may not be correct and >> are almost certainly not complete. > Thanks Markus, > Should I use it as my starting point and finish it or you intend to? If you have cycles to spare, you're quite welcome to take this patch and run with it! You may have noticed that my patch moves the code to add the boot device path in a few cases. I did this in the hope of simplifying error paths. Do not hesitate to undo such moves where they turn out not to simplify anything. Here's an issue that exists before my patch: DeviceClass method unrealize() should clean up everything done by realize(). In particular, unrealize() needs to remove any entry added to fw_boot_order by realize() via add_boot_device_path(). Code to do that doesn't exist, yet. Hot unplug is technically broken for all devices with bootindex. Impact unknown. Should probably be fixed in a separate patch.
On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > Marcel Apfelbaum <marcel.a@redhat.com> writes: > > > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >> Paolo Bonzini <pbonzini@redhat.com> writes: > >> > >> > Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > >> >> Qemu is expected to quit if the same boot index value is used by > >> >> two devices. > >> >> However, hot-plugging a device with a bootindex value already used should > >> >> fail with a friendly message rather than quitting a running VM. > >> > > >> > I think the problem is right where QEMU exits, i.e. in > >> > add_boot_device_path. This function should return an error instead, via > >> > an Error ** argument. > >> > >> Agree. > >> > >> > Callers, typically a device's init or realize function, will either > >> > print the error before returning an error code (e.g. -EBUSY for init) or > >> > propagate the error up (for realize). > >> > > >> > Returning/propagating failure will still cause QEMU to exit when the > >> > duplicate bootindexes are found on the command line. > >> > >> I have an unfinished patch in my tree that does exactly that. It's > >> unfinished, because cleanup on error paths needs work. Current state > >> appended with FIXMEs and all. Beware, the FIXMEs may not be correct and > >> are almost certainly not complete. > > Thanks Markus, > > Should I use it as my starting point and finish it or you intend to? > > If you have cycles to spare, you're quite welcome to take this patch and > run with it! I'll try to finish it, thanks! > > You may have noticed that my patch moves the code to add the boot device > path in a few cases. I did this in the hope of simplifying error paths. > Do not hesitate to undo such moves where they turn out not to simplify > anything. > > Here's an issue that exists before my patch: DeviceClass method > unrealize() should clean up everything done by realize(). In > particular, unrealize() needs to remove any entry added to fw_boot_order > by realize() via add_boot_device_path(). Code to do that doesn't exist, > yet. Hot unplug is technically broken for all devices with bootindex. > Impact unknown. Should probably be fixed in a separate patch. The outcome is that after hot-unplugging a device with bootindex=x, the device still remains in the fw_boot_order and no device can be hot-plugged with the same bootindex 'x'. Furthermore, because of the current issue Qemu quits on an operation that should succeed! I plan to send a patch to fix this too. Thanks, Marcel
On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > Marcel Apfelbaum <marcel.a@redhat.com> writes: > > > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >> Paolo Bonzini <pbonzini@redhat.com> writes: > >> > >> > Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > >> >> Qemu is expected to quit if the same boot index value is used by > >> >> two devices. > >> >> However, hot-plugging a device with a bootindex value already used should > >> >> fail with a friendly message rather than quitting a running VM. > >> > > >> > I think the problem is right where QEMU exits, i.e. in > >> > add_boot_device_path. This function should return an error instead, via > >> > an Error ** argument. > >> > >> Agree. I understood that the boot order is passed in fw cfg and updated only once at "machine done". There is no update of this list after this point. Modifying the boot order from monitor does not work at all. So in order to solve this issue we can: 1. Don't allow use of bootindex at hot-plug 2. Change the architecture so boot order changing during hot-plug will be possible. > >> > >> > Callers, typically a device's init or realize function, will either > >> > print the error before returning an error code (e.g. -EBUSY for init) or > >> > propagate the error up (for realize). > >> > > >> > Returning/propagating failure will still cause QEMU to exit when the > >> > duplicate bootindexes are found on the command line. > >> > >> I have an unfinished patch in my tree that does exactly that. It's > >> unfinished, because cleanup on error paths needs work. Current state > >> appended with FIXMEs and all. Beware, the FIXMEs may not be correct and > >> are almost certainly not complete. > > Thanks Markus, > > Should I use it as my starting point and finish it or you intend to? > > If you have cycles to spare, you're quite welcome to take this patch and > run with it! I really wanted to take this, but I think we need first to sort out the issues mentioned above and only then follow this path Thanks, Marcel > > You may have noticed that my patch moves the code to add the boot device > path in a few cases. I did this in the hope of simplifying error paths. > Do not hesitate to undo such moves where they turn out not to simplify > anything. > > Here's an issue that exists before my patch: DeviceClass method > unrealize() should clean up everything done by realize(). In > particular, unrealize() needs to remove any entry added to fw_boot_order > by realize() via add_boot_device_path(). Code to do that doesn't exist, > yet. Hot unplug is technically broken for all devices with bootindex. > Impact unknown. Should probably be fixed in a separate patch.
Il 16/09/2013 11:54, Marcel Apfelbaum ha scritto: > On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: >> Marcel Apfelbaum <marcel.a@redhat.com> writes: >> >>> On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: >>>> Paolo Bonzini <pbonzini@redhat.com> writes: >>>> >>>>> Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: >>>>>> Qemu is expected to quit if the same boot index value is used by >>>>>> two devices. >>>>>> However, hot-plugging a device with a bootindex value already used should >>>>>> fail with a friendly message rather than quitting a running VM. >>>>> >>>>> I think the problem is right where QEMU exits, i.e. in >>>>> add_boot_device_path. This function should return an error instead, via >>>>> an Error ** argument. >>>> >>>> Agree. > > I understood that the boot order is passed in fw cfg and updated only once at > "machine done". There is no update of this list after this point. > Modifying the boot order from monitor does not work at all. > > So in order to solve this issue we can: > 1. Don't allow use of bootindex at hot-plug > 2. Change the architecture so boot order changing during hot-plug will be possible. This is done relatively easily in add_boot_device_path (check the qdev_hotplug variable and return an error if it is 1). You can do it on top of Markus's patch. Paolo
On Mon, Sep 16, 2013 at 12:54:39PM +0300, Marcel Apfelbaum wrote: > On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > > Marcel Apfelbaum <marcel.a@redhat.com> writes: > > > > > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > > >> Paolo Bonzini <pbonzini@redhat.com> writes: > > >> > > >> > Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > > >> >> Qemu is expected to quit if the same boot index value is used by > > >> >> two devices. > > >> >> However, hot-plugging a device with a bootindex value already used should > > >> >> fail with a friendly message rather than quitting a running VM. > > >> > > > >> > I think the problem is right where QEMU exits, i.e. in > > >> > add_boot_device_path. This function should return an error instead, via > > >> > an Error ** argument. > > >> > > >> Agree. > > I understood that the boot order is passed in fw cfg and updated only once at > "machine done". There is no update of this list after this point. The reason it is done at his point is because when add_boot_device_path() is called dev is not fully instantiated so qdev_get_fw_dev_path() cannot be called on it yet. For hotplug we need to re-create boot device list when device is fully ready. > Modifying the boot order from monitor does not work at all. > > So in order to solve this issue we can: > 1. Don't allow use of bootindex at hot-plug I'd rather have a proper fix then workaround. BTW this will change qmp interface so a command that worked before (for some definition of "worked") will start to fail. Markus proposed to ignore bootindex clash, also simple solution and has no downside described above, but has others that we discussed. > 2. Change the architecture so boot order changing during hot-plug will be possible. > This is an easy part of the problem though. The hard part how not to exit when bootindex clash happens ans this is easy since nobody knows how well device creation errors are handled by qdev. -- Gleb.
On Mon, Sep 16, 2013 at 12:11:35PM +0200, Paolo Bonzini wrote: > Il 16/09/2013 11:54, Marcel Apfelbaum ha scritto: > > On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > >> Marcel Apfelbaum <marcel.a@redhat.com> writes: > >> > >>> On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >>>> Paolo Bonzini <pbonzini@redhat.com> writes: > >>>> > >>>>> Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > >>>>>> Qemu is expected to quit if the same boot index value is used by > >>>>>> two devices. > >>>>>> However, hot-plugging a device with a bootindex value already used should > >>>>>> fail with a friendly message rather than quitting a running VM. > >>>>> > >>>>> I think the problem is right where QEMU exits, i.e. in > >>>>> add_boot_device_path. This function should return an error instead, via > >>>>> an Error ** argument. > >>>> > >>>> Agree. > > > > I understood that the boot order is passed in fw cfg and updated only once at > > "machine done". There is no update of this list after this point. > > Modifying the boot order from monitor does not work at all. > > > > So in order to solve this issue we can: > > 1. Don't allow use of bootindex at hot-plug > > 2. Change the architecture so boot order changing during hot-plug will be possible. > > This is done relatively easily in add_boot_device_path (check the > qdev_hotplug variable and return an error if it is 1). This refers to 1) here? That's nice but it's not really all that helpful. > You can do it on top of Markus's patch. > > Paolo
Il 16/09/2013 17:14, Michael S. Tsirkin ha scritto: > On Mon, Sep 16, 2013 at 12:11:35PM +0200, Paolo Bonzini wrote: >> Il 16/09/2013 11:54, Marcel Apfelbaum ha scritto: >>> On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: >>>> Marcel Apfelbaum <marcel.a@redhat.com> writes: >>>> >>>>> On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: >>>>>> Paolo Bonzini <pbonzini@redhat.com> writes: >>>>>> >>>>>>> Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: >>>>>>>> Qemu is expected to quit if the same boot index value is used by >>>>>>>> two devices. >>>>>>>> However, hot-plugging a device with a bootindex value already used should >>>>>>>> fail with a friendly message rather than quitting a running VM. >>>>>>> >>>>>>> I think the problem is right where QEMU exits, i.e. in >>>>>>> add_boot_device_path. This function should return an error instead, via >>>>>>> an Error ** argument. >>>>>> >>>>>> Agree. >>> >>> I understood that the boot order is passed in fw cfg and updated only once at >>> "machine done". There is no update of this list after this point. >>> Modifying the boot order from monitor does not work at all. >>> >>> So in order to solve this issue we can: >>> 1. Don't allow use of bootindex at hot-plug >>> 2. Change the architecture so boot order changing during hot-plug will be possible. >> >> This is done relatively easily in add_boot_device_path (check the >> qdev_hotplug variable and return an error if it is 1). > > This refers to 1) here? That's nice but it's not really all that helpful. True, but the error propagation changes are the base for both cases, and (1) is just 4 lines of code. So this seems like a plan to me: do the error propagation changes and (1), then we can revert those four lines when (2) is ready. Paolo
On Mon, Sep 16, 2013 at 05:34:04PM +0200, Paolo Bonzini wrote: > Il 16/09/2013 17:14, Michael S. Tsirkin ha scritto: > > On Mon, Sep 16, 2013 at 12:11:35PM +0200, Paolo Bonzini wrote: > >> Il 16/09/2013 11:54, Marcel Apfelbaum ha scritto: > >>> On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > >>>> Marcel Apfelbaum <marcel.a@redhat.com> writes: > >>>> > >>>>> On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >>>>>> Paolo Bonzini <pbonzini@redhat.com> writes: > >>>>>> > >>>>>>> Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > >>>>>>>> Qemu is expected to quit if the same boot index value is used by > >>>>>>>> two devices. > >>>>>>>> However, hot-plugging a device with a bootindex value already used should > >>>>>>>> fail with a friendly message rather than quitting a running VM. > >>>>>>> > >>>>>>> I think the problem is right where QEMU exits, i.e. in > >>>>>>> add_boot_device_path. This function should return an error instead, via > >>>>>>> an Error ** argument. > >>>>>> > >>>>>> Agree. > >>> > >>> I understood that the boot order is passed in fw cfg and updated only once at > >>> "machine done". There is no update of this list after this point. > >>> Modifying the boot order from monitor does not work at all. > >>> > >>> So in order to solve this issue we can: > >>> 1. Don't allow use of bootindex at hot-plug > >>> 2. Change the architecture so boot order changing during hot-plug will be possible. > >> > >> This is done relatively easily in add_boot_device_path (check the > >> qdev_hotplug variable and return an error if it is 1). > > > > This refers to 1) here? That's nice but it's not really all that helpful. > > True, but the error propagation changes are the base for both cases, and > (1) is just 4 lines of code. So this seems like a plan to me: do the > error propagation changes and (1), then we can revert those four lines > when (2) is ready. > > Paolo I'm fine with applying error propagation, but maybe not (1). Basically if you accept incorrect code the result often is that controbutor disappears and it's never completed, others are discouraged from addressing the problem because incorrect code created maintainance issues, and a half-solved problem is a less interesting target than an unsolved one. So if a contributor basically says he does not want to address the complete problem, we should weight carefully whether we want to be stuck with half a solution for a long time. In this case, I don't think it's justified.
diff --git a/hw/block/fdc.c b/hw/block/fdc.c index c5a6c21..f8b2b27 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -2147,8 +2147,16 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) return; } - add_boot_device_path(isa->bootindexA, dev, "/floppy@0"); - add_boot_device_path(isa->bootindexB, dev, "/floppy@1"); + add_boot_device_path_err(isa->bootindexA, dev, "/floppy@0", &err); + if (err) { + error_propagate(errp, err); + return; + } + add_boot_device_path_err(isa->bootindexB, dev, "/floppy@1", &err); + if (err) { + error_propagate(errp, err); + return; + } } static void sysbus_fdc_initfn(Object *obj) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index e2f55cc..de7ca31 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -678,6 +678,10 @@ static int virtio_blk_device_init(VirtIODevice *vdev) return -1; } + if (add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0") < 0) { + return -1; + } + virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, sizeof(struct virtio_blk_config)); @@ -691,6 +695,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev) #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) { virtio_cleanup(vdev); + /* FIXME rm_boot_device_path() */ return -1; } s->migration_state_notifier.notify = virtio_blk_migration_state_changed; @@ -705,7 +710,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev) bdrv_iostatus_enable(s->bs); - add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0"); return 0; } diff --git a/hw/core/loader.c b/hw/core/loader.c index 7b3d3ee..c9568f5 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -687,12 +687,16 @@ int rom_add_file(const char *file, const char *fw_dir, snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr); } - add_boot_device_path(bootindex, NULL, devpath); + if (add_boot_device_path(bootindex, NULL, devpath) < 0) { + goto err_closed; + /* FIXME undo rom_insert()? */ + } return 0; err: if (fd != -1) close(fd); +err_closed: g_free(rom->data); g_free(rom->path); g_free(rom->name); diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 011764f..d532b21 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -1813,9 +1813,7 @@ 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; + return add_boot_device_path(dev->bootindex, &pci_dev->qdev, NULL); assigned_out: deassign_device(dev); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 18c4b7e..557be55 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -166,10 +166,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) return -1; } + if (add_boot_device_path(dev->conf.bootindex, &dev->qdev, + dev->unit ? "/disk@1" : "/disk@0") < 0) { + return -1; + } + if (ide_init_drive(s, dev->conf.bs, kind, dev->version, dev->serial, dev->model, dev->wwn, dev->conf.cyls, dev->conf.heads, dev->conf.secs, dev->chs_trans) < 0) { + /* FIXME rm_boot_device_path() */ return -1; } @@ -180,9 +186,6 @@ 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; } diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index a1c08fb..9c064ce 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -3185,7 +3185,10 @@ static int vfio_initfn(PCIDevice *pdev) } } - add_boot_device_path(vdev->bootindex, &pdev->qdev, NULL); + if (add_boot_device_path(vdev->bootindex, &pdev->qdev, NULL) < 0) { + // FIXME cleanup + goto out_teardown; + } vfio_register_err_notifier(vdev); return 0; diff --git a/hw/net/e1000.c b/hw/net/e1000.c index d3f274c..ede2a35 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -1490,7 +1490,10 @@ 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"); + if (add_boot_device_path(d->conf.bootindex, dev, "/ethernet-phy@0") < 0) { + // FIXME cleanup + return -1; + } 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); diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c index ffa60d5..3cb986e 100644 --- a/hw/net/eepro100.c +++ b/hw/net/eepro100.c @@ -1905,9 +1905,8 @@ 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; + return add_boot_device_path(s->conf.bootindex, &pci_dev->qdev, + "/ethernet-phy@0"); } static E100PCIDeviceInfo e100_devices[] = { diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c index c961258..5541f2f 100644 --- a/hw/net/ne2000.c +++ b/hw/net/ne2000.c @@ -740,9 +740,8 @@ 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; + return add_boot_device_path(s->c.bootindex, &pci_dev->qdev, + "/ethernet-phy@0"); } static void pci_ne2000_exit(PCIDevice *pci_dev) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 7cb47b3..66cac7c 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -1737,7 +1737,9 @@ 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"); + if (add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0") < 0) { + return -1; + } /* Initialize the PROM */ diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index c31199f..9e5b648 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -3538,9 +3538,7 @@ 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; + return add_boot_device_path(s->conf.bootindex, d, "/ethernet-phy@0"); } static Property rtl8139_properties[] = { diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index dd41008..020a8c1 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1564,8 +1564,7 @@ static int virtio_net_device_init(VirtIODevice *vdev) register_savevm(qdev, "virtio-net", -1, VIRTIO_NET_VM_VERSION, virtio_net_save, virtio_net_load, n); - add_boot_device_path(n->nic_conf.bootindex, qdev, "/ethernet-phy@0"); - return 0; + return add_boot_device_path(n->nic_conf.bootindex, qdev, "/ethernet-phy@0"); } static int virtio_net_device_exit(DeviceState *qdev) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 49c2466..1da4c7b 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2106,9 +2106,7 @@ 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; + return add_boot_device_path(s->conf.bootindex, dev, "/ethernet-phy@0"); } diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 74e6a14..3450782 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2120,8 +2120,7 @@ static int scsi_initfn(SCSIDevice *dev) bdrv_set_buffer_alignment(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); - return 0; + return add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, NULL); } static int scsi_hd_initfn(SCSIDevice *dev) diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 8f195be..2e830e3 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -433,7 +433,9 @@ static int scsi_generic_initfn(SCSIDevice *s) 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); + if (add_boot_device_path(s->conf.bootindex, &s->qdev, NULL) < 0) { + return -1; + } } switch (s->type) { diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c index 660d774..07d75af 100644 --- a/hw/usb/dev-network.c +++ b/hw/usb/dev-network.c @@ -1373,8 +1373,7 @@ 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; + return add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0"); } static USBDevice *usb_net_init(USBBus *bus, const char *cmdline) diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index 128955d..5724bb5 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -904,7 +904,9 @@ 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); + if (add_boot_device_path(s->bootindex, &udev->qdev, NULL) < 0) { + return -1; + } usb_host_auto_check(NULL); return 0; } diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 65cd3b4..c7a7bd0 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -1488,8 +1488,7 @@ static int usb_host_initfn(USBDevice *dev) if (s->match.bus_num != 0 && s->match.port != NULL) { usb_host_claim_port(s); } - add_boot_device_path(s->bootindex, &dev->qdev, NULL); - return 0; + return add_boot_device_path(s->bootindex, &dev->qdev, NULL); } static const VMStateDescription vmstate_usb_host = { diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 287a505..b4d01f6 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1314,7 +1314,9 @@ 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); + if (add_boot_device_path(dev->bootindex, &udev->qdev, NULL) < 0) { + return -1; + } return 0; } diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index b1aa059..b033d97 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -178,8 +178,10 @@ void usb_info(Monitor *mon, const QDict *qdict); void rtc_change_mon_event(struct tm *tm); -void add_boot_device_path(int32_t bootindex, DeviceState *dev, - const char *suffix); +void add_boot_device_path_err(int32_t bootindex, DeviceState *dev, + const char *suffix, Error **errp); +int add_boot_device_path(int32_t bootindex, DeviceState *dev, + const char *suffix) QEMU_WARN_UNUSED_RESULT; char *get_boot_devices_list(size_t *size); DeviceState *get_boot_device(uint32_t position); diff --git a/vl.c b/vl.c index 4e709d5..18f9297 100644 --- a/vl.c +++ b/vl.c @@ -1158,8 +1158,8 @@ static void restore_boot_order(void *opaque) g_free(normal_boot_order); } -void add_boot_device_path(int32_t bootindex, DeviceState *dev, - const char *suffix) +void add_boot_device_path_err(int32_t bootindex, DeviceState *dev, + const char *suffix, Error **errp) { FWBootEntry *node, *i; @@ -1176,8 +1176,9 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev, QTAILQ_FOREACH(i, &fw_boot_order, link) { if (i->bootindex == bootindex) { - fprintf(stderr, "Two devices with same boot index %d\n", bootindex); - exit(1); + error_setg(errp, "Two devices with same boot index %d", + bootindex); + return; } else if (i->bootindex < bootindex) { continue; } @@ -1187,6 +1188,20 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev, QTAILQ_INSERT_TAIL(&fw_boot_order, node, link); } +int add_boot_device_path(int32_t bootindex, DeviceState *dev, + const char *suffix) +{ + Error *local_err = NULL; + + add_boot_device_path_err(bootindex, dev, suffix, &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); + return -1; + } + return 0; +} + DeviceState *get_boot_device(uint32_t position) { uint32_t counter = 0;