If there are absent values in an already existing element
specifying rom settings, we simply use the old ones. This
behaviour is not desired, as users might think that deleting the
element from XML would delete the setting (because the hotplug
succeeds) - which does not happen. Because of that, we should not
accept an interface without elements that cannot be changed.

Therefore, we should not allow absent values for already existing
rom setting during hotplug.

Resolves: https://issues.redhat.com/browse/RHEL-7109

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
 src/qemu/qemu_hotplug.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1f4620d833..7f158ddcd5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3838,16 +3838,12 @@ qemuDomainChangeNet(virQEMUDriver *driver,
 
     /* device alias is checked already in virDomainDefCompatibleDevice */
 
-    if (newdev->info.rombar == VIR_TRISTATE_SWITCH_ABSENT)
-        newdev->info.rombar = olddev->info.rombar;
     if (olddev->info.rombar != newdev->info.rombar) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot modify network device rom bar setting"));
         goto cleanup;
     }
 
-    if (!newdev->info.romfile)
-        newdev->info.romfile = g_strdup(olddev->info.romfile);
     if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot modify network rom file"));
@@ -3860,8 +3856,6 @@ qemuDomainChangeNet(virQEMUDriver *driver,
         goto cleanup;
     }
 
-    if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT)
-        newdev->info.romenabled = olddev->info.romenabled;
     if (olddev->info.romenabled != newdev->info.romenabled) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot modify network device rom enabled setting"));
-- 
2.45.1

On 7/24/24 13:02, Kristina Hanicova wrote:
> If there are absent values in an already existing element
> specifying rom settings, we simply use the old ones. This
> behaviour is not desired, as users might think that deleting the
> element from XML would delete the setting (because the hotplug
> succeeds) - which does not happen. Because of that, we should not
> accept an interface without elements that cannot be changed.
> 
> Therefore, we should not allow absent values for already existing
> rom setting during hotplug.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-7109
> 
> Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 6 ------
>  1 file changed, 6 deletions(-)
> 

While this has a chance to annoy users, it is the only sensible thing to
do and we should never have had the code in the first place. It's bitten
us too many times.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal