[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion



On Wed, Aug 01, 2018 at 12:37:13PM +0200, Katerina Koukiou wrote:
We need to first perform the checks for changed/missing elements

If we copy all the missing elements,

and then we can overwrite the missing ones. Otherwise we might

there's no need to overwrite them again.

falsely report successfull update, because some elements got
overwritten before the validity checks.

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

Signed-off-by: Katerina Koukiou <kkoukiou redhat com>
---
src/qemu/qemu_hotplug.c | 43 ++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1488f0a7c2..8d98c149e2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3445,23 +3445,9 @@ 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 check for changes and
+     * then fill the missing newdev->info from olddev.
     */
-    if (!virDomainDeviceAddressIsValid(&newdev->info,
-                                       VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-        virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {

IOW we can get rid of 'virDomainDeviceInfoCopy' completely.
Here we only need to check and copy the PCI address,

-        goto cleanup;
-    }
-    if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci,
-                                  &newdev->info.addr.pci)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                       _("cannot modify network device guest PCI address"));
-        goto cleanup;
-    }
    /* grab alias from olddev if not set in newdev */
    if (!newdev->info.alias &&
        VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0)
@@ -3469,26 +3455,51 @@ 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;

Because all the other attributes are filled out here.

Jano

    if (olddev->info.romenabled != newdev->info.romenabled) {
        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                       _("cannot modify network device rom enabled setting"));
        goto cleanup;
    }

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]