On Sun, 2019-01-13 at 18:12 -0500, Cole Robinson wrote: [...]
@@ -5431,6 +5436,16 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk) return -1; }
+ if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + (disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL || + disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk model '%s' only supported for bus '%s'"), + virDomainDiskModelTypeToString(disk->model), + virDomainDiskBusTypeToString(disk->bus));
s/only supported/not supported/ I also wonder if more thorough validation would be a good idea: this only ensures that VIR_DOMAIN_DISK_MODEL_VIRTIO_* can't be used with other bus types, but if later on we add more VIR_DOMAIN_DISK_MODEL_* we won't be protected against those being use with VIR_DOMAIN_DISK_BUS_VIRTIO... [...]
+typedef enum { + VIR_DOMAIN_DISK_MODEL_DEFAULT = 0, + VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL, + VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL, + + VIR_DOMAIN_DISK_MODEL_LAST +} virDomainDiskModel;
I think we want to have VIR_DOMAIN_DISK_MODEL_VIRTIO as well, for consistency's sake if nothing else. For all other devices, model='virtio' will be (or already is) a perfectly valid configuration, so not accepting it for disks would violate the principle of least surprise. [...]
+++ b/tests/qemuxml2argvdata/virtio-non-transitional.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='block' device='disk' model='virtio-non-transitional'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0' model='none'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain>
You can strip this file further by removing the <currentMemory>, <os><boot>, <clock>, <on_*> and <input> elements at least. -- Andrea Bolognani / Red Hat / Virtualization