src/qemu/qemu_hotplug.c | 43 ++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-)
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 <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) {
- 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;
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(&newdev->info,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+ virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
+ 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;
+ }
/* (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
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; > } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.