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
---
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(>info,
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-virDomainDeviceInfoCopy(>info, >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(>info.addr.pci,
- >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;
}
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
We need to first perform the checks for changed/missing elements
and then we can overwrite the missing ones. Otherwise we might
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
---
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(>info,
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-virDomainDeviceInfoCopy(>info, >info) < 0) {
-goto cleanup;
-}
-if (!virPCIDeviceAddressEqual(>info.addr.pci,
- >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;
if (olddev->info.romenabled != newdev->info.romenabled) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom enabled setting"));
goto cleanup;
}
+
+/* if pci addr is missing or is invalid we overwrite all info struct */
+if (!virDomainDeviceAddressIsValid(>info,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+virDomainDeviceInfoCopy(>info, >info) < 0) {
+goto cleanup;
+}
+if (!virPCIDeviceAddressEqual(>info.addr.pci,
+ >info.addr.pci)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot modify network device guest PCI address"));
+goto cleanup;
+}
/* (end of device info checks) */
if (STRNEQ_NULLABLE(olddev->filter, newdev->filter) ||
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
