[libvirt] [PATCH 1/3] libvirt-domain: Introduce macro to save duplicated codes
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/libvirt-domain.c | 55 ++++++++++++++-------------------------------------- 1 file changed, 15 insertions(+), 40 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6e1aacd..ed07c9e 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -33,6 +33,13 @@ VIR_LOG_INIT("libvirt.domain"); #define VIR_FROM_THIS VIR_FROM_DOMAIN +#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH) \ + if (virFileAbsPath(PATH, ABSPATH) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("could not build absolute input file path")); \ + goto error; \ + } + /** * virConnectListDomains: @@ -830,11 +837,7 @@ virDomainSave(virDomainPtr domain, const char *to) char *absolute_to; /* We must absolutize the file path as the save is done out of process */ - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute output file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to); ret = conn->driver->domainSave(domain, absolute_to); @@ -918,11 +921,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, char *absolute_to; /* We must absolutize the file path as the save is done out of process */ - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute output file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to); ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml, flags); @@ -968,11 +967,7 @@ virDomainRestore(virConnectPtr conn, const char *from) char *absolute_from; /* We must absolutize the file path as the restore is done out of process */ - if (virFileAbsPath(from, &absolute_from) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute input file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(from, &absolute_from); ret = conn->driver->domainRestore(conn, absolute_from); @@ -1042,11 +1037,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, char *absolute_from; /* We must absolutize the file path as the restore is done out of process */ - if (virFileAbsPath(from, &absolute_from) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute input file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(from, &absolute_from); ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml, flags); @@ -1107,11 +1098,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file, char *absolute_file; /* We must absolutize the file path as the read is done out of process */ - if (virFileAbsPath(file, &absolute_file) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute input file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(file, &absolute_file); ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file, flags); @@ -1180,11 +1167,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file, char *absolute_file; /* We must absolutize the file path as the read is done out of process */ - if (virFileAbsPath(file, &absolute_file) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute input file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(file, &absolute_file); ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file, dxml, flags); @@ -1255,11 +1238,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags) char *absolute_to; /* We must absolutize the file path as the save is done out of process */ - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute core file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to); ret = conn->driver->domainCoreDump(domain, absolute_to, flags); @@ -1339,11 +1318,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to, char *absolute_to; /* We must absolutize the file path as the save is done out of process */ - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute core file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to); ret = conn->driver->domainCoreDumpWithFormat(domain, absolute_to, dumpformat, flags); -- 1.9.3
seclets ==> selects qualfied ==> qualified Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/libvirt-secret.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c index fa306e3..db42aec 100644 --- a/src/libvirt-secret.c +++ b/src/libvirt-secret.c @@ -110,7 +110,7 @@ virConnectNumOfSecrets(virConnectPtr conn) * selects secrets that are kept in persistent storage. * * The second group of @flags is used to filter secrets by privacy. Flag - * VIR_CONNECT_LIST_SECRETS_PRIVATE seclets secrets that are never revealed + * VIR_CONNECT_LIST_SECRETS_PRIVATE selects secrets that are never revealed * to any caller of libvirt nor to any other node. Flag * VIR_CONNECT_LIST_SECRETS_NO_PRIVATE selects non-private secrets. * @@ -454,7 +454,7 @@ virSecretGetUsageType(virSecretPtr secret) * secret is to be used. The format of the identifier is * dependent on the usage type of the secret. For a secret * with a usage type of VIR_SECRET_USAGE_TYPE_VOLUME the - * identifier will be a fully qualfied path name. The + * identifier will be a fully qualified path name. The * identifiers are intended to be unique within the set of * all secrets sharing the same usage type. ie, there shall * only ever be one secret for each volume path. -- 1.9.3
On Thu, Oct 15, 2015 at 17:12:19 +0800, Wei Jiangang wrote:
seclets ==> selects qualfied ==> qualified
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/libvirt-secret.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c index fa306e3..db42aec 100644 --- a/src/libvirt-secret.c +++ b/src/libvirt-secret.c @@ -110,7 +110,7 @@ virConnectNumOfSecrets(virConnectPtr conn) * selects secrets that are kept in persistent storage. * * The second group of @flags is used to filter secrets by privacy. Flag - * VIR_CONNECT_LIST_SECRETS_PRIVATE seclets secrets that are never revealed + * VIR_CONNECT_LIST_SECRETS_PRIVATE selects secrets that are never revealed * to any caller of libvirt nor to any other node. Flag * VIR_CONNECT_LIST_SECRETS_NO_PRIVATE selects non-private secrets. * @@ -454,7 +454,7 @@ virSecretGetUsageType(virSecretPtr secret) * secret is to be used. The format of the identifier is * dependent on the usage type of the secret. For a secret * with a usage type of VIR_SECRET_USAGE_TYPE_VOLUME the - * identifier will be a fully qualfied path name. The + * identifier will be a fully qualified path name. The * identifiers are intended to be unique within the set of * all secrets sharing the same usage type. ie, there shall * only ever be one secret for each volume path.
ACK and pushed. Jirka
Don't compare a bool variable against the literal, "true". Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/conf/nwfilter_conf.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 2 +- src/vz/vz_sdk.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index aed82ad..f7ccb75 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -3416,7 +3416,7 @@ virNWFilterRuleDefDetailsFormat(virBufferPtr buf, break; case DATATYPE_BOOLEAN: - if (item->u.boolean == true) + if (item->u.boolean) virBufferAddLit(buf, "true"); else virBufferAddLit(buf, "false"); diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index f331e22..f05d4a8 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1403,7 +1403,7 @@ virNWFilterDHCPSnoopThread(void *req0) } /* let creator know how well we initialized */ - if (error == true || !threadkey || tmp < 0 || !worker || + if (error || !threadkey || tmp < 0 || !worker || ifindex != req->ifindex) req->threadStatus = THREAD_STATUS_FAIL; else diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 7a2afd6..c24477c 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3293,7 +3293,7 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom, goto cleanup; } - if (bootDisk == true) { + if (bootDisk) { pret = PrlVmDev_GetIndex(sdkdisk, &devIndex); prlsdkCheckRetGoto(pret, cleanup); @@ -3551,7 +3551,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, for (i = 0; i < def->ndisks; i++) { bool bootDisk = false; - if (needBoot == true && + if (needBoot && def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { needBoot = false; -- 1.9.3
On Thu, Oct 15, 2015 at 17:12:20 +0800, Wei Jiangang wrote:
Don't compare a bool variable against the literal, "true".
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/conf/nwfilter_conf.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 2 +- src/vz/vz_sdk.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
ACK and pushed. Jirka
On Thu, Oct 15, 2015 at 17:12:18 +0800, Wei Jiangang wrote:
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/libvirt-domain.c | 55 ++++++++++++++-------------------------------------- 1 file changed, 15 insertions(+), 40 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6e1aacd..ed07c9e 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -33,6 +33,13 @@ VIR_LOG_INIT("libvirt.domain");
#define VIR_FROM_THIS VIR_FROM_DOMAIN
+#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH) \ + if (virFileAbsPath(PATH, ABSPATH) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("could not build absolute input file path")); \
This says "input" file path ...
+ goto error; \ + } +
/** * virConnectListDomains: @@ -830,11 +837,7 @@ virDomainSave(virDomainPtr domain, const char *to) char *absolute_to;
/* We must absolutize the file path as the save is done out of process */ - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute output file path"));
But this "output" ...
- goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to);
ret = conn->driver->domainSave(domain, absolute_to);
@@ -968,11 +967,7 @@ virDomainRestore(virConnectPtr conn, const char *from) char *absolute_from;
/* We must absolutize the file path as the restore is done out of process */ - if (virFileAbsPath(from, &absolute_from) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute input file path"));
While here correctly "input"
- goto error; - }
While saving code, this would actually break the error messages in some cases. Peter
On Thu, Oct 15, 2015 at 11:31:48 +0200, Peter Krempa wrote:
On Thu, Oct 15, 2015 at 17:12:18 +0800, Wei Jiangang wrote:
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/libvirt-domain.c | 55 ++++++++++++++-------------------------------------- 1 file changed, 15 insertions(+), 40 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6e1aacd..ed07c9e 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -33,6 +33,13 @@ VIR_LOG_INIT("libvirt.domain");
#define VIR_FROM_THIS VIR_FROM_DOMAIN
+#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH) \ + if (virFileAbsPath(PATH, ABSPATH) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("could not build absolute input file path")); \
This says "input" file path ...
+ goto error; \ + } ... While saving code, this would actually break the error messages in some cases.
Not to mention that the result is harder to read and easier to break (hidden goto error). I don't think it's a good idea at all. Jirka
On Thu, 2015-10-15 at 11:31 +0200, Peter Krempa wrote:
On Thu, Oct 15, 2015 at 17:12:18 +0800, Wei Jiangang wrote:
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> --- src/libvirt-domain.c | 55 ++++++++++++++-------------------------------------- 1 file changed, 15 insertions(+), 40 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 6e1aacd..ed07c9e 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -33,6 +33,13 @@ VIR_LOG_INIT("libvirt.domain");
#define VIR_FROM_THIS VIR_FROM_DOMAIN
+#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH) \ + if (virFileAbsPath(PATH, ABSPATH) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \ + _("could not build absolute input file path")); \
This says "input" file path ...
+ goto error; \ + } +
/** * virConnectListDomains: @@ -830,11 +837,7 @@ virDomainSave(virDomainPtr domain, const char *to) char *absolute_to;
/* We must absolutize the file path as the save is done out of process */ - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute output file path"));
But this "output" ...
- goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to);
ret = conn->driver->domainSave(domain, absolute_to);
@@ -968,11 +967,7 @@ virDomainRestore(virConnectPtr conn, const char *from) char *absolute_from;
/* We must absolutize the file path as the restore is done out of process */ - if (virFileAbsPath(from, &absolute_from) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute input file path"));
While here correctly "input"
- goto error; - }
While saving code, this would actually break the error messages in some cases. Thanks for your comments. If I make some adjustment like below, Is it acceptable?
+#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH, ERRSTR) \ + if (virFileAbsPath(PATH, ABSPATH) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", ERRSTR); \ + goto error; \ + } + - if (virFileAbsPath(to, &absolute_to) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not build absolute output file path")); - goto error; - } + VIR_ABSOLUTIZE_PATH(to, &absolute_to, + _("could not build absolute output file path")); Regards, wei
Peter
participants (4)
-
Jiri Denemark -
Peter Krempa -
Wei Jiangang -
Wei, Jiangang