Re: [PATCH] qemu: Compare group_names by STRNEQ not CHECK_EQ

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
 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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux