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

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

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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list