Re: [libvirt] [PATCH 2/4] qemu: introduce CHECK_STREQ_NULLABLE in qemuDomainDiskChangeSupported

On 3/28/19 10:34 AM, Ján Tomko wrote: >A marco for comparing string fields of the disk. Polo'ed-by: Laine Stump <laine(a)laine.org&gt; (seriously, though - s/marco/macro/ :-) > >https://bugzilla.redhat.com/show_bug.cgi?id=1601677 > >Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; >--- > src/qemu/qemu_domain.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > >diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >index bb3a672d47..72e322d6a7 100644 >--- a/src/qemu/qemu_domain.c >+++ b/src/qemu/qemu_domain.c >@@ -9322,6 +9322,18 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, > } \ > } while (0) >+#define CHECK_STREQ_NULLABLE(field, field_name) \ >+ do { \ >+ if (!disk->field) \ >+ break; \ So is a missing value in the updated XML equal to "no change"? Or Does a missing value actually mean "this should be un-set if it has been set to something"?

(I'm asking this because in the case of MTU for <interface>, if the existing interface has an mtu set (even to 1500), and the updated XML has no MTU, we consider that a change (and don't allow it). Reviewed-by: Laine Stump <laine(a)laine.org&gt; once the commit message typo is fixed, and if the meaning of "not specified" for a field in the update truly is meant to be "don't change" rather than "remove any previous setting of this field and return it to default". >+ if (STRNEQ_NULLABLE(disk->field, orig_disk->field)) { \ >+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \ >+ _("cannot modify field '%s' of the disk"), \ >+ field_name); \ >+ return false; \ >+ } \ >+ } while (0) >+ > CHECK_EQ(device, "device", false); > CHECK_EQ(bus, "bus", false); > if (STRNEQ(disk->dst, orig_disk->dst)) { >@@ -9469,6 +9481,7 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, > } > #undef CHECK_EQ >+#undef CHECK_STREQ_NULLABLE > return true; > }