HDDMODEL is insufficiently flexible, cannot add buses

[AdamWill]

In Fedora, we were trying to use the HDDMODEL variable for a SATA test. Long story short, it turns out that doesn't work. In os-autoinst 4.2, there was a bit of code that sneakily turned the SATA disk we were trying to add into a PATA disk. In os-autoinst 4.3, that's gone, but there's simply no way you can cause os-autoinst to construct a qemu invocation which will successfully include a SATA disk, because you can't add a SATA bus. The way HDDMODEL is set up, it's only capable of attaching a disk to a bus that the VM would have otherwise; you can't use HDDMODEL or any other variable I can find to add a bus to the VM, and nothing else causes os-autoinst VMs to have a SATA bus, so this just doesn't seem possible at present.

There's more details on this at https://phab.qadevel.cloud.fedoraproject.org/T691 . It would be good if we could fix this somehow, I don't know how much abstraction we want to put in; the low-abstraction way would just be to have numbered variables (DEVICE1, DEVICE2...) for constructing -device arguments, a high level way might be to have variables like SATA or SCSI which cause os-autoinst to add an appropriate -device argument to add the specified bus.

[okurz]

How about keeping it more generic and adding a variable like "HDDOPTIONS" which are added after HDDMODEL e.g. in 8eb1912#diff-ce37f278fec25235d9f905442ecd8e5aR429
as in

my $hdd = "$vars->{HDDMODEL},drive=hd$i" . $vars->{HDDOPTIONS} ? ",$vars->{HDDOPTIONS}" : "";
push(@params, "-device", $hdd);

[AdamWill]

AFAICS the choices are basically either:

• A completely generic "add any qemu parameter you like" variable
• A variable for adding -device arguments
• A very abstracted "add a specific bus" variable

I might be missing something (I'm hardly a qemu expert), but at least to my current understanding, what we need to add is a bus, and buses are just -device parameters, there's nothing inherently 'HDD' about them. So calling any of the choices HDDOPTIONS doesn't quite nail it, for me.

I might spend a bit of time today poking around the man pages and trying different commands and see what I come up with, I guess.

[sysrich]

typing off the top of my head (ie. I consider the opinion I am about to share as open to persuasion)

I am nervous about an "add any qemu parameter you like" variable as it would be too easy to create very easily broken combinations

I like the idea of adding specific -device arguments, and I think @AdamWill is right, we need to add a bus, and there is nothing 'HDD' about it - Maybe we need to rename/replace the HDDMODEL variable

A very abstracted 'add a specific bus' variable might be nice, but I dont think it'll be flexible enough

[AdamWill]

I guess I missed an option 4, which might be viable - I'll look into it and see if I can send a patch. It's simply:

• Make HDDMODEL work better

i.e. we could just have os-autoinst know all the reasonable values for HDDMODEL and have it add an appropriate -device parameter when necessary (and any other twiddling that's needed, I guess). I'll see if I can come up with something which works at least for PATA, SATA, SCSI and virtio, I guess.

[aaannz]

I tend to prefer 4th. option. Make a list of supported bus values which are allowed as HDDMODEL (or HDDBUS if we want to be somewhat backward compatible). That way we can connect right drive with right bus controller and all can be at one place in code if needed to update later.

Do we use qemu device name as HDDBUS?
That would be 'ich9-ahci, virtio-scsi-pci, virtio-blk-pci' now if I'm not mistaken. Plus we can add 'scsi-hd and ide-hd' if some ever wants to test Windows XP (O_o)

Later I would also like the availability to specify different buses and parameters for different drives. So to move from generic NUMDISKS to something like HDD_1_[MODEL (or BUS)|SIZE|PATHS (for multipath)|FORMAT]. But I don't have clear use case for this except for larger test space.

[AdamWill]

@aaannz I also considered the 'different settings for different drives' case, but figured since neither we nor you have any uses for it ATM, there's no point getting fancy. :)

My aim, if possible, is to remain backwards-compatible - so all values for HDDMODEL that either we or you currently use, or have previously used/tried to use, would be considered 'valid', and os-autoinst would add all the necessary arguments to fulfil the intent of that HDDMODEL value. So I'm not actually planning to change the expected values at all. I'll work on it this afternoon and see if it seems viable.

[AdamWill]

On the topic of multiple disks, btw, we already have HDD_1 and HDD_2 for attaching specific disk images (rather than just using NUMDISKS to have some number of 'standard' disks), so I would think the 'obvious' approach would be to allow HDDMODEL_1, HDDMODEL_2 etc. But I'm not promising I'll work on that. :)

[coolo]

Just to throw in my oppinion: I know the downsides, but I would favor option 1. I consider this command line creation through variables rather crude.

[coolo]

Translated my last comment into https://progress.opensuse.org/issues/18466