On Wed, Sep 06, 2023 at 10:50:30AM -0500, Praveen K Paladugu wrote:
Refactor the version processing logic in ch driver to support versions
from non-release cloud-hypervisor binaries. This version also supports
versions with branch prefixes in them.
Signed-off-by: Praveen K Paladugu <prapal@xxxxxxxxxxxxxxxxxxx>
---
src/ch/ch_conf.c | 42 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index a8565d9537..5573119b23 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -172,6 +172,37 @@ virCHDriverConfigDispose(void *obj)
#define MIN_VERSION ((15 * 1000000) + (0 * 1000) + (0))
+/**
+ * chPreProcessVersionString:
+ *
+ * Returns: a pointer to numerical version without branch/commit info
+ */
+static char *
+chPreProcessVersionString(char *version)
+{
+ char *tmp_version = version;
+ g_autofree char *ret_version = NULL;
+ char *sub_string;
+
+ if ((sub_string = strrchr(version, '/')) != NULL) {
+ tmp_version = sub_string + 1;
+ }
+
+ if (tmp_version[0] == 'v') {
+ tmp_version = tmp_version + 1;
+ }
+
+ // Duplicate the string in both cases. This would allow the calling method
+ // free the returned string in a consistent manner.
+ if ((sub_string = strchr(tmp_version, '-')) != NULL) {
+ ret_version = g_strndup(tmp_version, sub_string - tmp_version);
+ } else{
+ ret_version = g_strdup(tmp_version);
+ }
+
+ return g_steal_pointer(&ret_version);
+
+}
What would be wrong with the following?
static char *
chPreProcessVersionString(const char *version)
{
const char *tmp = strrchr(version, '/');
if (tmp)
version = tmp + 1;
if (version[0] == 'v')
version++;
tmp = strchr(tmp_version, '-');
if (tmp)
return g_strndup(version, tmp - version);
else
return g_strdup(version;
}
isn't that more readable?
int
chExtractVersion(virCHDriver *driver)
{
@@ -193,13 +224,20 @@ chExtractVersion(virCHDriver *driver)
tmp = help;
- /* expected format: cloud-hypervisor v<major>.<minor>.<micro> */
- if ((tmp = STRSKIP(tmp, "cloud-hypervisor v")) == NULL) {
+ /* Below are some example version formats and expected outputs:
+ * cloud-hypervisor v32.0.0 (expected: 32.0.0)
+ * cloud-hypervisor v33.0-104-ge0e3779e-dirty (expected: 33.0)
+ * cloud-hypervisor testing/v32.0.131-1-ga5d6db5c-dirty (expected: 32.0.131)
+ */
+ if ((tmp = STRSKIP(tmp, "cloud-hypervisor ")) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unexpected output of cloud-hypervisor binary"));
return -1;
}
+ tmp = chPreProcessVersionString(tmp);
+ VIR_DEBUG("Cloud-Hypervisor version detected: %s", tmp);
+
Also tmp is not free'd in this function which introduces a leak.
But since @help is not used for anything else we could simplify the
processing of the version by not duplicating anything:
static char *
chPreProcessVersionString(char *version)
{
char *tmp = strrchr(version, '/');
if (tmp)
version = tmp + 1;
if (version[0] == 'v')
version++;
tmp = strchr(version, '-');
if (tmp)
*tmp = '\0';
return version;
}
and keep this part just as you changed it.
if (virStringParseVersion(&version, tmp, true) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse cloud-hypervisor version: %1$s"), tmp);
--
2.41.0
Attachment:
signature.asc
Description: PGP signature
