Default shared configuration has incorrect assumptions

[pevogam]

The setting in

avocado-vt/virttest/shared/cfg/guest-hw.cfg

Line 88 in 80ca89f

image_boot~=yes

has preceding comment

        # Add -drive ...boot=yes unless qemu-kvm is 0.12.1.2 or newer
        # then kvm_vm will ignore this option.

but on most recent Qemu 8.2 it is not ignored and rather has a dramatic effect on our VT tests where the image will be strictly booted in cases of UEFI reinstallation and USB installation tests that previously relied on alternative boot order configuration using Avocado VT. Is this default really intended to be set? If not I would recommend

-         image_boot~=yes
+         # image_boot~=yes

which used to be the case long ago. Is there any other purpose behind this parameter and its meaning different then the above-mentioned comment?

@luckyh @YongxueHong Do you happen to know more about this choice?

[luckyh]

@pevogam sorry, I cannot recall on that since it was introduced too long ago (more than 10 years)... anyway, I'd like to prefer to drop the setting as well due to

• we don't apply such a setting to other kind of virtual block devices, that means we seems to be safe for dealing with boot orders in the case that image_boot not being present
• nowadays, qemu don't even have the option (-drive ...,boot=) documented in the manual page
• -blockdev was recommended to be the replacement of -drive from several years ago and the former don't support the boot option

does it sounds good to you?

[pevogam]

@pevogam sorry, I cannot recall on that since it was introduced too long ago (more than 10 years)...

Indeed, the difference is that back then it was commented out and later through the years it got uncommented and now ended up affecting our test suite as it was still making use of -drive options via the virttest parameter qemu_force_use_drive_expression. The ~= must have only affected users that defined the image_boot parameter on the first place but in our case it seems it somehow comes up and as a result this setting completely modifies our previous behavior when we switched to virtio storage.

anyway, I'd like to prefer to drop the setting as well due to

* we don't apply such a setting to other kind of virtual block devices, that means we seems to be safe for dealing with boot orders in the case that `image_boot` not being present

* nowadays, qemu don't even have the option (`-drive ...,boot=`) documented in the manual page

* `-blockdev` was recommended to be the replacement of `-drive` from several years ago and the former don't support the `boot` option

does it sounds good to you?

Perhaps it is worth keeping it a while longer for backwards compatibility. My main point and worry was that it introduces unexpected behavior in comparison to other storage hardware variants since we were using -drives there and didn't end up forced to always boot the same image (e.g. when wanting to boot from a CD or USB instead). And I definitely can't find image_boot anywhere in our configs but it got triggered by this line when switching to virtio storage hardware variant.

[pevogam]

Actually IIUC the ~= will set the parameter if not set before:

class LLazySet(LOperators):
    __slots__ = []
    identifier = "~="

    def apply_to_dict(self, d):
        if self.name not in _reserved_keys and self.name not in d:
            d[self.name] = _substitution(self.value, d)

which will always define this parameter for all users that use the legacy -devices together with VirtIO drivers and considering some users might have already defined multiple of many hard drives and which ones to boot from this could severely interfere with them. Note that this parameter used to be commented out before so the previous behavior was image_boot=no and along the way it was modified in an impactful way to image_boot~=yes which results in image_boot=yes for nearly everyone. I noticed for instance that the parameter is defined even when we use -blockdev instead of -drive because we rely on the virtio drivers.

[luckyh]

@pevogam sorry for the delayed reply.

My main point and worry was that it introduces unexpected behavior in comparison to other storage hardware variants since we were using -drives there and didn't end up forced to always boot the same image (e.g. when wanting to boot from a CD or USB instead).

Maybe we could consider using the bootindex parameter as the replacement, in order to having a consistent boot order. But I'd still suggest that we could simply remove the image_boot~=yes as, IMO, all the primary block devices should be applied with the same bunch of boot order/index settings by default.

Btw, which version of qemu were you using for the test? it looks that the boot option had already been removed from the source tree since ~8 years ago [1], so I'm a little curious why it taking the effect.

[1] https://gitlab.com/qemu-project/qemu/-/commit/0e153b04cccaeaa272a687194ea353167878b10f

[pevogam]

@pevogam sorry for the delayed reply.

My main point and worry was that it introduces unexpected behavior in comparison to other storage hardware variants since we were using -drives there and didn't end up forced to always boot the same image (e.g. when wanting to boot from a CD or USB instead).

Maybe we could consider using the bootindex parameter as the replacement, in order to having a consistent boot order. But I'd still suggest that we could simply remove the image_boot~=yes as, IMO, all the primary block devices should be applied with the same bunch of boot order/index settings by default.

The bootindex parameter seems to have some other problems I described in #4043 and I think it does not apply to legacy -drive options.

Btw, which version of qemu were you using for the test? it looks that the boot option had already been removed from the source tree since ~8 years ago [1], so I'm a little curious why it taking the effect.

[1] https://gitlab.com/qemu-project/qemu/-/commit/0e153b04cccaeaa272a687194ea353167878b10f

Actually the version of qemu I am using is one of the most recent ones:

# qemu-kvm --version
QEMU emulator version 8.2.7 (qemu-8.2.7-1.fc40)
Copyright (c) 2003-2023 Fabrice Bellard and the QEMU Project developers

I think the reason it took effect is because of the old drive expressions. You can see from above that if we move to block devices then it no longer has effect. So I am fine with the decision of VT developers to have the option there if it has effect only on drives but if this was the default option from the very beginning. As it seems it moved from default image_boot=no to image_boot~=yes which has a lot of unwanted side effects and ~= will not protect anyone without the parameter since it will definite it for them (also mentioned above).

So bottom line is that I advise I open a pull request removing this somewhat inverting commits a2cc3a4 and more importantly 68a6450.