[libvirt] [PATCH 0/2] qemuDomainDiskChangeSupported: Add missing 'address' check

Disk->info is not live updatable so add a check for this. Otherwise libvirt reports success even though no data was updated. Marc Hartmayer (2): conf: Make virDomainDeviceInfoAddressIsEqual() public qemu: qemuDomainDiskChangeSupported: Add missing 'address' check src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 13 +++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) -- 2.5.5

This function will be needed by the QEMU driver in an upcoming patch. Additionally, removed a useless empty line. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 14c12f8..826ca82 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3351,11 +3351,10 @@ virDomainDeviceInfoNeedsFormat(virDomainDeviceInfoPtr info, unsigned int flags) return false; } -static bool +bool virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, const virDomainDeviceInfo *b) { - if (a->type != b->type) return false; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 747c11e..dfd5eb3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3212,4 +3212,9 @@ virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def, virTypedParameterPtr params, int *nparams, int maxparams); + +bool +virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, + const virDomainDeviceInfo *b) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6303dec..861e53c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -276,6 +276,7 @@ virDomainDeviceDefFree; virDomainDeviceDefParse; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo; +virDomainDeviceInfoAddressIsEqual; virDomainDeviceInfoCopy; virDomainDeviceInfoIterate; virDomainDeviceTypeToString; -- 2.5.5

Disk->info is not live updatable so add a check for this. Otherwise libvirt reports success even though no data was updated. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- 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 4aae14d..5f7cd60 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5165,6 +5165,19 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, /* "snapshot" is a libvirt internal field and thus can be changed */ /* startupPolicy is allowed to be updated. Therefore not checked here. */ CHECK_EQ(transient, "transient", true); + + /* Note: For some address types the address auto generation for + * @disk has still not happened at this point (e.g. driver + * specific addresses) therefore we can't catch these possible + * address modifications here. */ + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&disk->info, &orig_disk->info)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify field '%s' of the disk"), + "address"); + return false; + } + CHECK_EQ(info.bootIndex, "boot order", true); CHECK_EQ(rawio, "rawio", true); CHECK_EQ(sgio, "sgio", true); -- 2.5.5

On 06.12.2016 15:30, Marc Hartmayer wrote:
Disk->info is not live updatable so add a check for this. Otherwise libvirt reports success even though no data was updated.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- 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 4aae14d..5f7cd60 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5165,6 +5165,19 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, /* "snapshot" is a libvirt internal field and thus can be changed */ /* startupPolicy is allowed to be updated. Therefore not checked here. */ CHECK_EQ(transient, "transient", true); + + /* Note: For some address types the address auto generation for + * @disk has still not happened at this point (e.g. driver + * specific addresses) therefore we can't catch these possible + * address modifications here. */ + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&disk->info, &orig_disk->info)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify field '%s' of the disk"), + "address"); + return false; + } + CHECK_EQ(info.bootIndex, "boot order", true); CHECK_EQ(rawio, "rawio", true); CHECK_EQ(sgio, "sgio", true);
ACK, but I guess other devices suffer the same problem as well. Mind looking into that? Michal

Seasonal ping... On 12/06/2016 03:30 PM, Marc Hartmayer wrote:
Disk->info is not live updatable so add a check for this. Otherwise libvirt reports success even though no data was updated.
Marc Hartmayer (2): conf: Make virDomainDeviceInfoAddressIsEqual() public qemu: qemuDomainDiskChangeSupported: Add missing 'address' check
src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 13 +++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-)
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 06.12.2016 15:30, Marc Hartmayer wrote:
Disk->info is not live updatable so add a check for this. Otherwise libvirt reports success even though no data was updated.
Marc Hartmayer (2): conf: Make virDomainDeviceInfoAddressIsEqual() public qemu: qemuDomainDiskChangeSupported: Add missing 'address' check
src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 13 +++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-)
ACKed and pushed. Michal

On Tue, Dec 20, 2016 at 11:33 AM +0100, Michal Privoznik <mprivozn@redhat.com> wrote:
On 06.12.2016 15:30, Marc Hartmayer wrote:
Disk->info is not live updatable so add a check for this. Otherwise libvirt reports success even though no data was updated.
Marc Hartmayer (2): conf: Make virDomainDeviceInfoAddressIsEqual() public qemu: qemuDomainDiskChangeSupported: Add missing 'address' check
src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 13 +++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-)
ACKed and pushed.
Michal
Thanks. -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (3)
-
Boris Fiuczynski
-
Marc Hartmayer
-
Michal Privoznik