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

[Ján Tomko]

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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Peter Krempa]

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 
> ---
>  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.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Han Han]

Fix issue introduced by 047cfb05ee. Since group_name is str, use STRNEQ
instead of CHECK_EQ to do comparition.

Signed-off-by: Han Han 
---
 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