[libvirt] [PATCH v1 0/7] qemu: Assign device aliases at define time
Rather big change because I had to update a lot of domain XMLs under tests/. You can find the patches at mu github too: https://github.com/zippy2/libvirt/commits/aliases All the changes to the tests are result of VIR_TEST_REGENERATE_OUTPUT=1. For more reference see: https://bugzilla.redhat.com/show_bug.cgi?id=1434451 Michal Privoznik (7): qemu_command: Drop useless alias checks qemuAssignDeviceRNGAlias: Allow index to be passed qemu: Move device alias assignment to separate functions qemu: Allow regeneration of aliases domain: Add virDomainDefAssignAliasesCallback qemu: Assign device aliases at define time conf: Format device alias into inactive XML too src/conf/domain_conf.c | 14 +- src/conf/domain_conf.h | 13 ++ src/qemu/qemu_alias.c | 225 +++++++++++++++++++-- src/qemu/qemu_alias.h | 7 +- src/qemu/qemu_command.c | 21 +- src/qemu/qemu_domain.c | 24 +++ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- .../disk_snapshot_redefine.xml | 21 +- .../external_vm_redefine.xml | 16 +- tests/domainsnapshotxml2xmlout/full_domain.xml | 16 +- tests/domainsnapshotxml2xmlout/metadata.xml | 16 +- tests/qemuargv2xmldata/qemuargv2xml-boot-cdrom.xml | 19 +- .../qemuargv2xmldata/qemuargv2xml-boot-floppy.xml | 24 ++- .../qemuargv2xmldata/qemuargv2xml-boot-network.xml | 19 +- .../qemuargv2xml-clock-localtime.xml | 19 +- tests/qemuargv2xmldata/qemuargv2xml-clock-utc.xml | 19 +- .../qemuargv2xml-console-compat.xml | 21 +- .../qemuargv2xml-disk-cdrom-empty.xml | 20 +- tests/qemuargv2xmldata/qemuargv2xml-disk-cdrom.xml | 20 +- .../qemuargv2xml-disk-drive-boot-cdrom.xml | 20 +- .../qemuargv2xml-disk-drive-boot-disk.xml | 20 +- .../qemuargv2xml-disk-drive-cache-directsync.xml | 20 +- .../qemuargv2xml-disk-drive-cache-unsafe.xml | 20 +- .../qemuargv2xml-disk-drive-cache-v2-none.xml | 20 +- .../qemuargv2xml-disk-drive-cache-v2-wb.xml | 20 +- .../qemuargv2xml-disk-drive-cache-v2-wt.xml | 20 +- ...muargv2xml-disk-drive-error-policy-enospace.xml | 20 +- .../qemuargv2xml-disk-drive-error-policy-stop.xml | 20 +- ...xml-disk-drive-error-policy-wreport-rignore.xml | 20 +- .../qemuargv2xml-disk-drive-fmt-qcow.xml | 20 +- .../qemuargv2xml-disk-drive-network-gluster.xml | 19 +- .../qemuargv2xml-disk-drive-network-iscsi-auth.xml | 19 +- .../qemuargv2xml-disk-drive-network-iscsi.xml | 19 +- .../qemuargv2xml-disk-drive-network-nbd-export.xml | 20 +- ...argv2xml-disk-drive-network-nbd-ipv6-export.xml | 20 +- .../qemuargv2xml-disk-drive-network-nbd-ipv6.xml | 20 +- .../qemuargv2xml-disk-drive-network-nbd-unix.xml | 20 +- .../qemuargv2xml-disk-drive-network-nbd.xml | 20 +- .../qemuargv2xml-disk-drive-network-rbd-auth.xml | 20 +- ...emuargv2xml-disk-drive-network-rbd-ceph-env.xml | 20 +- .../qemuargv2xml-disk-drive-network-rbd-ipv6.xml | 20 +- .../qemuargv2xml-disk-drive-network-rbd.xml | 23 ++- .../qemuargv2xml-disk-drive-network-sheepdog.xml | 20 +- .../qemuargv2xmldata/qemuargv2xml-disk-floppy.xml | 25 ++- tests/qemuargv2xmldata/qemuargv2xml-disk-many.xml | 22 +- tests/qemuargv2xmldata/qemuargv2xml-disk-usb.xml | 20 +- .../qemuargv2xmldata/qemuargv2xml-disk-virtio.xml | 22 +- .../qemuargv2xml-graphics-sdl-fullscreen.xml | 20 +- .../qemuargv2xmldata/qemuargv2xml-graphics-sdl.xml | 20 +- .../qemuargv2xml-graphics-vnc-policy.xml | 20 +- .../qemuargv2xml-graphics-vnc-sasl.xml | 20 +- .../qemuargv2xml-graphics-vnc-socket.xml | 20 +- .../qemuargv2xml-graphics-vnc-tls.xml | 20 +- .../qemuargv2xml-graphics-vnc-websocket.xml | 18 +- .../qemuargv2xmldata/qemuargv2xml-graphics-vnc.xml | 20 +- .../qemuargv2xml-hostdev-pci-address.xml | 20 +- .../qemuargv2xml-hostdev-usb-address.xml | 20 +- .../qemuargv2xmldata/qemuargv2xml-hyperv-panic.xml | 17 +- tests/qemuargv2xmldata/qemuargv2xml-hyperv.xml | 17 +- .../qemuargv2xml-input-usbmouse.xml | 23 ++- .../qemuargv2xml-input-usbtablet.xml | 23 ++- .../qemuargv2xmldata/qemuargv2xml-kvm-features.xml | 17 +- tests/qemuargv2xmldata/qemuargv2xml-kvmclock.xml | 17 +- .../qemuargv2xml-machine-aeskeywrap-off-argv.xml | 4 +- .../qemuargv2xml-machine-aeskeywrap-on-argv.xml | 4 +- .../qemuargv2xml-machine-core-off.xml | 19 +- .../qemuargv2xml-machine-core-on.xml | 19 +- .../qemuargv2xml-machine-deakeywrap-off-argv.xml | 4 +- .../qemuargv2xml-machine-deakeywrap-on-argv.xml | 4 +- .../qemuargv2xml-machine-keywrap-none-argv.xml | 4 +- tests/qemuargv2xmldata/qemuargv2xml-mem-scale.xml | 19 +- tests/qemuargv2xmldata/qemuargv2xml-migrate.xml | 19 +- tests/qemuargv2xmldata/qemuargv2xml-misc-acpi.xml | 19 +- .../qemuargv2xml-misc-disable-s3.xml | 19 +- .../qemuargv2xml-misc-disable-suspends.xml | 19 +- .../qemuargv2xml-misc-enable-s4.xml | 19 +- .../qemuargv2xml-misc-no-reboot.xml | 19 +- tests/qemuargv2xmldata/qemuargv2xml-misc-uuid.xml | 19 +- .../qemuargv2xml-net-eth-ifname.xml | 20 +- tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml | 20 +- tests/qemuargv2xmldata/qemuargv2xml-net-user.xml | 20 +- tests/qemuargv2xmldata/qemuargv2xml-net-virtio.xml | 20 +- .../qemuargv2xml-nographics-vga-display.xml | 19 +- .../qemuargv2xml-nographics-vga.xml | 19 +- .../qemuargv2xmldata/qemuargv2xml-nosharepages.xml | 19 +- .../qemuargv2xmldata/qemuargv2xml-parallel-tcp.xml | 20 +- .../qemuargv2xmldata/qemuargv2xml-pseries-disk.xml | 18 +- .../qemuargv2xml-pseries-nvram.xml | 6 +- .../qemuargv2xml-qemu-ns-no-env.xml | 19 +- .../qemuargv2xml-reboot-timeout-disabled.xml | 17 +- .../qemuargv2xml-reboot-timeout-enabled.xml | 17 +- tests/qemuargv2xmldata/qemuargv2xml-restore-v2.xml | 19 +- tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml | 21 +- .../qemuargv2xmldata/qemuargv2xml-serial-file.xml | 21 +- .../qemuargv2xmldata/qemuargv2xml-serial-many.xml | 22 +- tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml | 21 +- .../qemuargv2xml-serial-tcp-telnet.xml | 21 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml | 21 +- tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml | 22 +- .../qemuargv2xmldata/qemuargv2xml-serial-unix.xml | 21 +- tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml | 21 +- tests/qemuargv2xmldata/qemuargv2xml-smp.xml | 19 +- tests/qemuargv2xmldata/qemuargv2xml-sound.xml | 29 ++- tests/qemuargv2xmldata/qemuargv2xml-watchdog.xml | 23 ++- tests/qemuhotplugtest.c | 2 +- .../ppc64-modern-bulk-result-conf.xml | 14 +- .../ppc64-modern-individual-result-conf.xml | 14 +- .../x86-modern-bulk-result-conf.xml | 14 +- .../x86-modern-individual-add-result-conf.xml | 14 +- .../x86-old-bulk-result-conf.xml | 14 +- .../qemuxml2argv-blkdeviotune-group-num.xml | 17 +- .../qemuxml2argv-blkdeviotune-max-length.xml | 17 +- .../qemuxml2argv-blkdeviotune-max.xml | 17 +- .../qemuxml2argv-disk-drive-detect-zeroes.xml | 17 +- .../qemuxml2argv-encrypted-disk-usage.xml | 15 +- .../qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.xml | 4 + .../qemuxml2argv-hostdev-scsi-vhost-scsi-pci.xml | 18 +- .../qemuxml2argv-hugepages-memaccess.xml | 17 +- .../qemuxml2argv-hugepages-memaccess2.xml | 17 +- .../qemuxml2argv-hugepages-pages4.xml | 16 +- .../qemuxml2argv-hugepages-pages5.xml | 16 +- .../qemuxml2argv-hugepages-pages6.xml | 16 +- .../qemuxml2argv-hugepages-pages7.xml | 17 +- .../qemuxml2argv-intel-iommu-caching-mode.xml | 22 +- .../qemuxml2argv-intel-iommu-device-iotlb.xml | 17 +- .../qemuxml2argv-intel-iommu-eim.xml | 17 +- tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 16 +- .../qemuxml2argv-memory-hotplug-nvdimm-access.xml | 17 +- .../qemuxml2argv-memory-hotplug-nvdimm-label.xml | 17 +- .../qemuxml2argv-memory-hotplug-nvdimm.xml | 17 +- .../qemuxml2argv-net-user-addr.xml | 20 +- ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 19 +- .../qemuxml2argv-virtio-options.xml | 27 ++- tests/qemuxml2argvtest.c | 9 + .../qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml | 6 + .../qemuxml2xmlout-aarch64-gic-none-tcg.xml | 4 +- .../qemuxml2xmlout-aarch64-pci-serial.xml | 8 +- .../qemuxml2xmlout-aarch64-video-default.xml | 8 +- ...qemuxml2xmlout-aarch64-video-virtio-gpu-pci.xml | 8 +- .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 18 +- ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 11 +- .../qemuxml2xmlout-acpi-table.xml | 18 +- .../qemuxml2xmlout-autoindex.xml | 47 ++++- .../qemuxml2xmlout-balloon-device-auto.xml | 16 +- .../qemuxml2xmlout-balloon-device-period.xml | 16 +- .../qemuxml2xmlout-bios-nvram-os-interleave.xml | 22 +- .../qemuxml2xmlout-bios-nvram.xml | 20 +- .../qemuxml2xmlout-blkdeviotune.xml | 17 +- .../qemuxml2xmlout-blkiotune-device.xml | 16 +- .../qemuxml2xmlout-blkiotune.xml | 16 +- .../qemuxml2xmlout-boot-cdrom.xml | 19 +- .../qemuxml2xmlout-boot-floppy-q35.xml | 22 +- .../qemuxml2xmlout-boot-floppy.xml | 24 ++- ...muxml2xmlout-boot-menu-disable-with-timeout.xml | 16 +- .../qemuxml2xmlout-boot-menu-disable.xml | 16 +- ...emuxml2xmlout-boot-menu-enable-with-timeout.xml | 16 +- .../qemuxml2xmlout-boot-multi.xml | 16 +- .../qemuxml2xmlout-boot-network.xml | 19 +- .../qemuxml2xmlout-boot-order.xml | 24 ++- .../qemuxml2xmlout-bootindex-floppy-q35.xml | 22 +- .../qemuxml2xmlout-channel-guestfwd.xml | 17 +- .../qemuxml2xmlout-channel-unix-source-path.xml | 22 +- .../qemuxml2xmlout-channel-virtio-auto.xml | 25 ++- .../qemuxml2xmlout-channel-virtio-state-active.xml | 20 +- ...emuxml2xmlout-channel-virtio-state-inactive.xml | 20 +- .../qemuxml2xmlout-channel-virtio.xml | 18 +- .../qemuxml2xmlout-chardev-label.xml | 19 +- .../qemuxml2xmlout-clock-catchup.xml | 16 +- .../qemuxml2xmlout-clock-localtime.xml | 19 +- .../qemuxml2xmlout-clock-timer-hyperv-rtc.xml | 14 +- .../qemuxml2xmlout-clock-utc.xml | 19 +- .../qemuxml2xmlout-console-compat-auto.xml | 18 +- .../qemuxml2xmlout-console-compat.xml | 21 +- .../qemuxml2xmlout-console-compat2.xml | 23 ++- .../qemuxml2xmlout-console-virtio-many.xml | 22 +- .../qemuxml2xmlout-console-virtio.xml | 18 +- .../qemuxml2xmlout-controller-usb-order.xml | 16 +- .../qemuxml2xmlout-cpu-check-default-none.xml | 14 +- .../qemuxml2xmlout-cpu-check-default-none2.xml | 14 +- .../qemuxml2xmlout-cpu-check-default-partial.xml | 14 +- .../qemuxml2xmlout-cpu-check-default-partial2.xml | 14 +- .../qemuxml2xmlout-cpu-check-full.xml | 14 +- .../qemuxml2xmlout-cpu-check-none.xml | 14 +- .../qemuxml2xmlout-cpu-check-partial.xml | 14 +- .../qemuxml2xmlout-cpu-empty.xml | 14 +- .../qemuxml2xmlout-cpu-eoi-disabled.xml | 14 +- .../qemuxml2xmlout-cpu-eoi-enabled.xml | 14 +- .../qemuxml2xmlout-cpu-host-kvmclock.xml | 14 +- .../qemuxml2xmlout-cpu-host-model-features.xml | 15 +- ...emuxml2xmlout-cpu-host-passthrough-features.xml | 16 +- .../qemuxml2xmlout-cpu-kvmclock.xml | 14 +- .../qemuxml2xmlout-cpu-numa-disjoint.xml | 14 +- .../qemuxml2xmlout-cpu-numa-disordered.xml | 14 +- .../qemuxml2xmlout-cpu-numa-memshared.xml | 14 +- .../qemuxml2xmlout-cpu-numa-no-memory-element.xml | 14 +- .../qemuxml2xmlout-cpu-numa1.xml | 14 +- .../qemuxml2xmlout-cpu-numa2.xml | 14 +- .../qemuxml2xmlout-cputune-iothreads.xml | 16 +- ...l2xmlout-cputune-iothreadsched-zeropriority.xml | 16 +- .../qemuxml2xmlout-cputune-iothreadsched.xml | 16 +- .../qemuxml2xmlout-cputune-numatune.xml | 17 +- .../qemuxml2xmlout-cputune-zero-shares.xml | 16 +- .../qemuxml2xmloutdata/qemuxml2xmlout-cputune.xml | 16 +- .../qemuxml2xmlout-default-kvm-host-arch.xml | 14 +- .../qemuxml2xmlout-default-qemu-host-arch.xml | 14 +- .../qemuxml2xmlout-disk-active-commit.xml | 16 +- .../qemuxml2xmloutdata/qemuxml2xmlout-disk-aio.xml | 17 +- .../qemuxml2xmlout-disk-backing-chains-active.xml | 19 +- ...qemuxml2xmlout-disk-backing-chains-inactive.xml | 19 +- .../qemuxml2xmlout-disk-cdrom-empty.xml | 20 +- .../qemuxml2xmlout-disk-cdrom.xml | 20 +- .../qemuxml2xmlout-disk-copy_on_read.xml | 19 +- .../qemuxml2xmlout-disk-drive-boot-cdrom.xml | 20 +- .../qemuxml2xmlout-disk-drive-boot-disk.xml | 20 +- .../qemuxml2xmlout-disk-drive-cache-directsync.xml | 20 +- .../qemuxml2xmlout-disk-drive-cache-unsafe.xml | 20 +- .../qemuxml2xmlout-disk-drive-cache-v2-none.xml | 20 +- .../qemuxml2xmlout-disk-drive-cache-v2-wb.xml | 20 +- .../qemuxml2xmlout-disk-drive-cache-v2-wt.xml | 20 +- .../qemuxml2xmlout-disk-drive-copy-on-read.xml | 16 +- .../qemuxml2xmlout-disk-drive-discard.xml | 17 +- ...xml2xmlout-disk-drive-error-policy-enospace.xml | 20 +- ...qemuxml2xmlout-disk-drive-error-policy-stop.xml | 20 +- ...out-disk-drive-error-policy-wreport-rignore.xml | 20 +- .../qemuxml2xmlout-disk-drive-fmt-qcow.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-gluster.xml | 20 +- ...emuxml2xmlout-disk-drive-network-iscsi-auth.xml | 19 +- .../qemuxml2xmlout-disk-drive-network-iscsi.xml | 19 +- ...emuxml2xmlout-disk-drive-network-nbd-export.xml | 20 +- ...l2xmlout-disk-drive-network-nbd-ipv6-export.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-nbd-ipv6.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-nbd-unix.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-nbd.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-rbd-auth.xml | 20 +- ...uxml2xmlout-disk-drive-network-rbd-ceph-env.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-rbd-ipv6.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-rbd.xml | 23 ++- .../qemuxml2xmlout-disk-drive-network-sheepdog.xml | 20 +- .../qemuxml2xmlout-disk-drive-network-vxhs.xml | 18 +- .../qemuxml2xmlout-disk-floppy.xml | 25 ++- .../qemuxml2xmlout-disk-many.xml | 22 +- .../qemuxml2xmlout-disk-mirror-active.xml | 19 +- .../qemuxml2xmlout-disk-mirror-inactive.xml | 19 +- .../qemuxml2xmlout-disk-mirror-old-inactive.xml | 19 +- .../qemuxml2xmlout-disk-mirror-old.xml | 19 +- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 18 +- .../qemuxml2xmlout-disk-scsi-device.xml | 18 +- .../qemuxml2xmlout-disk-scsi-disk-vpd.xml | 18 +- ...muxml2xmlout-disk-scsi-lun-passthrough-sgio.xml | 18 +- .../qemuxml2xmlout-disk-scsi-megasas.xml | 18 +- .../qemuxml2xmlout-disk-scsi-mptsas1068.xml | 18 +- .../qemuxml2xmlout-disk-scsi-virtio-scsi.xml | 18 +- .../qemuxml2xmlout-disk-scsi-vscsi.xml | 18 +- .../qemuxml2xmlout-disk-source-pool-mode.xml | 19 +- .../qemuxml2xmlout-disk-source-pool.xml | 18 +- .../qemuxml2xmlout-disk-usb-device.xml | 17 +- ...qemuxml2xmlout-disk-virtio-scsi-cmd_per_lun.xml | 16 +- .../qemuxml2xmlout-disk-virtio-scsi-ioeventfd.xml | 16 +- ...qemuxml2xmlout-disk-virtio-scsi-max_sectors.xml | 16 +- .../qemuxml2xmlout-disk-virtio-scsi-num_queues.xml | 16 +- .../qemuxml2xmlout-disk-virtio.xml | 22 +- .../qemuxml2xmlout-encrypted-disk.xml | 15 +- .../qemuxml2xmlout-eoi-disabled.xml | 14 +- .../qemuxml2xmlout-eoi-enabled.xml | 14 +- .../qemuxml2xmlout-event_idx.xml | 19 +- .../qemuxml2xmlout-floppy-drive-fat.xml | 20 +- .../qemuxml2xmlout-graphics-listen-network.xml | 17 +- .../qemuxml2xmlout-graphics-listen-network2.xml | 17 +- .../qemuxml2xmlout-graphics-sdl-fullscreen.xml | 20 +- .../qemuxml2xmlout-graphics-sdl.xml | 20 +- ...muxml2xmlout-graphics-spice-auto-socket-cfg.xml | 19 +- .../qemuxml2xmlout-graphics-spice-auto-socket.xml | 19 +- .../qemuxml2xmlout-graphics-spice-compression.xml | 18 +- .../qemuxml2xmlout-graphics-spice-qxl-vga.xml | 18 +- .../qemuxml2xmlout-graphics-spice-socket.xml | 19 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 25 ++- .../qemuxml2xmlout-graphics-spice.xml | 18 +- ...qemuxml2xmlout-graphics-vnc-auto-socket-cfg.xml | 20 +- .../qemuxml2xmlout-graphics-vnc-auto-socket.xml | 19 +- .../qemuxml2xmlout-graphics-vnc-no-listen-attr.xml | 20 +- ...graphics-vnc-remove-generated-socket-active.xml | 20 +- ...aphics-vnc-remove-generated-socket-inactive.xml | 20 +- .../qemuxml2xmlout-graphics-vnc-sasl.xml | 20 +- .../qemuxml2xmlout-graphics-vnc-socket.xml | 19 +- .../qemuxml2xmlout-graphics-vnc-tls.xml | 20 +- .../qemuxml2xmlout-graphics-vnc-websocket.xml | 18 +- .../qemuxml2xmlout-graphics-vnc.xml | 20 +- .../qemuxml2xmlout-hostdev-mdev-precreated.xml | 20 +- .../qemuxml2xmlout-hostdev-pci-address.xml | 20 +- ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 29 ++- .../qemuxml2xmlout-hostdev-scsi-large-unit.xml | 18 +- .../qemuxml2xmlout-hostdev-scsi-lsi-iscsi-auth.xml | 19 +- .../qemuxml2xmlout-hostdev-scsi-lsi-iscsi.xml | 19 +- .../qemuxml2xmlout-hostdev-scsi-lsi.xml | 18 +- .../qemuxml2xmlout-hostdev-scsi-rawio.xml | 18 +- .../qemuxml2xmlout-hostdev-scsi-readonly.xml | 18 +- .../qemuxml2xmlout-hostdev-scsi-sgio.xml | 18 +- .../qemuxml2xmlout-hostdev-scsi-shareable.xml | 18 +- ...muxml2xmlout-hostdev-scsi-virtio-iscsi-auth.xml | 19 +- .../qemuxml2xmlout-hostdev-scsi-virtio-iscsi.xml | 19 +- .../qemuxml2xmlout-hostdev-scsi-virtio-scsi.xml | 18 +- .../qemuxml2xmlout-hostdev-usb-address.xml | 20 +- .../qemuxml2xmlout-hostdev-vfio.xml | 17 +- .../qemuxml2xmlout-hugepages-pages.xml | 16 +- .../qemuxml2xmlout-hugepages-pages2.xml | 16 +- .../qemuxml2xmlout-hugepages-pages3.xml | 16 +- .../qemuxml2xmlout-hugepages-shared.xml | 16 +- .../qemuxml2xmlout-hugepages.xml | 16 +- .../qemuxml2xmlout-hyperv-off.xml | 14 +- .../qemuxml2xmlout-hyperv-panic.xml | 17 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv.xml | 17 +- .../qemuxml2xmlout-input-usbmouse.xml | 23 ++- .../qemuxml2xmlout-input-usbtablet.xml | 23 ++- .../qemuxml2xmlout-intel-iommu-machine.xml | 17 +- .../qemuxml2xmlout-intel-iommu.xml | 17 +- .../qemuxml2xmlout-interface-driver.xml | 21 +- .../qemuxml2xmlout-interface-server.xml | 33 ++- .../qemuxml2xmlout-iothreads-disk-virtio-ccw.xml | 7 +- .../qemuxml2xmlout-iothreads-disk.xml | 21 +- .../qemuxml2xmlout-iothreads-ids-partial.xml | 19 +- .../qemuxml2xmlout-iothreads-ids.xml | 19 +- .../qemuxml2xmlout-iothreads-virtio-scsi-ccw.xml | 8 +- .../qemuxml2xmlout-iothreads-virtio-scsi-pci.xml | 22 +- .../qemuxml2xmlout-iothreads.xml | 19 +- .../qemuxml2xmlout-kvm-features-off.xml | 14 +- .../qemuxml2xmlout-kvm-features.xml | 17 +- .../qemuxml2xmloutdata/qemuxml2xmlout-kvmclock.xml | 17 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-lease.xml | 17 +- .../qemuxml2xmlout-machine-core-off.xml | 19 +- .../qemuxml2xmlout-machine-core-on.xml | 19 +- ...t-machine-loadparm-multiple-disks-nets-s390.xml | 5 + .../qemuxml2xmlout-memory-hotplug-dimm.xml | 18 +- .../qemuxml2xmlout-memory-hotplug-nonuma.xml | 14 +- .../qemuxml2xmlout-memory-hotplug.xml | 16 +- .../qemuxml2xmlout-memorybacking-set.xml | 14 +- .../qemuxml2xmlout-memorybacking-unset.xml | 14 +- .../qemuxml2xmlout-memtune-unlimited.xml | 16 +- .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml | 16 +- .../qemuxml2xmlout-metadata-duplicate.xml | 16 +- .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 16 +- .../qemuxml2xmloutdata/qemuxml2xmlout-migrate.xml | 19 +- .../qemuxml2xmloutdata/qemuxml2xmlout-minimal.xml | 16 +- .../qemuxml2xmlout-misc-acpi.xml | 19 +- .../qemuxml2xmlout-misc-disable-s3.xml | 19 +- .../qemuxml2xmlout-misc-disable-suspends.xml | 19 +- .../qemuxml2xmlout-misc-enable-s4.xml | 19 +- .../qemuxml2xmlout-misc-no-reboot.xml | 19 +- .../qemuxml2xmlout-misc-uuid.xml | 19 +- .../qemuxml2xmlout-net-bandwidth.xml | 27 ++- .../qemuxml2xmlout-net-bandwidth2.xml | 25 ++- .../qemuxml2xmlout-net-coalesce.xml | 22 +- .../qemuxml2xmlout-net-eth-hostip.xml | 20 +- .../qemuxml2xmlout-net-eth-ifname.xml | 20 +- .../qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml | 20 +- .../qemuxml2xmlout-net-hostdev-vfio.xml | 17 +- .../qemuxml2xmlout-net-hostdev.xml | 17 +- .../qemuxml2xmlout-net-midonet.xml | 17 +- .../qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml | 22 +- .../qemuxml2xmlout-net-openvswitch.xml | 17 +- .../qemuxml2xmloutdata/qemuxml2xmlout-net-udp.xml | 17 +- .../qemuxml2xmloutdata/qemuxml2xmlout-net-user.xml | 20 +- .../qemuxml2xmlout-net-vhostuser.xml | 22 +- .../qemuxml2xmlout-net-virtio-device.xml | 17 +- .../qemuxml2xmlout-net-virtio-disable-offloads.xml | 17 +- ...qemuxml2xmlout-net-virtio-network-portgroup.xml | 19 +- .../qemuxml2xmlout-net-virtio-rxtxqueuesize.xml | 17 +- .../qemuxml2xmlout-net-virtio.xml | 20 +- .../qemuxml2xmlout-nographics-vga.xml | 19 +- .../qemuxml2xmlout-nosharepages.xml | 19 +- ...emuxml2xmlout-numad-auto-memory-vcpu-cpuset.xml | 16 +- ...ad-auto-memory-vcpu-no-cpuset-and-placement.xml | 16 +- .../qemuxml2xmlout-numad-auto-vcpu-no-numatune.xml | 16 +- ...emuxml2xmlout-numad-static-vcpu-no-numatune.xml | 16 +- .../qemuxml2xmlout-numatune-auto-prefer.xml | 14 +- .../qemuxml2xmlout-numatune-memnode-no-memory.xml | 14 +- .../qemuxml2xmlout-numatune-memnode.xml | 14 +- .../qemuxml2xmlout-panic-double.xml | 17 +- .../qemuxml2xmlout-panic-isa.xml | 20 +- .../qemuxml2xmlout-panic-no-address.xml | 20 +- .../qemuxml2xmlout-panic-pseries.xml | 8 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml | 20 +- .../qemuxml2xmlout-pci-autoadd-addr.xml | 24 ++- .../qemuxml2xmlout-pci-autoadd-idx.xml | 25 ++- .../qemuxml2xmlout-pci-autofill-addr.xml | 17 +- .../qemuxml2xmlout-pci-bridge-many-disks.xml | 125 +++++++++++- .../qemuxml2xmlout-pci-bridge.xml | 51 ++++- .../qemuxml2xmlout-pci-expander-bus.xml | 52 ++++- .../qemuxml2xmloutdata/qemuxml2xmlout-pci-many.xml | 45 ++++- .../qemuxml2xmloutdata/qemuxml2xmlout-pci-rom.xml | 20 +- .../qemuxml2xmlout-pci-serial-dev-chardev.xml | 18 +- .../qemuxml2xmlout-pcie-expander-bus.xml | 85 +++++++- ...qemuxml2xmlout-pcie-root-port-model-generic.xml | 20 +- ...qemuxml2xmlout-pcie-root-port-model-ioh3420.xml | 19 +- .../qemuxml2xmlout-pcie-root-port.xml | 21 +- .../qemuxml2xmlout-pcie-root.xml | 17 +- .../qemuxml2xmlout-pcie-switch-downstream-port.xml | 29 ++- .../qemuxml2xmlout-pcie-switch-upstream-port.xml | 23 ++- .../qemuxml2xmlout-pcihole64-gib.xml | 11 +- .../qemuxml2xmlout-pcihole64-none.xml | 11 +- .../qemuxml2xmlout-pcihole64-q35.xml | 16 +- .../qemuxml2xmlout-pcihole64.xml | 11 +- .../qemuxml2xmlout-pmu-feature-off.xml | 17 +- .../qemuxml2xmlout-pmu-feature.xml | 17 +- .../qemuxml2xmlout-ppc64-usb-controller-legacy.xml | 3 + .../qemuxml2xmlout-ppc64-usb-controller.xml | 3 + .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 8 +- .../qemuxml2xmlout-pseries-cpu-compat.xml | 8 +- .../qemuxml2xmlout-pseries-cpu-exact.xml | 8 +- .../qemuxml2xmlout-pseries-hostdevs-1.xml | 14 +- .../qemuxml2xmlout-pseries-hostdevs-2.xml | 14 +- .../qemuxml2xmlout-pseries-hostdevs-3.xml | 13 +- .../qemuxml2xmlout-pseries-many-buses-1.xml | 11 +- .../qemuxml2xmlout-pseries-many-buses-2.xml | 11 +- .../qemuxml2xmlout-pseries-many-devices.xml | 42 +++- .../qemuxml2xmlout-pseries-nvram.xml | 6 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 8 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 8 +- .../qemuxml2xmlout-pseries-phb-default-missing.xml | 11 +- .../qemuxml2xmlout-pseries-phb-numa-node.xml | 12 +- .../qemuxml2xmlout-pseries-phb-simple.xml | 11 +- .../qemuxml2xmlout-pv-spinlock-disabled.xml | 14 +- .../qemuxml2xmlout-pv-spinlock-enabled.xml | 14 +- .../qemuxml2xmlout-q35-default-devices-only.xml | 18 +- .../qemuxml2xmlout-q35-multifunction.xml | 33 ++- .../qemuxml2xmlout-q35-pci-force-address.xml | 26 ++- .../qemuxml2xmlout-q35-pcie-autoadd.xml | 41 +++- .../qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml | 42 +++- .../qemuxml2xmlout-q35-usb2-multi.xml | 33 ++- .../qemuxml2xmlout-q35-usb2-reorder.xml | 33 ++- .../qemuxml2xmloutdata/qemuxml2xmlout-q35-usb2.xml | 25 ++- .../qemuxml2xmlout-q35-virt-manager-basic.xml | 33 ++- .../qemuxml2xmlout-q35-virtio-pci.xml | 42 +++- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 23 ++- .../qemuxml2xmlout-qemu-ns-no-env.xml | 19 +- .../qemuxml2xmlout-reboot-timeout-disabled.xml | 17 +- .../qemuxml2xmlout-reboot-timeout-enabled.xml | 17 +- .../qemuxml2xmlout-restore-v2.xml | 19 +- .../qemuxml2xmlout-s390-defaultconsole.xml | 9 +- .../qemuxml2xmlout-s390-panic-missing.xml | 3 + .../qemuxml2xmlout-s390-panic-no-address.xml | 3 + .../qemuxml2xmlout-s390-panic.xml | 3 + .../qemuxml2xmlout-seclabel-dac-none.xml | 16 +- .../qemuxml2xmlout-seclabel-device-multiple.xml | 16 +- .../qemuxml2xmlout-seclabel-dynamic-baselabel.xml | 16 +- .../qemuxml2xmlout-seclabel-dynamic-labelskip.xml | 16 +- ...emuxml2xmlout-seclabel-dynamic-none-relabel.xml | 18 +- .../qemuxml2xmlout-seclabel-dynamic-none.xml | 16 +- .../qemuxml2xmlout-seclabel-dynamic-override.xml | 17 +- .../qemuxml2xmlout-seclabel-dynamic-relabel.xml | 16 +- .../qemuxml2xmlout-seclabel-none.xml | 16 +- .../qemuxml2xmlout-seclabel-static-labelskip.xml | 16 +- .../qemuxml2xmlout-seclabel-static.xml | 16 +- .../qemuxml2xmlout-serial-spiceport-nospice.xml | 18 +- .../qemuxml2xmlout-serial-spiceport.xml | 23 ++- .../qemuxml2xmlout-serial-target-port-auto.xml | 20 +- .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 19 +- .../qemuxml2xmlout-shmem-plain-doorbell.xml | 25 ++- tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml | 25 ++- .../qemuxml2xmlout-smartcard-controller.xml | 19 +- .../qemuxml2xmlout-smartcard-host-certificates.xml | 19 +- .../qemuxml2xmlout-smartcard-host.xml | 19 +- ...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 19 +- .../qemuxml2xmlout-smartcard-passthrough-tcp.xml | 19 +- .../qemuxml2xmlout-smbios-multiple-type2.xml | 16 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-smbios.xml | 16 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-smp.xml | 19 +- .../qemuxml2xmlout-sound-device.xml | 34 +++- tests/qemuxml2xmloutdata/qemuxml2xmlout-sound.xml | 29 ++- .../qemuxml2xmlout-tap-vhost-incorrect.xml | 20 +- .../qemuxml2xmlout-tap-vhost.xml | 21 +- .../qemuxml2xmlout-tpm-passthrough.xml | 15 +- .../qemuxml2xmlout-usb-controller-default-q35.xml | 18 +- .../qemuxml2xmlout-usb-controller-explicit-q35.xml | 18 +- .../qemuxml2xmlout-usb-controller.xml | 14 +- .../qemuxml2xmlout-usb-ich9-ehci-addr.xml | 25 ++- .../qemuxml2xmloutdata/qemuxml2xmlout-usb-none.xml | 17 +- .../qemuxml2xmlout-usb-piix3-controller.xml | 14 +- .../qemuxml2xmlout-usb-port-missing.xml | 17 +- .../qemuxml2xmlout-usb-redir-filter-version.xml | 16 +- .../qemuxml2xmlout-usb-redir-filter.xml | 19 +- .../qemuxml2xmlout-usb-redir.xml | 19 +- .../qemuxml2xmlout-vcpu-placement-static.xml | 17 +- .../qemuxml2xmlout-vhost_queues.xml | 21 +- ...qemuxml2xmlout-video-device-pciaddr-default.xml | 19 +- .../qemuxml2xmlout-video-qxl-heads.xml | 19 +- .../qemuxml2xmlout-video-qxl-noheads.xml | 17 +- .../qemuxml2xmlout-video-virtio-gpu-device.xml | 17 +- .../qemuxml2xmlout-video-virtio-gpu-secondary.xml | 16 +- .../qemuxml2xmlout-video-virtio-gpu-spice-gl.xml | 17 +- .../qemuxml2xmlout-video-virtio-gpu-virgl.xml | 17 +- .../qemuxml2xmlout-virtio-input-passthrough.xml | 15 +- .../qemuxml2xmlout-virtio-input.xml | 17 +- .../qemuxml2xmlout-virtio-lun.xml | 19 +- .../qemuxml2xmlout-virtio-rng-egd.xml | 15 +- .../qemuxml2xmlout-virtio-rng-random.xml | 15 +- .../qemuxml2xmloutdata/qemuxml2xmlout-watchdog.xml | 23 ++- tests/qemuxml2xmltest.c | 63 +++++- 499 files changed, 7768 insertions(+), 1683 deletions(-) -- 2.13.5
At the point when we are building the command line all aliases are assigned. There's no point in checking. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abeb24846..47ece35f6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3526,12 +3526,6 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, const char *backendType; char *ret = NULL; - if (!mem->info.alias) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("memory device alias is not assigned")); - return NULL; - } - if (virAsprintf(&alias, "mem%s", mem->info.alias) < 0) goto cleanup; @@ -3555,12 +3549,6 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) virBuffer buf = VIR_BUFFER_INITIALIZER; const char *device; - if (!mem->info.alias) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing alias for memory device")); - return NULL; - } - switch ((virDomainMemoryModel) mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: case VIR_DOMAIN_MEMORY_MODEL_DIMM: @@ -3925,8 +3913,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, if (vlan >= 0) { virBufferAsprintf(&buf, "vlan=%d,", vlan); - if (net->info.alias) - virBufferAsprintf(&buf, "name=host%s,", net->info.alias); + virBufferAsprintf(&buf, "name=host%s,", net->info.alias); } else { virBufferAsprintf(&buf, "id=host%s,", net->info.alias); } @@ -5860,12 +5847,6 @@ qemuBuildRNGCommandLine(virLogManagerPtr logManager, virDomainRNGDefPtr rng = def->rngs[i]; char *tmp; - if (!rng->info.alias) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("RNG device is missing alias")); - return -1; - } - /* possibly add character device for backend */ if (qemuBuildRNGBackendChrdevStr(logManager, cmd, cfg, def, rng, qemuCaps, &tmp, -- 2.13.5
When generating aliases for RNG at the domain startup phase it make no sense trying to recalculate the next free index every time. We already know the index - it's the same as the index in the def->rngs[] array. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 22 +++++++++++++--------- src/qemu/qemu_alias.h | 3 ++- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 914b2b94d..4ac41947c 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -314,18 +314,22 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, int qemuAssignDeviceRNGAlias(virDomainDefPtr def, - virDomainRNGDefPtr rng) + virDomainRNGDefPtr rng, + int idx) { - size_t i; - int maxidx = 0; - int idx; + if (idx == -1) { + size_t i; + idx = 0; + for (i = 0; i < def->nrngs; i++) { + int thisidx; + thisidx = qemuDomainDeviceAliasIndex(&def->rngs[i]->info, "rng"); - for (i = 0; i < def->nrngs; i++) { - if ((idx = qemuDomainDeviceAliasIndex(&def->rngs[i]->info, "rng")) >= maxidx) - maxidx = idx + 1; + if (thisidx >= idx) + idx = thisidx + 1; + } } - if (virAsprintf(&rng->info.alias, "rng%d", maxidx) < 0) + if (virAsprintf(&rng->info.alias, "rng%d", idx) < 0) return -1; return 0; @@ -490,7 +494,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->nrngs; i++) { - if (virAsprintf(&def->rngs[i]->info.alias, "rng%zu", i) < 0) + if (qemuAssignDeviceRNGAlias(def, def->rngs[i], i) < 0) return -1; } if (def->tpm) { diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 300fd4de5..860ab6c0c 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -55,7 +55,8 @@ int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, int idx); int qemuAssignDeviceRNGAlias(virDomainDefPtr def, - virDomainRNGDefPtr rng); + virDomainRNGDefPtr rng, + int idx); int qemuAssignDeviceMemoryAlias(virDomainDefPtr def, virDomainMemoryDefPtr mems, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7dd6e5fd9..816dbd46e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2073,7 +2073,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, int ret = -1; int rv; - if (qemuAssignDeviceRNGAlias(vm->def, rng) < 0) + if (qemuAssignDeviceRNGAlias(vm->def, rng, -1) < 0) goto cleanup; /* preallocate space for the device definition */ -- 2.13.5
Let's move all the virAsprintf()-s into separate functions for better structure of the code. Later, when somebody wants to generate a device alias, all they need is to expose the function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 90 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 4ac41947c..e1431e2a2 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -286,6 +286,78 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def, } +static int +qemuAssignDeviceFSAlias(virDomainFSDefPtr fss, + int idx) +{ + return virAsprintf(&fss->info.alias, "fs%d", idx); +} + + +static int +qemuAssignDeviceSoundAlias(virDomainSoundDefPtr sound, + int idx) +{ + return virAsprintf(&sound->info.alias, "sound%d", idx); +} + + +static int +qemuAssignDeviceVideoAlias(virDomainVideoDefPtr video, + int idx) +{ + return virAsprintf(&video->info.alias, "video%d", idx); +} + + +static int +qemuAssignDeviceInputAlias(virDomainInputDefPtr input, + int idx) +{ + return virAsprintf(&input->info.alias, "input%d", idx); +} + + +static int +qemuAssignDeviceHubAlias(virDomainHubDefPtr hub, + int idx) +{ + return virAsprintf(&hub->info.alias, "hub%d", idx); +} + + +static int +qemuAssignDeviceSmartcardAlias(virDomainSmartcardDefPtr smartcard, + int idx) +{ + return virAsprintf(&smartcard->info.alias, "smartcard%d", idx); +} + + +static int +qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog, + int idx) +{ + return virAsprintf(&watchdog->info.alias, "watchdog%d", idx); +} + + +static int +qemuAssingDeviceMemballoonAlias(virDomainMemballoonDefPtr memballoon, + int idx) +{ + return virAsprintf(&memballoon->info.alias, "balloon%d", idx); +} + + +static int +qemuAssignDeviceTPMAlias(virDomainTPMDefPtr tpm, + int idx) +{ + return virAsprintf(&tpm->info.alias, "tpm%d", idx); +} + + int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, @@ -424,11 +496,11 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) } for (i = 0; i < def->nfss; i++) { - if (virAsprintf(&def->fss[i]->info.alias, "fs%zu", i) < 0) + if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { - if (virAsprintf(&def->sounds[i]->info.alias, "sound%zu", i) < 0) + if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0) return -1; } for (i = 0; i < def->nhostdevs; i++) { @@ -446,7 +518,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->nvideos; i++) { - if (virAsprintf(&def->videos[i]->info.alias, "video%zu", i) < 0) + if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0) return -1; } for (i = 0; i < def->ncontrollers; i++) { @@ -454,7 +526,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->ninputs; i++) { - if (virAsprintf(&def->inputs[i]->info.alias, "input%zu", i) < 0) + if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { @@ -474,7 +546,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->nhubs; i++) { - if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0) + if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0) return -1; } for (i = 0; i < def->nshmems; i++) { @@ -482,15 +554,15 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->nsmartcards; i++) { - if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%zu", i) < 0) + if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0) return -1; } if (def->watchdog) { - if (virAsprintf(&def->watchdog->info.alias, "watchdog%d", 0) < 0) + if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0) return -1; } if (def->memballoon) { - if (virAsprintf(&def->memballoon->info.alias, "balloon%d", 0) < 0) + if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0) return -1; } for (i = 0; i < def->nrngs; i++) { @@ -498,7 +570,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } if (def->tpm) { - if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0) + if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0) return -1; } for (i = 0; i < def->nmems; i++) { -- 2.13.5
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index e1431e2a2..00868c50f 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -481,25 +481,99 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def, } +/* + * qemuAssignDeviceAliases: + * @def: domain definition + * @qemuCaps: qemu capabilities + * @regenerate: forcibly regenerate aliases + * + * Assign aliases to domain devices. If @regenerate is false only + * the missing aliases are generated leaving already assigned + * aliases untouched. If @regenerate is true any preexisting + * aliases are overwritten. + * + * Returns 0 on success, -1 otherwise. + */ int -qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuAssignDeviceAliases(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool regenerate) { size_t i; + if (regenerate) { + /* If we need to regenerate the aliases, we have to free + * them all upfront because there are some dependencies, + * e.g. an interface can be a hostdev and so on. + * Therefore, if we'd do the freeing in the loop below we + * might not start from zero. */ + for (i = 0; i < def->ndisks; i++) + VIR_FREE(def->disks[i]->info.alias); + for (i = 0; i < def->nnets; i++) + VIR_FREE(def->nets[i]->info.alias); + for (i = 0; i < def->nfss; i++) + VIR_FREE(def->fss[i]->info.alias); + for (i = 0; i < def->nsounds; i++) + VIR_FREE(def->sounds[i]->info.alias); + for (i = 0; i < def->nhostdevs; i++) + VIR_FREE(def->hostdevs[i]->info->alias); + for (i = 0; i < def->nredirdevs; i++) + VIR_FREE(def->redirdevs[i]->info.alias); + for (i = 0; i < def->nvideos; i++) + VIR_FREE(def->videos[i]->info.alias); + for (i = 0; i < def->ncontrollers; i++) + VIR_FREE(def->controllers[i]->info.alias); + for (i = 0; i < def->ninputs; i++) + VIR_FREE(def->inputs[i]->info.alias); + for (i = 0; i < def->nparallels; i++) + VIR_FREE(def->parallels[i]->info.alias); + for (i = 0; i < def->nserials; i++) + VIR_FREE(def->serials[i]->info.alias); + for (i = 0; i < def->nchannels; i++) + VIR_FREE(def->channels[i]->info.alias); + for (i = 0; i < def->nconsoles; i++) + VIR_FREE(def->consoles[i]->info.alias); + for (i = 0; i < def->nhubs; i++) + VIR_FREE(def->hubs[i]->info.alias); + for (i = 0; i < def->nshmems; i++) + VIR_FREE(def->shmems[i]->info.alias); + for (i = 0; i < def->nsmartcards; i++) + VIR_FREE(def->smartcards[i]->info.alias); + if (def->watchdog) + VIR_FREE(def->watchdog->info.alias); + if (def->memballoon) + VIR_FREE(def->memballoon->info.alias); + for (i = 0; i < def->nrngs; i++) + VIR_FREE(def->rngs[i]->info.alias); + if (def->tpm) + VIR_FREE(def->tpm->info.alias); + for (i = 0; i < def->nmems; i++) + VIR_FREE(def->mems[i]->info.alias); + } + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0) return -1; } for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceNetAlias(def, def->nets[i], -1) < 0) return -1; } for (i = 0; i < def->nfss; i++) { + if (def->fss[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0) return -1; } @@ -509,71 +583,102 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) * linked to a NetDef, they will share an info and the alias * will already be set, so don't try to set it again. */ + if (def->hostdevs[i]->info->alias && !regenerate) + continue; if (!def->hostdevs[i]->info->alias && qemuAssignDeviceHostdevAlias(def, &def->hostdevs[i]->info->alias, -1) < 0) return -1; } for (i = 0; i < def->nredirdevs; i++) { + if (def->redirdevs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0) return -1; } for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0) return -1; } for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceControllerAlias(def, qemuCaps, def->controllers[i]) < 0) return -1; } for (i = 0; i < def->ninputs; i++) { + if (def->inputs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { + if (def->parallels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0) return -1; } for (i = 0; i < def->nserials; i++) { + if (def->serials[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0) return -1; } for (i = 0; i < def->nchannels; i++) { + if (def->channels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0) return -1; } for (i = 0; i < def->nconsoles; i++) { + if (def->consoles[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) return -1; } for (i = 0; i < def->nhubs; i++) { + if (def->hubs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0) return -1; } for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0) return -1; } for (i = 0; i < def->nsmartcards; i++) { + if (def->smartcards[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0) return -1; } - if (def->watchdog) { + if (def->watchdog && + (!def->watchdog->info.alias || regenerate)) { if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0) return -1; } - if (def->memballoon) { + if (def->memballoon && + (!def->memballoon->info.alias || regenerate)) { if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0) return -1; } for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRNGAlias(def, def->rngs[i], i) < 0) return -1; } - if (def->tpm) { + if (def->tpm && + (!def->tpm->info.alias || regenerate)) { if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0) return -1; } for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0) return -1; } diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 860ab6c0c..e33a05580 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -66,7 +66,9 @@ int qemuAssignDeviceShmemAlias(virDomainDefPtr def, virDomainShmemDefPtr shmem, int idx); -int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); +int qemuAssignDeviceAliases(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool regenerate); int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, const char *prefix); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b271792d..64ba7d454 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16145,7 +16145,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, def->emulator))) goto cleanup; - if (qemuAssignDeviceAliases(def, qemuCaps) < 0) + if (qemuAssignDeviceAliases(def, qemuCaps, true) < 0) goto cleanup; if (!(vm = virDomainObjListAdd(driver->domains, def, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e6cc41e13..448de679c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5340,7 +5340,7 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; } - if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) + if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps, false) < 0) goto cleanup; VIR_DEBUG("Setting graphics devices"); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 23a498617..0b1b00176 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -95,7 +95,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, goto cleanup; } - if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0) + if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps, true) < 0) goto cleanup; (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID; -- 2.13.5
On Thu, Sep 21, 2017 at 16:47:08 +0200, Michal Privoznik wrote:
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index e1431e2a2..00868c50f 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -481,25 +481,99 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def, }
+/* + * qemuAssignDeviceAliases: + * @def: domain definition + * @qemuCaps: qemu capabilities + * @regenerate: forcibly regenerate aliases + * + * Assign aliases to domain devices. If @regenerate is false only + * the missing aliases are generated leaving already assigned + * aliases untouched. If @regenerate is true any preexisting + * aliases are overwritten. + * + * Returns 0 on success, -1 otherwise. + */ int -qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuAssignDeviceAliases(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool regenerate) { size_t i;
+ if (regenerate) { + /* If we need to regenerate the aliases, we have to free + * them all upfront because there are some dependencies, + * e.g. an interface can be a hostdev and so on. + * Therefore, if we'd do the freeing in the loop below we + * might not start from zero. */ + for (i = 0; i < def->ndisks; i++) + VIR_FREE(def->disks[i]->info.alias); + for (i = 0; i < def->nnets; i++) + VIR_FREE(def->nets[i]->info.alias); + for (i = 0; i < def->nfss; i++) + VIR_FREE(def->fss[i]->info.alias); + for (i = 0; i < def->nsounds; i++) + VIR_FREE(def->sounds[i]->info.alias); + for (i = 0; i < def->nhostdevs; i++) + VIR_FREE(def->hostdevs[i]->info->alias); + for (i = 0; i < def->nredirdevs; i++) + VIR_FREE(def->redirdevs[i]->info.alias); + for (i = 0; i < def->nvideos; i++) + VIR_FREE(def->videos[i]->info.alias); + for (i = 0; i < def->ncontrollers; i++) + VIR_FREE(def->controllers[i]->info.alias); + for (i = 0; i < def->ninputs; i++) + VIR_FREE(def->inputs[i]->info.alias); + for (i = 0; i < def->nparallels; i++) + VIR_FREE(def->parallels[i]->info.alias); + for (i = 0; i < def->nserials; i++) + VIR_FREE(def->serials[i]->info.alias); + for (i = 0; i < def->nchannels; i++) + VIR_FREE(def->channels[i]->info.alias); + for (i = 0; i < def->nconsoles; i++) + VIR_FREE(def->consoles[i]->info.alias); + for (i = 0; i < def->nhubs; i++) + VIR_FREE(def->hubs[i]->info.alias); + for (i = 0; i < def->nshmems; i++) + VIR_FREE(def->shmems[i]->info.alias); + for (i = 0; i < def->nsmartcards; i++) + VIR_FREE(def->smartcards[i]->info.alias); + if (def->watchdog) + VIR_FREE(def->watchdog->info.alias); + if (def->memballoon) + VIR_FREE(def->memballoon->info.alias); + for (i = 0; i < def->nrngs; i++) + VIR_FREE(def->rngs[i]->info.alias); + if (def->tpm) + VIR_FREE(def->tpm->info.alias); + for (i = 0; i < def->nmems; i++) + VIR_FREE(def->mems[i]->info.alias); + } + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0) return -1; } for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceNetAlias(def, def->nets[i], -1) < 0) return -1; }
for (i = 0; i < def->nfss; i++) { + if (def->fss[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0) return -1; } @@ -509,71 +583,102 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) * linked to a NetDef, they will share an info and the alias * will already be set, so don't try to set it again. */ + if (def->hostdevs[i]->info->alias && !regenerate) + continue; if (!def->hostdevs[i]->info->alias && qemuAssignDeviceHostdevAlias(def, &def->hostdevs[i]->info->alias, -1) < 0) return -1; } for (i = 0; i < def->nredirdevs; i++) { + if (def->redirdevs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0) return -1; } for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0) return -1; } for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceControllerAlias(def, qemuCaps, def->controllers[i]) < 0) return -1; } for (i = 0; i < def->ninputs; i++) { + if (def->inputs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { + if (def->parallels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0) return -1; } for (i = 0; i < def->nserials; i++) { + if (def->serials[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0) return -1; } for (i = 0; i < def->nchannels; i++) { + if (def->channels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0) return -1; } for (i = 0; i < def->nconsoles; i++) { + if (def->consoles[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) return -1; } for (i = 0; i < def->nhubs; i++) { + if (def->hubs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0) return -1; } for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0) return -1; } for (i = 0; i < def->nsmartcards; i++) { + if (def->smartcards[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0) return -1; } - if (def->watchdog) { + if (def->watchdog && + (!def->watchdog->info.alias || regenerate)) { if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0) return -1; } - if (def->memballoon) { + if (def->memballoon && + (!def->memballoon->info.alias || regenerate)) { if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0) return -1; } for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRNGAlias(def, def->rngs[i], i) < 0) return -1; }
So if you want to make aliases persistent, at least this will not work properly with device coldplug. If you have two devices, detach the first one, then the second one moves to index 0 but will still have alias ending with 1. Then if you cold-add another device that will be put into index 1, and when starting the VM it will get assigned the same alias as the one which has the old one. This applies to all devices where the alias depends on the ordering. Additionally this series specifically does not assign aliases for coldplugged devices so there's a discrepancy between when defining a XML and modifying it with the APIs which should not really be the case.
On Thu, Sep 21, 2017 at 06:05:19PM +0200, Peter Krempa wrote:
On Thu, Sep 21, 2017 at 16:47:08 +0200, Michal Privoznik wrote:
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index e1431e2a2..00868c50f 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -481,25 +481,99 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def, }
+/* + * qemuAssignDeviceAliases: + * @def: domain definition + * @qemuCaps: qemu capabilities + * @regenerate: forcibly regenerate aliases + * + * Assign aliases to domain devices. If @regenerate is false only + * the missing aliases are generated leaving already assigned + * aliases untouched. If @regenerate is true any preexisting + * aliases are overwritten. + * + * Returns 0 on success, -1 otherwise. + */ int -qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuAssignDeviceAliases(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool regenerate) { size_t i;
+ if (regenerate) { + /* If we need to regenerate the aliases, we have to free + * them all upfront because there are some dependencies, + * e.g. an interface can be a hostdev and so on. + * Therefore, if we'd do the freeing in the loop below we + * might not start from zero. */ + for (i = 0; i < def->ndisks; i++) + VIR_FREE(def->disks[i]->info.alias); + for (i = 0; i < def->nnets; i++) + VIR_FREE(def->nets[i]->info.alias); + for (i = 0; i < def->nfss; i++) + VIR_FREE(def->fss[i]->info.alias); + for (i = 0; i < def->nsounds; i++) + VIR_FREE(def->sounds[i]->info.alias); + for (i = 0; i < def->nhostdevs; i++) + VIR_FREE(def->hostdevs[i]->info->alias); + for (i = 0; i < def->nredirdevs; i++) + VIR_FREE(def->redirdevs[i]->info.alias); + for (i = 0; i < def->nvideos; i++) + VIR_FREE(def->videos[i]->info.alias); + for (i = 0; i < def->ncontrollers; i++) + VIR_FREE(def->controllers[i]->info.alias); + for (i = 0; i < def->ninputs; i++) + VIR_FREE(def->inputs[i]->info.alias); + for (i = 0; i < def->nparallels; i++) + VIR_FREE(def->parallels[i]->info.alias); + for (i = 0; i < def->nserials; i++) + VIR_FREE(def->serials[i]->info.alias); + for (i = 0; i < def->nchannels; i++) + VIR_FREE(def->channels[i]->info.alias); + for (i = 0; i < def->nconsoles; i++) + VIR_FREE(def->consoles[i]->info.alias); + for (i = 0; i < def->nhubs; i++) + VIR_FREE(def->hubs[i]->info.alias); + for (i = 0; i < def->nshmems; i++) + VIR_FREE(def->shmems[i]->info.alias); + for (i = 0; i < def->nsmartcards; i++) + VIR_FREE(def->smartcards[i]->info.alias); + if (def->watchdog) + VIR_FREE(def->watchdog->info.alias); + if (def->memballoon) + VIR_FREE(def->memballoon->info.alias); + for (i = 0; i < def->nrngs; i++) + VIR_FREE(def->rngs[i]->info.alias); + if (def->tpm) + VIR_FREE(def->tpm->info.alias); + for (i = 0; i < def->nmems; i++) + VIR_FREE(def->mems[i]->info.alias); + } + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0) return -1; } for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceNetAlias(def, def->nets[i], -1) < 0) return -1; }
for (i = 0; i < def->nfss; i++) { + if (def->fss[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0) return -1; } @@ -509,71 +583,102 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) * linked to a NetDef, they will share an info and the alias * will already be set, so don't try to set it again. */ + if (def->hostdevs[i]->info->alias && !regenerate) + continue; if (!def->hostdevs[i]->info->alias && qemuAssignDeviceHostdevAlias(def, &def->hostdevs[i]->info->alias, -1) < 0) return -1; } for (i = 0; i < def->nredirdevs; i++) { + if (def->redirdevs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0) return -1; } for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0) return -1; } for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceControllerAlias(def, qemuCaps, def->controllers[i]) < 0) return -1; } for (i = 0; i < def->ninputs; i++) { + if (def->inputs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { + if (def->parallels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0) return -1; } for (i = 0; i < def->nserials; i++) { + if (def->serials[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0) return -1; } for (i = 0; i < def->nchannels; i++) { + if (def->channels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0) return -1; } for (i = 0; i < def->nconsoles; i++) { + if (def->consoles[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) return -1; } for (i = 0; i < def->nhubs; i++) { + if (def->hubs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0) return -1; } for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0) return -1; } for (i = 0; i < def->nsmartcards; i++) { + if (def->smartcards[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0) return -1; } - if (def->watchdog) { + if (def->watchdog && + (!def->watchdog->info.alias || regenerate)) { if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0) return -1; } - if (def->memballoon) { + if (def->memballoon && + (!def->memballoon->info.alias || regenerate)) { if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0) return -1; } for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRNGAlias(def, def->rngs[i], i) < 0) return -1; }
So if you want to make aliases persistent, at least this will not work properly with device coldplug.
If you have two devices, detach the first one, then the second one moves to index 0 but will still have alias ending with 1. Then if you cold-add another device that will be put into index 1, and when starting the VM it will get assigned the same alias as the one which has the old one.
This applies to all devices where the alias depends on the ordering.
Yep, we would need to maintain a hash table remembering all currently assigned aliases, and then increment the counter until we find a free one for that dev type. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 09/21/2017 07:18 PM, Daniel P. Berrange wrote:
On Thu, Sep 21, 2017 at 06:05:19PM +0200, Peter Krempa wrote:
On Thu, Sep 21, 2017 at 16:47:08 +0200, Michal Privoznik wrote:
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-)
So if you want to make aliases persistent, at least this will not work properly with device coldplug.
Ah, good point.
If you have two devices, detach the first one, then the second one moves to index 0 but will still have alias ending with 1. Then if you cold-add another device that will be put into index 1, and when starting the VM it will get assigned the same alias as the one which has the old one.
This applies to all devices where the alias depends on the ordering.
Yep, we would need to maintain a hash table remembering all currently assigned aliases, and then increment the counter until we find a free one for that dev type.
Alternatively, every time we want to assign an alias for a device we traverse its siblings and see if it's taken. for (i = 0; ; i++) { alias = "device$i"; for (j = 0; j < def->ndevice; j++) { if (STREQ(def->device[j]->info.alias, alias)) continue; break } if (j != def->ndevice) { /* alias is free */ device->alias = alias; break; } } This is roughly the same as your approach except the hash table is taken out. Which I find better because: 1) the hash table would have to live under qemuDomainObjPrivatePtr (or under virDomainObjPtr) and I'd have to pass pointer to it all the way down to virDomainDeviceInfoClear() -> huge change 2) we would have two copies of aliases in memory. IOW, my approach is more time complex but less memory complex. Michal
On Fri, Sep 22, 2017 at 10:18:02 +0200, Michal Privoznik wrote:
On 09/21/2017 07:18 PM, Daniel P. Berrange wrote:
On Thu, Sep 21, 2017 at 06:05:19PM +0200, Peter Krempa wrote:
On Thu, Sep 21, 2017 at 16:47:08 +0200, Michal Privoznik wrote:
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-)
So if you want to make aliases persistent, at least this will not work properly with device coldplug.
Ah, good point.
If you have two devices, detach the first one, then the second one moves to index 0 but will still have alias ending with 1. Then if you cold-add another device that will be put into index 1, and when starting the VM it will get assigned the same alias as the one which has the old one.
This applies to all devices where the alias depends on the ordering.
Yep, we would need to maintain a hash table remembering all currently assigned aliases, and then increment the counter until we find a free one for that dev type.
Alternatively, every time we want to assign an alias for a device we traverse its siblings and see if it's taken.
for (i = 0; ; i++) { alias = "device$i"; for (j = 0; j < def->ndevice; j++) { if (STREQ(def->device[j]->info.alias, alias)) continue; break } if (j != def->ndevice) { /* alias is free */ device->alias = alias; break; } }
You can also generate them of a global sequence number. Qemu does not care that they are not consecutive. It might trigger some OCD based eye twitching, but it's way less yucky.
This is roughly the same as your approach except the hash table is taken out. Which I find better because:
1) the hash table would have to live under qemuDomainObjPrivatePtr (or under virDomainObjPtr) and I'd have to pass pointer to it all the way down to virDomainDeviceInfoClear() -> huge change
2) we would have two copies of aliases in memory.
IOW, my approach is more time complex but less memory complex.
Preferably as with disks or memory modules, you can base the alias on something which is already guaranteed to be unique (IDE unit number, DIMM slot number) which breaks the dependancy and gives you uniqueness for free.
On Fri, Sep 22, 2017 at 10:25:46AM +0200, Peter Krempa wrote:
On Fri, Sep 22, 2017 at 10:18:02 +0200, Michal Privoznik wrote:
On 09/21/2017 07:18 PM, Daniel P. Berrange wrote:
On Thu, Sep 21, 2017 at 06:05:19PM +0200, Peter Krempa wrote:
On Thu, Sep 21, 2017 at 16:47:08 +0200, Michal Privoznik wrote:
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-)
So if you want to make aliases persistent, at least this will not work properly with device coldplug.
Ah, good point.
If you have two devices, detach the first one, then the second one moves to index 0 but will still have alias ending with 1. Then if you cold-add another device that will be put into index 1, and when starting the VM it will get assigned the same alias as the one which has the old one.
This applies to all devices where the alias depends on the ordering.
Yep, we would need to maintain a hash table remembering all currently assigned aliases, and then increment the counter until we find a free one for that dev type.
Alternatively, every time we want to assign an alias for a device we traverse its siblings and see if it's taken.
for (i = 0; ; i++) { alias = "device$i"; for (j = 0; j < def->ndevice; j++) { if (STREQ(def->device[j]->info.alias, alias)) continue; break } if (j != def->ndevice) { /* alias is free */ device->alias = alias; break; } }
You can also generate them of a global sequence number. Qemu does not care that they are not consecutive. It might trigger some OCD based eye twitching, but it's way less yucky.
Yeah a single global counter would be reasonable IMHO. Still has a little cpu time to initialize it when libvirtd starts up & loads persistent configs, but that's not unreasonable. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Michal Privoznik <mprivozn@redhat.com> [2017-09-21, 04:47PM +0200]:
In the near future the qemuAssignDeviceAliases() function is going to be called multiple times: once at the domain define time, then in domain prepare phase. To avoid regenerating the same aliases the second time we need to be able to tell the function which time it is being called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_alias.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 4 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuhotplugtest.c | 2 +- 5 files changed, 115 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index e1431e2a2..00868c50f 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -481,25 +481,99 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def, }
+/* + * qemuAssignDeviceAliases: + * @def: domain definition + * @qemuCaps: qemu capabilities + * @regenerate: forcibly regenerate aliases + * + * Assign aliases to domain devices. If @regenerate is false only + * the missing aliases are generated leaving already assigned + * aliases untouched. If @regenerate is true any preexisting + * aliases are overwritten. + * + * Returns 0 on success, -1 otherwise. + */ int -qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +qemuAssignDeviceAliases(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool regenerate) { size_t i;
+ if (regenerate) { + /* If we need to regenerate the aliases, we have to free + * them all upfront because there are some dependencies, + * e.g. an interface can be a hostdev and so on. + * Therefore, if we'd do the freeing in the loop below we + * might not start from zero. */ + for (i = 0; i < def->ndisks; i++) + VIR_FREE(def->disks[i]->info.alias); + for (i = 0; i < def->nnets; i++) + VIR_FREE(def->nets[i]->info.alias); + for (i = 0; i < def->nfss; i++) + VIR_FREE(def->fss[i]->info.alias); + for (i = 0; i < def->nsounds; i++) + VIR_FREE(def->sounds[i]->info.alias); + for (i = 0; i < def->nhostdevs; i++) + VIR_FREE(def->hostdevs[i]->info->alias); + for (i = 0; i < def->nredirdevs; i++) + VIR_FREE(def->redirdevs[i]->info.alias); + for (i = 0; i < def->nvideos; i++) + VIR_FREE(def->videos[i]->info.alias); + for (i = 0; i < def->ncontrollers; i++) + VIR_FREE(def->controllers[i]->info.alias); + for (i = 0; i < def->ninputs; i++) + VIR_FREE(def->inputs[i]->info.alias); + for (i = 0; i < def->nparallels; i++) + VIR_FREE(def->parallels[i]->info.alias); + for (i = 0; i < def->nserials; i++) + VIR_FREE(def->serials[i]->info.alias); + for (i = 0; i < def->nchannels; i++) + VIR_FREE(def->channels[i]->info.alias); + for (i = 0; i < def->nconsoles; i++) + VIR_FREE(def->consoles[i]->info.alias); + for (i = 0; i < def->nhubs; i++) + VIR_FREE(def->hubs[i]->info.alias); + for (i = 0; i < def->nshmems; i++) + VIR_FREE(def->shmems[i]->info.alias); + for (i = 0; i < def->nsmartcards; i++) + VIR_FREE(def->smartcards[i]->info.alias); + if (def->watchdog) + VIR_FREE(def->watchdog->info.alias); + if (def->memballoon) + VIR_FREE(def->memballoon->info.alias); + for (i = 0; i < def->nrngs; i++) + VIR_FREE(def->rngs[i]->info.alias); + if (def->tpm) + VIR_FREE(def->tpm->info.alias); + for (i = 0; i < def->nmems; i++) + VIR_FREE(def->mems[i]->info.alias); + } + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0) return -1; } for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceNetAlias(def, def->nets[i], -1) < 0) return -1; }
for (i = 0; i < def->nfss; i++) { + if (def->fss[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0) return -1; } @@ -509,71 +583,102 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) * linked to a NetDef, they will share an info and the alias * will already be set, so don't try to set it again. */ + if (def->hostdevs[i]->info->alias && !regenerate) + continue; if (!def->hostdevs[i]->info->alias && qemuAssignDeviceHostdevAlias(def, &def->hostdevs[i]->info->alias, -1) < 0) return -1; } for (i = 0; i < def->nredirdevs; i++) { + if (def->redirdevs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0) return -1; } for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0) return -1; } for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceControllerAlias(def, qemuCaps, def->controllers[i]) < 0) return -1; } for (i = 0; i < def->ninputs; i++) { + if (def->inputs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { + if (def->parallels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0) return -1; } for (i = 0; i < def->nserials; i++) { + if (def->serials[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0) return -1; } for (i = 0; i < def->nchannels; i++) { + if (def->channels[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0) return -1; } for (i = 0; i < def->nconsoles; i++) { + if (def->consoles[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) return -1; } for (i = 0; i < def->nhubs; i++) { + if (def->hubs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0) return -1; } for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0) return -1; } for (i = 0; i < def->nsmartcards; i++) { + if (def->smartcards[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0) return -1; } - if (def->watchdog) { + if (def->watchdog && + (!def->watchdog->info.alias || regenerate)) {
Can you make this conditional consistent to the others?
if (qemuAssignDeviceWatchdogAlias(def->watchdog, 0) < 0) return -1; } - if (def->memballoon) { + if (def->memballoon && + (!def->memballoon->info.alias || regenerate)) {
And this.
if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0) return -1; } for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceRNGAlias(def, def->rngs[i], i) < 0) return -1; } - if (def->tpm) { + if (def->tpm && + (!def->tpm->info.alias || regenerate)) {
This too.
if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0) return -1; } for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->info.alias && !regenerate) + continue; if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0) return -1; } diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 860ab6c0c..e33a05580 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -66,7 +66,9 @@ int qemuAssignDeviceShmemAlias(virDomainDefPtr def, virDomainShmemDefPtr shmem, int idx);
-int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); +int qemuAssignDeviceAliases(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool regenerate);
int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, const char *prefix); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b271792d..64ba7d454 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16145,7 +16145,7 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, def->emulator))) goto cleanup;
- if (qemuAssignDeviceAliases(def, qemuCaps) < 0) + if (qemuAssignDeviceAliases(def, qemuCaps, true) < 0) goto cleanup;
if (!(vm = virDomainObjListAdd(driver->domains, def, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e6cc41e13..448de679c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5340,7 +5340,7 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; }
- if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) + if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps, false) < 0) goto cleanup;
VIR_DEBUG("Setting graphics devices"); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 23a498617..0b1b00176 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -95,7 +95,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, goto cleanup; }
- if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0) + if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps, true) < 0)
Instead of passing a state flag into the function, would it be possible to manually clear the aliases before calling qemuAssignDeviceAliases and dropping the regenerate flag? I find boolean parameters that fundamentally change the behaviour of a function hard to follow at a certain time.
goto cleanup;
(*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID; -- 2.13.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- IBM Systems Linux on z Systems & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Schönaicher Str. 220 71032 Böblingen Phone: +49 7031 16 1819 E-Mail: bwalk@de.ibm.com ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
This will be called at the end of virDomainDefPostParse to allow hypervisor drivers to fill in device aliases. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 7 +++++++ src/conf/domain_conf.h | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e3e9fc748..653357531 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5011,6 +5011,13 @@ virDomainDefPostParseInternal(virDomainDefPtr def, goto cleanup; } + if (xmlopt->config.assignAliasesCallback) { + ret = xmlopt->config.assignAliasesCallback(def, caps, parseFlags, + xmlopt->config.priv, + data.parseOpaque); + if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0) + goto cleanup; + } if ((ret = virDomainDefPostParseCheckFeatures(def, xmlopt)) < 0) goto cleanup; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e11ae5247..764360ccb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2559,6 +2559,18 @@ typedef int (*virDomainDefAssignAddressesCallback)(virDomainDef *def, void *opaque, void *parseOpaque); +/* Driver callback for assigning device aliases, called at the + * end of parsing, after all defaults and implicit devices have + * been added and all addresses were assigned. @parseOpaque is + * opaque data passed by virDomainDefParse *caller, @opaque is + * opaque data set by driver (usually pointer to driver private + * data). */ +typedef int (*virDomainDefAssignAliasesCallback)(virDomainDefPtr def, + virCapsPtr caps, + unsigned int parseFlags, + void *opaque, + void *parseOpaque); + typedef int (*virDomainDefPostParseDataAlloc)(const virDomainDef *def, virCapsPtr caps, unsigned int parseFlags, @@ -2588,6 +2600,7 @@ struct _virDomainDefParserConfig { virDomainDefPostParseCallback domainPostParseCallback; virDomainDeviceDefPostParseCallback devicesPostParseCallback; virDomainDefAssignAddressesCallback assignAddressesCallback; + virDomainDefAssignAliasesCallback assignAliasesCallback; virDomainDefPostParseDataFree domainPostParseDataFree; /* validation callbacks */ -- 2.13.5
https://bugzilla.redhat.com/show_bug.cgi?id=1434451 Users might want to uniquely identify devices even before the domain is started. Therefore we must generate device aliases right at the define time. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 24 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 ++++++++ .../qemuxml2xmlout-channel-virtio-state-active.xml | 20 +++++++++++++++--- .../qemuxml2xmlout-disk-active-commit.xml | 16 ++++++++++++--- .../qemuxml2xmlout-disk-backing-chains-active.xml | 19 ++++++++++++++--- .../qemuxml2xmlout-disk-mirror-active.xml | 19 ++++++++++++++--- .../qemuxml2xmlout-disk-mirror-old.xml | 19 ++++++++++++++--- .../qemuxml2xmlout-seclabel-static-labelskip.xml | 16 ++++++++++++--- 8 files changed, 124 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee6641847..15fd88dcc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3936,6 +3936,29 @@ qemuDomainDefAssignAddresses(virDomainDef *def, } +static int +qemuDomainDefAssignAliases(virDomainDef *def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED, + void *parseOpaque) +{ + /* Note that qemuCaps may be NULL when this function is called. This + * function shall not fail in that case. It will be re-run on VM startup + * with the capabilities populated. */ + virQEMUCapsPtr qemuCaps = parseOpaque; + + /* Skip address assignment if @qemuCaps is not present. In such case devices + * which are automatically added may be missing. Additionally @qemuCaps should + * only be missing when reloading configs, thus aliases were already + * assigned. */ + if (!qemuCaps) + return 1; + + return qemuAssignDeviceAliases(def, qemuCaps, true); +} + + static int qemuDomainPostParseDataAlloc(const virDomainDef *def, virCapsPtr caps ATTRIBUTE_UNUSED, @@ -3969,6 +3992,7 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { .devicesPostParseCallback = qemuDomainDeviceDefPostParse, .domainPostParseCallback = qemuDomainDefPostParse, .assignAddressesCallback = qemuDomainDefAssignAddresses, + .assignAliasesCallback = qemuDomainDefAssignAliases, .domainValidateCallback = qemuDomainDefValidate, .deviceValidateCallback = qemuDomainDeviceDefValidate, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 70be0c32d..5c2a45fd3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -485,6 +485,15 @@ testCompareXMLToArgv(const void *data) if (testUpdateQEMUCaps(info, vm, driver.caps) < 0) goto cleanup; + /* Forcibly regenerate device aliases. They are generated in + * virDomainDefParseFile(), however, qemuCaps are filled out + * just partially therefore the generated aliases might be + * wrong. It's only after testUpdateQEMUCaps() call that we + * have full set of caps. However, we can't move the call any + * sooner because it needs vm->def. */ + if (qemuAssignDeviceAliases(vm->def, info->qemuCaps, true) < 0) + goto cleanup; + log = virTestLogContentAndReset(); VIR_FREE(log); virResetLastError(); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml index 8cddbeff7..326e285cb 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-state-active.xml @@ -18,33 +18,47 @@ <source dev='/dev/HostVG/QEMUGuest1'/> <backingStore/> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'> + <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> + <alias name='ide'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='virtio-serial' index='1'> + <alias name='virtio-serial1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.foo' state='connected'/> + <alias name='channel0'/> <address type='virtio-serial' controller='1' bus='0' port='3'/> </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.foo1' state='disconnected'/> + <alias name='channel1'/> <address type='virtio-serial' controller='1' bus='0' port='4'/> </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.foo2'/> + <alias name='channel2'/> <address type='virtio-serial' controller='1' bus='0' port='5'/> </channel> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml index cc26af109..f63e8c3b7 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-active-commit.xml @@ -27,18 +27,28 @@ <source dev='/dev/HostVG/QEMUGuest1'/> </mirror> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'> + <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> + <alias name='ide'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml index 83d47df56..5abe21270 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-backing-chains-active.xml @@ -25,6 +25,7 @@ <backingStore/> </backingStore> <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> <disk type='network' device='disk'> @@ -58,6 +59,7 @@ </backingStore> </backingStore> <target dev='vdb' bus='virtio'/> + <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <disk type='network' device='disk'> @@ -67,6 +69,7 @@ </source> <backingStore/> <target dev='vdc' bus='virtio'/> + <alias name='virtio-disk2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> <disk type='network' device='disk'> @@ -85,6 +88,7 @@ <backingStore/> </backingStore> <target dev='vdd' bus='virtio'/> + <alias name='virtio-disk3'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </disk> <disk type='block' device='disk'> @@ -92,15 +96,24 @@ <source dev='/dev/HostVG/QEMUGuest11'/> <backingStore/> <target dev='vde' bus='virtio'/> + <alias name='virtio-disk4'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> <controller type='usb' index='0'> + <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml index c1e8a33ec..ebe6e4471 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-active.xml @@ -21,6 +21,7 @@ <source dev='/dev/HostVG/QEMUGuest1Copy'/> </mirror> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='block' device='cdrom'> @@ -28,6 +29,7 @@ <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> + <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <disk type='file' device='disk'> @@ -38,6 +40,7 @@ <source file='/tmp/copy.img'/> </mirror> <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> <disk type='file' device='disk'> @@ -48,18 +51,28 @@ <source file='/tmp/logcopy.img'/> </mirror> <target dev='vdb' bus='virtio'/> + <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <controller type='usb' index='0'> + <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> + <alias name='ide'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml index e390bc02f..451737621 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml @@ -21,6 +21,7 @@ <source file='/dev/HostVG/QEMUGuest1Copy'/> </mirror> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='block' device='cdrom'> @@ -28,6 +29,7 @@ <backingStore/> <target dev='hdc' bus='ide'/> <readonly/> + <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <disk type='file' device='disk'> @@ -38,24 +40,35 @@ <source file='/tmp/copy.img'/> </mirror> <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> <disk type='file' device='disk'> <source file='/tmp/logs.img'/> <backingStore/> <target dev='vdb' bus='virtio'/> + <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <controller type='usb' index='0'> + <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> + <alias name='ide'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml index d37b950cb..8ea5a29d7 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-static-labelskip.xml @@ -20,18 +20,28 @@ </source> <backingStore/> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'> + <alias name='usb'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> + <alias name='ide'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </memballoon> </devices> -- 2.13.5
Now that we are generating the device aliases at the define time, we might as well format them into inactive XML. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- I've trimmed out all the test/ XML changes. I've generated them using: tests $ VIR_TEST_REGENERATE_OUTPUT=1 ./test With the changes in this patch would be ~1MB in size. If interested regenerate them on your own. Alternatively, you can find them at my github. src/conf/domain_conf.c | 7 +- tests/qemuxml2xmltest.c | 63 ++++++++++- 489 files changed, 7417 insertions(+), 1623 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 653357531..ae38bae47 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5770,10 +5770,9 @@ virDomainDeviceInfoFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (info->alias && - !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) { + + if (info->alias) virBufferAsprintf(buf, "<alias name='%s'/>\n", info->alias); - } if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) { virBufferAsprintf(buf, "<master startport='%d'/>\n", @@ -6333,7 +6332,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { if (alias == NULL && - !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && + !(flags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) && virXMLNodeNameEqual(cur, "alias")) { alias = cur; } else if (address == NULL && diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4b2fbd990..c9afafe14 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -13,11 +13,16 @@ #ifdef WITH_QEMU # include "internal.h" +# include "qemu/qemu_alias.h" # include "qemu/qemu_domain_address.h" # include "qemu/qemu_domain.h" # include "testutilsqemu.h" # include "virstring.h" +# define __QEMU_CAPSPRIV_H_ALLOW__ +# include "qemu/qemu_capspriv.h" +# undef __QEMU_CAPSPRIV_H_ALLOW__ + # define VIR_FROM_THIS VIR_FROM_NONE static virQEMUDriver driver; @@ -38,6 +43,25 @@ struct testInfo { virQEMUCapsPtr qemuCaps; }; + +static int +testUpdateQEMUCaps(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + virQEMUCapsSetArch(qemuCaps, def->os.arch); + + virQEMUCapsInitQMPBasicArch(qemuCaps); + + /* We need to pretend QEMU 2.0.0 is in use so that pSeries guests + * will get the correct alias assigned to their buses. + * See virQEMUCapsHasPCIMultiBus() */ + virQEMUCapsSetVersion(qemuCaps, 2000000); + + return 0; +} + + + static int qemuXML2XMLActivePreFormatCallback(virDomainDefPtr def, const void *opaque) @@ -48,6 +72,19 @@ qemuXML2XMLActivePreFormatCallback(virDomainDefPtr def, if (!info->activeVcpus) info->activeVcpus = virDomainDefGetOnlineVcpumap(def); + /* Update capabilities according to domain definition, + * just like we're doing in qemuxml2argvtest. */ + testUpdateQEMUCaps(def, info->qemuCaps); + + /* Forcibly regenerate device aliases. They are generated in + * virDomainDefParseFile(), however, qemuCaps are filled out + * just partially therefore the generated aliases might be + * wrong. It's only after testUpdateQEMUCaps() call that we + * have full set of caps. However, we can't move the call any + * sooner because it needs vm->def. */ + if (qemuAssignDeviceAliases(def, info->qemuCaps, true) < 0) + return -1; + return 0; } @@ -64,6 +101,29 @@ testXML2XMLActive(const void *opaque) } +static int +qemuXML2XMLInactivePreFormatCallback(virDomainDefPtr def, + const void *opaque) +{ + struct testInfo *info = (struct testInfo *) opaque; + + /* Update capabilities according to domain definition, + * just like we're doing in qemuxml2argvtest. */ + testUpdateQEMUCaps(def, info->qemuCaps); + + /* Forcibly regenerate device aliases. They are generated in + * virDomainDefParseFile(), however, qemuCaps are filled out + * just partially therefore the generated aliases might be + * wrong. It's only after testUpdateQEMUCaps() call that we + * have full set of caps. However, we can't move the call any + * sooner because it needs vm->def. */ + if (qemuAssignDeviceAliases(def, info->qemuCaps, true) < 0) + return -1; + + return 0; +} + + static int testXML2XMLInactive(const void *opaque) { @@ -71,7 +131,8 @@ testXML2XMLInactive(const void *opaque) return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName, info->outInactiveName, false, - NULL, opaque, 0, + qemuXML2XMLInactivePreFormatCallback, + opaque, 0, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); } -- 2.13.5
participants (4)
-
Bjoern Walk -
Daniel P. Berrange -
Michal Privoznik -
Peter Krempa