5:39 a.m.
Rename virDomainCheckVirtioOptions into
virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
options are absent. The old name was very misleading.
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_validate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index dd4c6e0fb3..a2f236c299 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -227,7 +227,7 @@ virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr
*seclabels,
static int
-virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio)
+virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio)
{
if (!virtio)
return 0;
@@ -316,7 +316,7 @@ virDomainDiskDefValidate(const virDomainDef *def,
return -1;
}
- if (virDomainCheckVirtioOptions(disk->virtio) < 0)
+ if (virDomainCheckVirtioOptionsAreAbsent(disk->virtio) < 0)
return -1;
}
@@ -1363,7 +1363,7 @@ virDomainNetDefValidate(const virDomainNetDef *net)
}
if (!virDomainNetIsVirtioModel(net) &&
- virDomainCheckVirtioOptions(net->virtio) < 0) {
+ virDomainCheckVirtioOptionsAreAbsent(net->virtio) < 0) {
return -1;
}
@@ -1513,7 +1513,7 @@ virDomainVsockDefValidate(const virDomainVsockDef *vsock)
}
if (!virDomainVsockIsVirtioModel(vsock) &&
- virDomainCheckVirtioOptions(vsock->virtio) < 0)
+ virDomainCheckVirtioOptionsAreAbsent(vsock->virtio) < 0)
return -1;
return 0;
--
2.26.2
5:48 a.m.
On 1/29/21 8:39 AM, Boris Fiuczynski wrote:
Rename virDomainCheckVirtioOptions into
virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
s/virDomainCheckVirtioOptionsAreAbent/virDomainCheckVirtioOptionsAreAbsent
options are absent. The old name was very misleading.
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/conf/domain_validate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index dd4c6e0fb3..a2f236c299 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -227,7 +227,7 @@ virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr
*seclabels,
static int
-virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio)
+virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio)
{
if (!virtio)
return 0;
@@ -316,7 +316,7 @@ virDomainDiskDefValidate(const virDomainDef *def,
return -1;
}
- if (virDomainCheckVirtioOptions(disk->virtio) < 0)
+ if (virDomainCheckVirtioOptionsAreAbsent(disk->virtio) < 0)
return -1;
}
@@ -1363,7 +1363,7 @@ virDomainNetDefValidate(const virDomainNetDef *net)
}
if (!virDomainNetIsVirtioModel(net) &&
- virDomainCheckVirtioOptions(net->virtio) < 0) {
+ virDomainCheckVirtioOptionsAreAbsent(net->virtio) < 0) {
return -1;
}
@@ -1513,7 +1513,7 @@ virDomainVsockDefValidate(const virDomainVsockDef *vsock)
}
if (!virDomainVsockIsVirtioModel(vsock) &&
- virDomainCheckVirtioOptions(vsock->virtio) < 0)
+ virDomainCheckVirtioOptionsAreAbsent(vsock->virtio) < 0)
return -1;
return 0;
6:38 a.m.
On 1/29/21 12:48 PM, Daniel Henrique Barboza wrote:
On 1/29/21 8:39 AM, Boris Fiuczynski wrote:
> Rename virDomainCheckVirtioOptions into
> virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
s/virDomainCheckVirtioOptionsAreAbent/virDomainCheckVirtioOptionsAreAbsent
> options are absent. The old name was very misleading.
>
> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Yep, fixed and pushed.
Michal
6:35 a.m.
On Fri, Jan 29, 2021 at 12:39:22PM +0100, Boris Fiuczynski wrote:
Rename virDomainCheckVirtioOptions into
virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
options are absent. The old name was very misleading.
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_validate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
NACK to this patch. I don't see any point in this rename. The whole file
mostly checks if something is missing. If anything I would go for
virDomainVirtioOptionsValidate.
Pavel
6:44 a.m.
On 1/29/21 1:35 PM, Pavel Hrdina wrote:
On Fri, Jan 29, 2021 at 12:39:22PM +0100, Boris Fiuczynski wrote:
> Rename virDomainCheckVirtioOptions into
> virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
> options are absent. The old name was very misleading.
>
> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> ---
> src/conf/domain_validate.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
NACK to this patch. I don't see any point in this rename. The whole file
mostly checks if something is missing. If anything I would go for
virDomainVirtioOptionsValidate.
Hm.. meanwhile I pushed it. The problem with the current name is that it
is very misleading. What the function does is it checks whether user did
not enable a virtio option for a non-virtio device, e.g. ACS for e1000
NIC. That explains why callers do very misleading check:
if (model != virtio)
virDomainCheckVirtioOptions(model->virtio)
Therefore, renaming to AreAbsent() express what is done better. My other
idea was to push model != virtio check into the function itself, e.g.
like this:
virDomainCheckVirtioOptions(model == virtio, model->virtio)
This way we would need to rename the function and still can keep the checks.
Michal
6:37 a.m.
On Fri, Jan 29, 2021 at 12:39:22 +0100, Boris Fiuczynski wrote:
Rename virDomainCheckVirtioOptions into
virDomainCheckVirtioOptionsAreAbent since it checks if all virtio
options are absent. The old name was very misleading.
We usually have functions which check presence using the 'Has' verb,
thus in this case it'd become:
virDomainHasVirtioOptions
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_validate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
1424
days inactive
1424
days old
5 comments
5 participants
participants (5)
-
Boris Fiuczynski -
Daniel Henrique Barboza -
Michal Privoznik -
Pavel Hrdina -
Peter Krempa