[libvirt] [PATCH] qemu: Compare group_names by STRNEQ not CHECK_EQ
Fix issue introduced by 047cfb05ee. Since group_name is str, use STRNEQ instead of CHECK_EQ to do comparition. Signed-off-by: Han Han <hhan@redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86e80391e1..e6d0fbef04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9387,9 +9387,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, CHECK_EQ(blkdeviotune.size_iops_sec, "blkdeviotune size_iops_sec", true); - CHECK_EQ(blkdeviotune.group_name, - "blkdeviotune group_name", - true); + if (disk->blkdeviotune.group_name) { + if (STRNEQ(disk->blkdeviotune.group_name, orig_disk->blkdeviotune.group_name)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("cannot modify field '%s' of the disk"), + "target"); + return false; + } + } if (disk->serial && STRNEQ_NULLABLE(disk->serial, orig_disk->serial)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, -- 2.20.1
On Mon, Mar 18, 2019 at 18:27:05 +0800, Han Han wrote:
Fix issue introduced by 047cfb05ee. Since group_name is str, use STRNEQ instead of CHECK_EQ to do comparition.
Signed-off-by: Han Han <hhan@redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86e80391e1..e6d0fbef04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9387,9 +9387,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, CHECK_EQ(blkdeviotune.size_iops_sec, "blkdeviotune size_iops_sec", true); - CHECK_EQ(blkdeviotune.group_name, - "blkdeviotune group_name", - true); + if (disk->blkdeviotune.group_name) { + if (STRNEQ(disk->blkdeviotune.group_name, orig_disk->blkdeviotune.group_name)) {
This will crash in case when orig_disk->blkdeviotune.group_name is NULL. You need to use STRNEQ_NULLABLE. It's also questionable whether we should do anything if the new value is NULL as we can't reset the group name, but I think it's okay to assume that it's impossible to delete the group name at least in context of qemu.
On Mon, Mar 18, 2019 at 12:11:17PM +0100, Peter Krempa wrote:
On Mon, Mar 18, 2019 at 18:27:05 +0800, Han Han wrote:
Fix issue introduced by 047cfb05ee. Since group_name is str, use STRNEQ instead of CHECK_EQ to do comparition.
comparison
Signed-off-by: Han Han <hhan@redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86e80391e1..e6d0fbef04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9387,9 +9387,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, CHECK_EQ(blkdeviotune.size_iops_sec, "blkdeviotune size_iops_sec", true); - CHECK_EQ(blkdeviotune.group_name, - "blkdeviotune group_name", - true);
Also, there are many uses of the if (field && STRNEQ_NULLABLE) pattern, looks like introducing a CHECK_STREQ would reduce the line count.
+ if (disk->blkdeviotune.group_name) { + if (STRNEQ(disk->blkdeviotune.group_name, orig_disk->blkdeviotune.group_name)) {
This will crash in case when orig_disk->blkdeviotune.group_name is NULL.
You need to use STRNEQ_NULLABLE. It's also questionable whether we should do anything if the new value is NULL as we can't reset the group name, but I think it's okay to assume that it's impossible to delete the group name at least in context of qemu.
It is also impossible to tell whether the user omitted the parameter out of laziness or they want to remove a particular parameter. Jano
participants (3)
-
Han Han -
Ján Tomko -
Peter Krempa