11:40 a.m.
There is an online service call LGTM (Looks Good To Me) which does
static analysis of open source projects and I happened to learn that
they include coverage of libvirt
https://lgtm.com/projects/g/libvirt/libvirt
I looked at the alerts they reported. Currently no errors, 41 warnings
and 90 recommendations (79 of which are FIXME comments :-).
There's nothing particularly important they identify right now, but I
felt like addressing a few of them anyway, hence this series.
Daniel P. Berrangé (6):
conf: remove pointless check on enum value
remote: remove variable whose value is a constant
storage: pass struct _virStorageBackendQemuImgInfo by reference
qemu: pass virDomainDeviceInfo by reference
hyperv: remove unused 'total' variable
hyperv: use "is None" not "== None" for PEP-8 compliance
src/conf/domain_conf.c | 20 ++++++++---------
src/hyperv/hyperv_wmi_generator.py | 3 +--
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_domain.c | 10 ++++-----
src/qemu/qemu_domain.h | 9 ++++----
src/qemu/qemu_domain_address.c | 2 +-
src/remote/remote_daemon_dispatch.c | 8 ++-----
src/storage/storage_util.c | 35 ++++++++++++++---------------
8 files changed, 42 insertions(+), 49 deletions(-)
--
2.20.1
11:40 a.m.
New subject: [libvirt] [PATCH 1/6] conf: remove pointless check on enum value
'val' is initialized from virDomainCapsFeatureTypeFromString and a
few lines earlier there was already a check for 'val < 0'.
The 'val >= 0' is thus always true. The enum conversion similarly
ensures that the val will be less than VIR_DOMAIN_CAPS_FEATURE_LAST,
so "val < VIR_DOMAIN_CAPS_FEATURE_LAST' is thus always true too.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/domain_conf.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9409d93c23..f2ef53a9a3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20524,18 +20524,16 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (val >= 0 && val < VIR_DOMAIN_CAPS_FEATURE_LAST) {
- if ((tmp = virXMLPropString(nodes[i], "state"))) {
- if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp))
== -1) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown state attribute '%s' of
feature capability '%s'"),
- tmp, virDomainFeatureTypeToString(val));
- goto error;
- }
- VIR_FREE(tmp);
- } else {
- def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
+ if ((tmp = virXMLPropString(nodes[i], "state"))) {
+ if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp)) ==
-1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown state attribute '%s' of feature
capability '%s'"),
+ tmp, virDomainFeatureTypeToString(val));
+ goto error;
}
+ VIR_FREE(tmp);
+ } else {
+ def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
}
}
VIR_FREE(nodes);
--
2.20.1
3:07 p.m.
New subject: [libvirt] [PATCH 1/6] conf: remove pointless check on enum value
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
'val' is initialized from virDomainCapsFeatureTypeFromString
and a
few lines earlier there was already a check for 'val < 0'.
The 'val >= 0' is thus always true. The enum conversion similarly
ensures that the val will be less than VIR_DOMAIN_CAPS_FEATURE_LAST,
so "val < VIR_DOMAIN_CAPS_FEATURE_LAST' is thus always true too.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/domain_conf.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
However, do see below for a change that either could be made now or as a
separate patch afterwards with an implied R-by as well.
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9409d93c23..f2ef53a9a3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20524,18 +20524,16 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (val >= 0 && val < VIR_DOMAIN_CAPS_FEATURE_LAST) {
- if ((tmp = virXMLPropString(nodes[i], "state"))) {
- if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp))
== -1) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown state attribute '%s' of
feature capability '%s'"),
- tmp, virDomainFeatureTypeToString(val));
- goto error;
- }
- VIR_FREE(tmp);
- } else {
- def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
+ if ((tmp = virXMLPropString(nodes[i], "state"))) {
+ if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp)) ==
-1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown state attribute '%s' of feature
capability '%s'"),
+ tmp, virDomainFeatureTypeToString(val));
^^^^
This caught my eye...
Since @val was sourced from virDomainCapsFeatureTypeFromString so one
would think it should use virDomainCapsFeatureTypeToString(val) at the
very least. Or use 'nodes[i]->name' directly.
As a test I modified tests/domainschemadata/domain-caps-features.xml to
set state to "foo" and then creating the lxc guest would result in :
error: unsupported configuration: unknown state attribute 'foo' of
feature capability 'hpt'
fixing to use DomainCapsFeature got:
error: unsupported configuration: unknown state attribute 'foo' of
feature capability 'mknod'
I checked other virDomainFeatureTypeToString callers and they all seemed
OK - it was just this one.
+ goto error;
}
+ VIR_FREE(tmp);
+ } else {
+ def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
}
}
VIR_FREE(nodes);
4:22 a.m.
New subject: [libvirt] [PATCH 1/6] conf: remove pointless check on enum value
On Wed, Jan 30, 2019 at 04:07:13PM -0500, John Ferlan wrote:
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
> 'val' is initialized from virDomainCapsFeatureTypeFromString and a
> few lines earlier there was already a check for 'val < 0'.
>
> The 'val >= 0' is thus always true. The enum conversion similarly
> ensures that the val will be less than VIR_DOMAIN_CAPS_FEATURE_LAST,
> so "val < VIR_DOMAIN_CAPS_FEATURE_LAST' is thus always true too.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/conf/domain_conf.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
However, do see below for a change that either could be made now or as a
separate patch afterwards with an implied R-by as well.
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9409d93c23..f2ef53a9a3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -20524,18 +20524,16 @@ virDomainDefParseXML(xmlDocPtr xml,
> goto error;
> }
>
> - if (val >= 0 && val < VIR_DOMAIN_CAPS_FEATURE_LAST) {
> - if ((tmp = virXMLPropString(nodes[i], "state"))) {
> - if ((def->caps_features[val] =
virTristateSwitchTypeFromString(tmp)) == -1) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("unknown state attribute '%s' of
feature capability '%s'"),
> - tmp, virDomainFeatureTypeToString(val));
> - goto error;
> - }
> - VIR_FREE(tmp);
> - } else {
> - def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
> + if ((tmp = virXMLPropString(nodes[i], "state"))) {
> + if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp))
== -1) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown state attribute '%s' of
feature capability '%s'"),
> + tmp, virDomainFeatureTypeToString(val));
^^^^
This caught my eye...
Since @val was sourced from virDomainCapsFeatureTypeFromString so one
would think it should use virDomainCapsFeatureTypeToString(val) at the
very least. Or use 'nodes[i]->name' directly.
As a test I modified tests/domainschemadata/domain-caps-features.xml to
set state to "foo" and then creating the lxc guest would result in :
error: unsupported configuration: unknown state attribute 'foo' of
feature capability 'hpt'
fixing to use DomainCapsFeature got:
error: unsupported configuration: unknown state attribute 'foo' of
feature capability 'mknod'
I checked other virDomainFeatureTypeToString callers and they all seemed
OK - it was just this one.
Oh fun. I'll fix this in a separate patch with your R-b.
> + goto error;
> }
> + VIR_FREE(tmp);
> + } else {
> + def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
> }
> }
> VIR_FREE(nodes);
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:40 a.m.
New subject: [libvirt] [PATCH 2/6] remote: remove variable whose value is a constant
The 'rv' variable is never changed after being declared, so can be
removed.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index fcd602304f..df28259042 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -3659,7 +3659,6 @@ remoteDispatchAuthSaslStart(virNetServerPtr server,
const char *serverout;
size_t serveroutlen;
int err;
- int rv = -1;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
const char *identity;
@@ -3739,8 +3738,7 @@ remoteDispatchAuthSaslStart(virNetServerPtr server,
virResetLastError();
virReportError(VIR_ERR_AUTH_FAILED, "%s",
_("authentication failed"));
- if (rv < 0)
- virNetMessageSaveError(rerr);
+ virNetMessageSaveError(rerr);
virMutexUnlock(&priv->lock);
return -1;
}
@@ -3757,7 +3755,6 @@ remoteDispatchAuthSaslStep(virNetServerPtr server,
const char *serverout;
size_t serveroutlen;
int err;
- int rv = -1;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
const char *identity;
@@ -3837,8 +3834,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server,
virResetLastError();
virReportError(VIR_ERR_AUTH_FAILED, "%s",
_("authentication failed"));
- if (rv < 0)
- virNetMessageSaveError(rerr);
+ virNetMessageSaveError(rerr);
virMutexUnlock(&priv->lock);
return -1;
}
--
2.20.1
3:07 p.m.
New subject: [libvirt] [PATCH 2/6] remote: remove variable whose value is a constant
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
The 'rv' variable is never changed after being declared, so
can be
removed.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:40 a.m.
New subject: [libvirt] [PATCH 3/6] storage: pass struct _virStorageBackendQemuImgInfo by reference
The struct _virStorageBackendQemuImgInfo is quite large so it is
preferrable to pass it by reference instead of by value. This requires
us to stop modifying the "compat" field.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/storage/storage_util.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 37b3d58667..de6f8ec2bd 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -742,38 +742,40 @@ struct _virStorageBackendQemuImgInfo {
static int
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo,
char **opts,
- struct _virStorageBackendQemuImgInfo info)
+ struct _virStorageBackendQemuImgInfo *info)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (info.backingPath)
+ if (info->backingPath)
virBufferAsprintf(&buf, "backing_fmt=%s,",
- virStorageFileFormatTypeToString(info.backingFormat));
+ virStorageFileFormatTypeToString(info->backingFormat));
if (encinfo)
- virQEMUBuildQemuImgKeySecretOpts(&buf, encinfo, info.secretAlias);
+ virQEMUBuildQemuImgKeySecretOpts(&buf, encinfo, info->secretAlias);
- if (info.preallocate) {
- if (info.size_arg > info.allocation)
+ if (info->preallocate) {
+ if (info->size_arg > info->allocation)
virBufferAddLit(&buf, "preallocation=metadata,");
else
virBufferAddLit(&buf, "preallocation=falloc,");
}
- if (info.nocow)
+ if (info->nocow)
virBufferAddLit(&buf, "nocow=on,");
- if (info.compat)
- virBufferAsprintf(&buf, "compat=%s,", info.compat);
+ if (info->compat)
+ virBufferAsprintf(&buf, "compat=%s,", info->compat);
+ else if (info->format == VIR_STORAGE_FILE_QCOW2)
+ virBufferAddLit(&buf, "compat=0.10,");
- if (info.features && info.format == VIR_STORAGE_FILE_QCOW2) {
- if (virBitmapIsBitSet(info.features,
+ if (info->features && info->format == VIR_STORAGE_FILE_QCOW2) {
+ if (virBitmapIsBitSet(info->features,
VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS)) {
- if (STREQ_NULLABLE(info.compat, "0.10")) {
+ if (STREQ_NULLABLE(info->compat, "0.10")) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("lazy_refcounts not supported with compat"
" level %s"),
- info.compat);
+ info->compat);
goto error;
}
virBufferAddLit(&buf, "lazy_refcounts,");
@@ -942,13 +944,10 @@ storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
static int
storageBackendCreateQemuImgSetOptions(virCommandPtr cmd,
virStorageEncryptionInfoDefPtr encinfo,
- struct _virStorageBackendQemuImgInfo info)
+ struct _virStorageBackendQemuImgInfo *info)
{
char *opts = NULL;
- if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat)
- info.compat = "0.10";
-
if (storageBackendCreateQemuImgOpts(encinfo, &opts, info) < 0)
return -1;
if (opts)
@@ -1196,7 +1195,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
}
if (convertStep != VIR_STORAGE_VOL_ENCRYPT_CONVERT) {
- if (storageBackendCreateQemuImgSetOptions(cmd, encinfo, info) < 0)
+ if (storageBackendCreateQemuImgSetOptions(cmd, encinfo, &info) < 0)
goto error;
if (info.inputPath)
virCommandAddArg(cmd, info.inputPath);
--
2.20.1
3:08 p.m.
New subject: [libvirt] [PATCH 3/6] storage: pass struct _virStorageBackendQemuImgInfo by reference
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
The struct _virStorageBackendQemuImgInfo is quite large so it is
preferrable to pass it by reference instead of by value. This requires
us to stop modifying the "compat" field.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/storage/storage_util.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:40 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: pass virDomainDeviceInfo by reference
The virDomainDeviceInfo parameter is a large struct so it is preferrable
to pass it by reference instead of by value.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_domain.c | 10 +++++-----
src/qemu/qemu_domain.h | 9 +++++----
src/qemu/qemu_domain_address.c | 2 +-
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3e46f3ced3..a59583fb75 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1894,7 +1894,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
if (qemuCheckDiskConfig(disk, qemuCaps) < 0)
goto error;
- if (!qemuDomainCheckCCWS390AddressSupport(def, disk->info, qemuCaps,
disk->dst))
+ if (!qemuDomainCheckCCWS390AddressSupport(def, &disk->info, qemuCaps,
disk->dst))
goto error;
if (disk->iothread && !qemuCheckIOThreads(def, disk))
@@ -5961,7 +5961,7 @@ qemuBuildRNGDevStr(const virDomainDef *def,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (!qemuDomainCheckCCWS390AddressSupport(def, dev->info, qemuCaps,
+ if (!qemuDomainCheckCCWS390AddressSupport(def, &dev->info, qemuCaps,
dev->source.file))
goto error;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 63879e3e4c..6a78c023c8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5807,7 +5807,7 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef
*controller,
{
int ret = 0;
- if (!qemuDomainCheckCCWS390AddressSupport(def, controller->info, qemuCaps,
+ if (!qemuDomainCheckCCWS390AddressSupport(def, &controller->info, qemuCaps,
"controller"))
return -1;
@@ -5861,7 +5861,7 @@ qemuDomainDeviceDefValidateVsock(const virDomainVsockDef *vsock,
return -1;
}
- if (!qemuDomainCheckCCWS390AddressSupport(def, vsock->info, qemuCaps,
+ if (!qemuDomainCheckCCWS390AddressSupport(def, &vsock->info, qemuCaps,
"vsock"))
return -1;
@@ -13474,11 +13474,11 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
*/
bool
qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def,
- virDomainDeviceInfo info,
+ const virDomainDeviceInfo *info,
virQEMUCapsPtr qemuCaps,
const char *devicename)
{
- if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
if (!qemuDomainIsS390CCW(def)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("cannot use CCW address type for device "
@@ -13491,7 +13491,7 @@ qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def,
"this QEMU"));
return false;
}
- } else if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
+ } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtio S390 address type is not supported by "
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index defbffbf94..462da8224d 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1064,10 +1064,11 @@ int
qemuDomainObjPrivateXMLParseAllowReboot(xmlXPathContextPtr ctxt,
virTristateBool *allowReboot);
-bool qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def,
- virDomainDeviceInfo info,
- virQEMUCapsPtr qemuCaps,
- const char *devicename);
+bool
+qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def,
+ const virDomainDeviceInfo *info,
+ virQEMUCapsPtr qemuCaps,
+ const char *devicename);
int
qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk,
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index d50744a952..c376f3f897 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -3226,7 +3226,7 @@ qemuDomainEnsureVirtioAddress(bool *releaseAddr,
else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390))
info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
} else {
- if (!qemuDomainCheckCCWS390AddressSupport(vm->def, *info, priv->qemuCaps,
+ if (!qemuDomainCheckCCWS390AddressSupport(vm->def, info, priv->qemuCaps,
devicename))
return -1;
}
--
2.20.1
3:11 p.m.
New subject: [libvirt] [PATCH 4/6] qemu: pass virDomainDeviceInfo by reference
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
The virDomainDeviceInfo parameter is a large struct so it is
preferrable
to pass it by reference instead of by value.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_domain.c | 10 +++++-----
src/qemu/qemu_domain.h | 9 +++++----
src/qemu/qemu_domain_address.c | 2 +-
4 files changed, 13 insertions(+), 12 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:40 a.m.
New subject: [libvirt] [PATCH 5/6] hyperv: remove unused 'total' variable
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/hyperv/hyperv_wmi_generator.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py
index 518a55fd6d..fc1370955f 100755
--- a/src/hyperv/hyperv_wmi_generator.py
+++ b/src/hyperv/hyperv_wmi_generator.py
@@ -280,7 +280,6 @@ class WmiClass:
# alter each version's property list so that common members are first
# and in the same order as in the common dictionary
- total = len(common)
for cls in self.versions:
index = 0
count = len(cls.properties)
--
2.20.1
3:11 p.m.
New subject: [libvirt] [PATCH 5/6] hyperv: remove unused 'total' variable
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/hyperv/hyperv_wmi_generator.py | 1 -
1 file changed, 1 deletion(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:40 a.m.
New subject: [libvirt] [PATCH 6/6] hyperv: use "is None" not "== None" for PEP-8 compliance
PEP 8 says:
"Comparisons to singletons like None should always be done
with 'is' or 'is not', never the equality operators."
There are potentially semantics differences, though in the case of this
libvirt code its merely a style change:
http://jaredgrubb.blogspot.com/2009/04/python-is-none-vs-none.html
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/hyperv/hyperv_wmi_generator.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py
index fc1370955f..a9ece0ff00 100755
--- a/src/hyperv/hyperv_wmi_generator.py
+++ b/src/hyperv/hyperv_wmi_generator.py
@@ -65,7 +65,7 @@ class WmiClass:
# because we'll generate "common" member and will be the
"base" name
if len(self.versions) > 1:
first = self.versions[0]
- if first.version == None:
+ if first.version is None:
first.version = "v1"
first.name = "%s_%s" % (first.name, first.version)
--
2.20.1
3:12 p.m.
New subject: [libvirt] [PATCH 6/6] hyperv: use "is None" not "== None" for PEP-8 compliance
On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
PEP 8 says:
"Comparisons to singletons like None should always be done
with 'is' or 'is not', never the equality operators."
There are potentially semantics differences, though in the case of this
libvirt code its merely a style change:
http://jaredgrubb.blogspot.com/2009/04/python-is-none-vs-none.html
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/hyperv/hyperv_wmi_generator.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2122
days inactive
2123
days old
13 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
John Ferlan