6:27 p.m.
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(a)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
7:11 p.m.
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(a)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.
7:43 p.m.
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(a)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
2054
days inactive
2054
days old
2 comments
3 participants
participants (3)
-
Han Han -
Ján Tomko -
Peter Krempa