This patch ensures that changes in attributes of interfaces will emit
errors accept if they are missing from the XML.
Previously we were falsely reporting successful updates, because some
changed attributes got overwritten before the validity checks.

https://bugzilla.redhat.com/show_bug.cgi?id=1599513

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
---
Changes from v3:

Adjusted to Jano's comments in v3; type and address are now checked together.

 src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0b84a503bb..4f290b5648 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3598,16 +3598,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
         goto cleanup;
     }
 
-    /* info: if newdev->info is empty, fill it in from olddev,
-     * otherwise verify that it matches - nothing is allowed to
-     * change. (There is no helper function to do this, so
-     * individually check the few feidls of virDomainDeviceInfo that
-     * are relevant in this case).
+    /* info: Nothing is allowed to change. First fill the missing newdev->info
+     * from olddev and then check for changes.
      */
-    if (!virDomainDeviceAddressIsValid(&newdev->info,
-                                       VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-        virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
-        goto cleanup;
+    /* if pci addr is missing or is invalid we overwrite it from olddev */
+    if (newdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+        !virDomainDeviceAddressIsValid(&newdev->info,
+                                       newdev->info.type)) {
+        newdev->info.type = olddev->info.type;
+        newdev->info.addr = olddev->info.addr;
+    }
+    if (olddev->info.type != newdev->info.type) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot modify network device address type"));
     }
     if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci,
                                   &newdev->info.addr.pci)) {
@@ -3622,21 +3625,33 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 
     /* device alias is checked already in virDomainDefCompatibleDevice */
 
+    if (newdev->info.rombar == VIR_TRISTATE_BOOL_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 &&
+        VIR_STRDUP(newdev->info.romfile, olddev->info.romfile) < 0)
+        goto cleanup;
     if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot modify network rom file"));
         goto cleanup;
     }
+
+    if (newdev->info.bootIndex == 0)
+        newdev->info.bootIndex = olddev->info.bootIndex;
     if (olddev->info.bootIndex != newdev->info.bootIndex) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot modify network device boot index setting"));
         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.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Aug 27, 2018 at 09:50:07AM +0200, Katerina Koukiou wrote:
>This patch ensures that changes in attributes of interfaces will emit
>errors accept if they are missing from the XML.

except

>Previously we were falsely reporting successful updates, because some
>changed attributes got overwritten before the validity checks.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1599513
>
>Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
>---
>Changes from v3:
>
>Adjusted to Jano's comments in v3; type and address are now checked together.
>
> src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 9 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list