11:26 a.m.
Previous commits to eliminate gnulib modules accidentally
used some glib APIs not available in Debian 9.
GLib has support for compile time version checking of
APIs but we mistakenly havent enabled it in libvirt.
This series fixes the bad API usage, then enables
version checkihng, and finally adds Debian to the
Travis config
Daniel P. Berrangé (7):
util: always replace g_fsync usage with our wrapper
util: keep glib compat methods in alphabetical order
util: fix indent depth for glib compat impls
util: add compat impl of g_canonicalize_filename
src: remove use of g_date_time_new_from_iso8601 function
util: introduce compile time API version checking
travis: add build for Debian 9
.travis.yml | 7 ++
config-post.h | 10 ++
src/conf/domain_conf.c | 28 ++++--
src/esx/esx_vi_types.c | 56 +++++++++--
src/libvirt_private.syms | 1 +
src/util/glibcompat.c | 196 ++++++++++++++++++++++++++++++++++-----
src/util/glibcompat.h | 10 +-
src/vz/vz_sdk.c | 24 ++++-
8 files changed, 286 insertions(+), 46 deletions(-)
--
2.24.1
11:26 a.m.
New subject: [libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper
g_fsync was introduced in 2.63 which is newer than our minimum
glib version. A future commit will introduce compile time
checking of API versions to prevent accidental usage of APIs
from glib newer than our min declared.
To avoid triggering this warning, however, we need to ensure
that we always use our wrapper function via glibcompat.c,
which will disable the API version warnings.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/glibcompat.c | 1 +
src/util/glibcompat.h | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 4ebefb4478..9fba54cb79 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -55,6 +55,7 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
}
+/* Drop when min glib >= 2.63.0 */
gint
vir_g_fsync(gint fd)
{
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index d6b83f4b93..7878ad24ed 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -32,6 +32,5 @@ gint vir_g_fsync(gint fd);
# define g_strdup_vprintf vir_g_strdup_vprintf
#endif
-#if !GLIB_CHECK_VERSION(2, 63, 0)
-# define g_fsync vir_g_fsync
-#endif
+#undef g_fsync
+#define g_fsync vir_g_fsync
--
2.24.1
2:08 a.m.
New subject: [libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper
On Mon, Jan 06, 2020 at 05:26:49PM +0000, Daniel P. Berrangé wrote:
g_fsync was introduced in 2.63 which is newer than our minimum
glib version. A future commit will introduce compile time
checking of API versions to prevent accidental usage of APIs
from glib newer than our min declared.
To avoid triggering this warning, however, we need to ensure
that we always use our wrapper function via glibcompat.c,
which will disable the API version warnings.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/glibcompat.c | 1 +
src/util/glibcompat.h | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 4ebefb4478..9fba54cb79 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -55,6 +55,7 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
}
+/* Drop when min glib >= 2.63.0 */
These "drop when something >= version" are usually overlooked and stick
around longer then necessary, should we add a check that would produce
compile time warning if we bump the minimal version of glib?
Pavel
3:48 a.m.
New subject: [libvirt] [PATCH 1/7] util: always replace g_fsync usage with our wrapper
On Tue, Jan 07, 2020 at 09:08:03AM +0100, Pavel Hrdina wrote:
On Mon, Jan 06, 2020 at 05:26:49PM +0000, Daniel P. Berrangé wrote:
> g_fsync was introduced in 2.63 which is newer than our minimum
> glib version. A future commit will introduce compile time
> checking of API versions to prevent accidental usage of APIs
> from glib newer than our min declared.
>
> To avoid triggering this warning, however, we need to ensure
> that we always use our wrapper function via glibcompat.c,
> which will disable the API version warnings.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/util/glibcompat.c | 1 +
> src/util/glibcompat.h | 5 ++---
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
> index 4ebefb4478..9fba54cb79 100644
> --- a/src/util/glibcompat.c
> +++ b/src/util/glibcompat.c
> @@ -55,6 +55,7 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
> }
>
>
> +/* Drop when min glib >= 2.63.0 */
These "drop when something >= version" are usually overlooked and stick
around longer then necessary, should we add a check that would produce
compile time warning if we bump the minimal version of glib?
I don't think that's a real problem with the way we do things here. The
key value of having a glibcompat.{c,h} file is that it isolates the
conditional logic in one place, so when bumping min glib we only need
scan the glibcompat.{c,h} files for things to eliminate. So chance of
missing this is minimal.
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:26 a.m.
New subject: [libvirt] [PATCH 2/7] util: keep glib compat methods in alphabetical order
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/glibcompat.c | 26 +++++++++++++-------------
src/util/glibcompat.h | 2 +-
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 9fba54cb79..e19cf74e72 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -23,9 +23,21 @@
#include "glibcompat.h"
+#undef g_fsync
#undef g_strdup_printf
#undef g_strdup_vprintf
-#undef g_fsync
+
+
+gint
+vir_g_fsync(gint fd)
+{
+#ifdef G_OS_WIN32
+ return _commit(fd);
+#else
+ return fsync(fd);
+#endif
+}
+
/* Due to a bug in glib, g_strdup_printf() nor g_strdup_vprintf()
* abort on OOM. It's fixed in glib's upstream. Provide our own
@@ -53,15 +65,3 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
abort();
return ret;
}
-
-
-/* Drop when min glib >= 2.63.0 */
-gint
-vir_g_fsync(gint fd)
-{
-#ifdef G_OS_WIN32
- return _commit(fd);
-#else
- return fsync(fd);
-#endif
-}
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index 7878ad24ed..ce31a4de04 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -21,11 +21,11 @@
#include <glib.h>
#include <glib/gstdio.h>
+gint vir_g_fsync(gint fd);
char *vir_g_strdup_printf(const char *msg, ...)
G_GNUC_PRINTF(1, 2);
char *vir_g_strdup_vprintf(const char *msg, va_list args)
G_GNUC_PRINTF(1, 0);
-gint vir_g_fsync(gint fd);
#if !GLIB_CHECK_VERSION(2, 64, 0)
# define g_strdup_printf vir_g_strdup_printf
--
2.24.1
2:09 a.m.
New subject: [libvirt] [PATCH 2/7] util: keep glib compat methods in alphabetical order
On Mon, Jan 06, 2020 at 05:26:50PM +0000, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/glibcompat.c | 26 +++++++++++++-------------
src/util/glibcompat.h | 2 +-
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 9fba54cb79..e19cf74e72 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -23,9 +23,21 @@
#include "glibcompat.h"
+#undef g_fsync
#undef g_strdup_printf
#undef g_strdup_vprintf
-#undef g_fsync
+
+
Missing comment when this function should be dropped.
+gint
+vir_g_fsync(gint fd)
+{
+#ifdef G_OS_WIN32
+ return _commit(fd);
+#else
+ return fsync(fd);
+#endif
+}
+
/* Due to a bug in glib, g_strdup_printf() nor g_strdup_vprintf()
* abort on OOM. It's fixed in glib's upstream. Provide our own
@@ -53,15 +65,3 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
abort();
return ret;
}
-
-
-/* Drop when min glib >= 2.63.0 */
-gint
-vir_g_fsync(gint fd)
-{
-#ifdef G_OS_WIN32
- return _commit(fd);
-#else
- return fsync(fd);
-#endif
-}
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index 7878ad24ed..ce31a4de04 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -21,11 +21,11 @@
#include <glib.h>
#include <glib/gstdio.h>
+gint vir_g_fsync(gint fd);
char *vir_g_strdup_printf(const char *msg, ...)
G_GNUC_PRINTF(1, 2);
char *vir_g_strdup_vprintf(const char *msg, va_list args)
G_GNUC_PRINTF(1, 0);
-gint vir_g_fsync(gint fd);
#if !GLIB_CHECK_VERSION(2, 64, 0)
# define g_strdup_printf vir_g_strdup_printf
--
2.24.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
11:26 a.m.
New subject: [libvirt] [PATCH 3/7] util: fix indent depth for glib compat impls
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/glibcompat.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index e19cf74e72..42f9353c8f 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -32,9 +32,9 @@ gint
vir_g_fsync(gint fd)
{
#ifdef G_OS_WIN32
- return _commit(fd);
+ return _commit(fd);
#else
- return fsync(fd);
+ return fsync(fd);
#endif
}
@@ -45,23 +45,23 @@ vir_g_fsync(gint fd)
char *
vir_g_strdup_printf(const char *msg, ...)
{
- va_list args;
- char *ret;
- va_start(args, msg);
- ret = g_strdup_vprintf(msg, args);
- if (!ret)
- abort();
- va_end(args);
- return ret;
+ va_list args;
+ char *ret;
+ va_start(args, msg);
+ ret = g_strdup_vprintf(msg, args);
+ if (!ret)
+ abort();
+ va_end(args);
+ return ret;
}
char *
vir_g_strdup_vprintf(const char *msg, va_list args)
{
- char *ret;
- ret = g_strdup_vprintf(msg, args);
- if (!ret)
- abort();
- return ret;
+ char *ret;
+ ret = g_strdup_vprintf(msg, args);
+ if (!ret)
+ abort();
+ return ret;
}
--
2.24.1
11:26 a.m.
New subject: [libvirt] [PATCH 4/7] util: add compat impl of g_canonicalize_filename
g_canonicalize_filename was not introduced until glib 2.58
so we need a temporary backport of its impl.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/glibcompat.c | 106 +++++++++++++++++++++++++++++++++++++++
src/util/glibcompat.h | 3 ++
3 files changed, 110 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 951ba7f0ca..bb2b461cea 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1504,6 +1504,7 @@ virSecurityManagerVerify;
# util/glibcompat.h
+vir_g_canonicalize_filename;
vir_g_fsync;
vir_g_strdup_printf;
vir_g_strdup_vprintf;
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index 42f9353c8f..c6390c5c2e 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -19,15 +19,121 @@
#include <config.h>
#include <stdlib.h>
+#include <string.h>
#include <unistd.h>
#include "glibcompat.h"
+#undef g_canonicalize_filename
#undef g_fsync
#undef g_strdup_printf
#undef g_strdup_vprintf
+gchar *
+vir_g_canonicalize_filename(const gchar *filename,
+ const gchar *relative_to)
+{
+#if GLIB_CHECK_VERSION(2, 58, 0)
+ return g_canonicalize_filename(filename, relative_to);
+#else /* ! GLIB_CHECK_VERSION(2, 58, 0) */
+ gchar *canon, *start, *p, *q;
+ guint i;
+
+ g_return_val_if_fail(relative_to == NULL || g_path_is_absolute(relative_to), NULL);
+
+ if (!g_path_is_absolute(filename)) {
+ gchar *cwd_allocated = NULL;
+ const gchar *cwd;
+
+ if (relative_to != NULL)
+ cwd = relative_to;
+ else
+ cwd = cwd_allocated = g_get_current_dir();
+
+ canon = g_build_filename(cwd, filename, NULL);
+ g_free(cwd_allocated);
+ } else {
+ canon = g_strdup(filename);
+ }
+
+ start = (char *)g_path_skip_root(canon);
+
+ if (start == NULL) {
+ /* This shouldn't really happen, as g_get_current_dir() should
+ return an absolute pathname, but bug 573843 shows this is
+ not always happening */
+ g_free(canon);
+ return g_build_filename(G_DIR_SEPARATOR_S, filename, NULL);
+ }
+
+ /* POSIX allows double slashes at the start to
+ * mean something special (as does windows too).
+ * So, "//" != "/", but more than two slashes
+ * is treated as "/".
+ */
+ i = 0;
+ for (p = start - 1;
+ (p >= canon) &&
+ G_IS_DIR_SEPARATOR(*p);
+ p--)
+ i++;
+ if (i > 2) {
+ i -= 1;
+ start -= i;
+ memmove(start, start+i, strlen(start+i) + 1);
+ }
+
+ /* Make sure we're using the canonical dir separator */
+ p++;
+ while (p < start && G_IS_DIR_SEPARATOR(*p))
+ *p++ = G_DIR_SEPARATOR;
+
+ p = start;
+ while (*p != 0) {
+ if (p[0] == '.' && (p[1] == 0 || G_IS_DIR_SEPARATOR(p[1]))) {
+ memmove(p, p+1, strlen(p+1)+1);
+ } else if (p[0] == '.' && p[1] == '.' &&
+ (p[2] == 0 || G_IS_DIR_SEPARATOR(p[2]))) {
+ q = p + 2;
+ /* Skip previous separator */
+ p = p - 2;
+ if (p < start)
+ p = start;
+ while (p > start && !G_IS_DIR_SEPARATOR(*p))
+ p--;
+ if (G_IS_DIR_SEPARATOR(*p))
+ *p++ = G_DIR_SEPARATOR;
+ memmove(p, q, strlen(q)+1);
+ } else {
+ /* Skip until next separator */
+ while (*p != 0 && !G_IS_DIR_SEPARATOR(*p))
+ p++;
+
+ if (*p != 0) {
+ /* Canonicalize one separator */
+ *p++ = G_DIR_SEPARATOR;
+ }
+ }
+
+ /* Remove additional separators */
+ q = p;
+ while (*q && G_IS_DIR_SEPARATOR(*q))
+ q++;
+
+ if (p != q)
+ memmove(p, q, strlen(q) + 1);
+ }
+
+ /* Remove trailing slashes */
+ if (p > start && G_IS_DIR_SEPARATOR(*(p-1)))
+ *(p-1) = 0;
+
+ return canon;
+#endif /* ! GLIB_CHECK_VERSION(2, 58, 0) */
+}
+
+
gint
vir_g_fsync(gint fd)
{
diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index ce31a4de04..6f50a76f3c 100644
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -21,6 +21,8 @@
#include <glib.h>
#include <glib/gstdio.h>
+gchar * vir_g_canonicalize_filename(const gchar *filename,
+ const gchar *relative_to);
gint vir_g_fsync(gint fd);
char *vir_g_strdup_printf(const char *msg, ...)
G_GNUC_PRINTF(1, 2);
@@ -32,5 +34,6 @@ char *vir_g_strdup_vprintf(const char *msg, va_list args)
# define g_strdup_vprintf vir_g_strdup_vprintf
#endif
+#define g_canonicalize_filename vir_g_canonicalize_filename
#undef g_fsync
#define g_fsync vir_g_fsync
--
2.24.1
11:26 a.m.
New subject: [libvirt] [PATCH 5/7] src: remove use of g_date_time_new_from_iso8601 function
The g_date_time_new_from_iso8601() function was introduced as
a replacement for strptime in
commit 810613a60efe3924c536b3663246900bc08910a5
Author: Daniel P. Berrangé <berrange(a)redhat.com>
Date: Mon Dec 23 15:37:26 2019 +0000
src: replace strptime()/timegm()/mktime() with GDateTime APIs set
Unfortunately g_date_time_new_from_iso8601 isn't available until
glib 2.56, and backporting it requires alot of code copying and
poking at private glib structs.
This reverts domain_conf.c back to its original parsing logic prior
to 810613a60efe3924c536b3663246900bc08910a5, but using g_date_time_new()
instead of gmtime(). The other files are then adapted to follow a
similar approach.
---
src/conf/domain_conf.c | 28 ++++++++++++++++-----
src/esx/esx_vi_types.c | 56 +++++++++++++++++++++++++++++++++++++-----
src/vz/vz_sdk.c | 24 ++++++++++++++----
3 files changed, 91 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ee33b7caf0..69967d4cb7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13675,15 +13675,31 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node,
if (validTo) {
g_autoptr(GDateTime) then = NULL;
g_autoptr(GTimeZone) tz = g_time_zone_new_utc();
-
- then = g_date_time_new_from_iso8601(validTo, tz);
- if (!then) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("password validity time '%s' values out of
range"), validTo);
+ char *tmp;
+ int year, mon, mday, hour, min, sec;
+
+ /* Expect: YYYY-MM-DDTHH:MM:SS (%d-%d-%dT%d:%d:%d) eg 2010-11-28T14:29:01 */
+ if (/* year */
+ virStrToLong_i(validTo, &tmp, 10, &year) < 0 || *tmp !=
'-' ||
+ /* month */
+ virStrToLong_i(tmp+1, &tmp, 10, &mon) < 0 || *tmp != '-'
||
+ /* day */
+ virStrToLong_i(tmp+1, &tmp, 10, &mday) < 0 || *tmp != 'T'
||
+ /* hour */
+ virStrToLong_i(tmp+1, &tmp, 10, &hour) < 0 || *tmp != ':'
||
+ /* minute */
+ virStrToLong_i(tmp+1, &tmp, 10, &min) < 0 || *tmp != ':'
||
+ /* second */
+ virStrToLong_i(tmp+1, &tmp, 10, &sec) < 0 || *tmp != '\0')
{
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse password validity time '%s',
expect YYYY-MM-DDTHH:MM:SS"),
+ validTo);
+ VIR_FREE(def->passwd);
return -1;
}
- def->validTo = (int)g_date_time_to_unix(then);
+ then = g_date_time_new(tz, year, mon, mday, hour, min, sec);
+ def->validTo = (time_t)g_date_time_to_unix(then);
def->expires = true;
}
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 434313dfa4..ad40ddf54b 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -1473,8 +1473,10 @@ int
esxVI_DateTime_ConvertToCalendarTime(esxVI_DateTime *dateTime,
long long *secondsSinceEpoch)
{
+ char *tmp;
g_autoptr(GDateTime) then = NULL;
g_autoptr(GTimeZone) tz = NULL;
+ int year, mon, mday, hour, min, sec, milliseconds;
if (!dateTime || !secondsSinceEpoch) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid
argument"));
@@ -1489,22 +1491,64 @@ esxVI_DateTime_ConvertToCalendarTime(esxVI_DateTime *dateTime,
*
* map negative years to 0, since the base for time_t is the year 1970.
*/
- if (*(dateTime->value) == '-') {
+ if (dateTime->value[0] == '-') {
*secondsSinceEpoch = 0;
return 0;
}
- tz = g_time_zone_new_utc();
- then = g_date_time_new_from_iso8601(dateTime->value, tz);
-
- if (!then) {
+ if (/* year */
+ virStrToLong_i(dateTime->value, &tmp, 10, &year) < 0 || *tmp !=
'-' ||
+ /* month */
+ virStrToLong_i(tmp+1, &tmp, 10, &mon) < 0 || *tmp != '-' ||
+ /* day */
+ virStrToLong_i(tmp+1, &tmp, 10, &mday) < 0 || *tmp != 'T' ||
+ /* hour */
+ virStrToLong_i(tmp+1, &tmp, 10, &hour) < 0 || *tmp != ':' ||
+ /* minute */
+ virStrToLong_i(tmp+1, &tmp, 10, &min) < 0 || *tmp != ':' ||
+ /* second */
+ virStrToLong_i(tmp+1, &tmp, 10, &sec) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("xsd:dateTime value '%s' has unexpected
format"),
dateTime->value);
return -1;
}
- *secondsSinceEpoch = g_date_time_to_unix(then);
+ if (*tmp != '\0') {
+ /* skip .ssssss part if present */
+ if (*tmp == '.' &&
+ virStrToLong_i(tmp + 1, &tmp, 10, &milliseconds) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("xsd:dateTime value '%s' has unexpected
format"),
+ dateTime->value);
+ return -1;
+ }
+
+ /* parse timezone offset if present. if missing assume UTC */
+ if (*tmp == '+' || *tmp == '-') {
+ tz = g_time_zone_new(tmp);
+ } else if (STREQ(tmp, "Z")) {
+ tz = g_time_zone_new_utc();
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("xsd:dateTime value '%s' has unexpected
format"),
+ dateTime->value);
+ return -1;
+ }
+ } else {
+ tz = g_time_zone_new_utc();
+ }
+
+ /*
+ * xsd:dateTime represents local time relative to the optional timezone
+ * given as offset. pretend the local time is in UTC and use timegm in
+ * order to avoid interference with the timezone to this computer.
+ * apply timezone correction afterwards, because it's simpler than
+ * handling all the possible over- and underflows when trying to apply
+ * it to the tm struct.
+ */
+ then = g_date_time_new(tz, year, mon, mday, hour, min, sec);
+ *secondsSinceEpoch = (long long)g_date_time_to_unix(then);
return 0;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index c98542c244..26e9e38729 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4608,16 +4608,30 @@ static long long
prlsdkParseDateTime(const char *str)
{
g_autoptr(GDateTime) then = NULL;
- g_autoptr(GTimeZone) tz = g_time_zone_new_local();
-
- then = g_date_time_new_from_iso8601(str, tz);
- if (!then) {
+ g_autoptr(GTimeZone) tz = g_time_zone_new_utc();
+ char *tmp;
+ int year, mon, mday, hour, min, sec;
+
+ /* Expect: YYYY-MM-DD HH:MM:SS (%d-%d-%dT%d:%d:%d) eg 2010-11-28 14:29:01 */
+ if (/* year */
+ virStrToLong_i(str, &tmp, 10, &year) < 0 || *tmp != '-' ||
+ /* month */
+ virStrToLong_i(tmp+1, &tmp, 10, &mon) < 0 || *tmp != '-' ||
+ /* day */
+ virStrToLong_i(tmp+1, &tmp, 10, &mday) < 0 || *tmp != ' ' ||
+ /* hour */
+ virStrToLong_i(tmp+1, &tmp, 10, &hour) < 0 || *tmp != ':' ||
+ /* minute */
+ virStrToLong_i(tmp+1, &tmp, 10, &min) < 0 || *tmp != ':' ||
+ /* second */
+ virStrToLong_i(tmp+1, &tmp, 10, &sec) < 0 || *tmp != '\0') {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected DateTime format: '%s'"), str);
return -1;
}
- return g_date_time_to_unix(then);
+ then = g_date_time_new(tz, year, mon, mday, hour, min, sec);
+ return (long long)g_date_time_to_unix(then);
}
static virDomainSnapshotObjListPtr
--
2.24.1
11:26 a.m.
New subject: [libvirt] [PATCH 6/7] util: introduce compile time API version checking
GLib header files annotate every API with a version number.
It is possible to define some constants before including
glib.h which will result in useful compile time warnings.
Setting GLIB_VERSION_MIN_REQUIRED will result in a warning
if libvirt uses an API that was deprecated in the declared
version, or before. Such API usage should be rewritten to
use the documented new replacement API.
Setting GLIB_VERSION_MAX_ALLOWED will result in a warning
if libvirt uses an API that was not introduced until a
version of GLib that's newer than our minimum declared
version. This avoids accidentally using functionality
that is not available on some supported platforms.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
config-post.h | 10 ++++++++++
src/util/glibcompat.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/config-post.h b/config-post.h
index 415cc8cc72..de007393da 100644
--- a/config-post.h
+++ b/config-post.h
@@ -49,3 +49,13 @@
#else
# error You either need at least GCC 4.8 or Clang 3.4 or XCode Clang 5.1 to compile
libvirt
#endif
+
+/* Ask for warnings for anything that was marked deprecated in
+ * the defined version, or before. It is a candidate for rewrite.
+ */
+#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_48
+
+/* Ask for warnings if code tries to use function that did not
+ * exist in the defined version. These risk breaking builds
+ */
+#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_48
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index c6390c5c2e..06fe5b3d33 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -24,6 +24,45 @@
#include "glibcompat.h"
+/*
+ * Note that because of the GLIB_VERSION_MAX_ALLOWED constant in
+ * config-post.h, allowing use of functions from newer GLib via
+ * this compat impl needs a little trickery to prevent warnings
+ * being emitted.
+ *
+ * Consider a function from newer glib-X.Y that we want to use
+ *
+ * int g_foo(const char *wibble)
+ *
+ * We must define a function with the same signature that does
+ * what we need, but with a "vir_" prefix e.g.
+ *
+ * void vir_g_foo(const char *wibble)
+ * {
+ * #if GLIB_CHECK_VERSION(X, Y, 0)
+ * g_foo(wibble)
+ * #else
+ * g_something_equivalent_in_older_glib(wibble);
+ * #endif
+ * }
+ *
+ * The #pragma at the top of this file turns off -Wdeprecated-declarations,
+ * ensuring this wrapper function impl doesn't trigger the compiler
+ * warning about using too new glib APIs. Finally in glibcompat.hu we can
+ * add
+ *
+ * #define g_foo(a) vir_g_foo(a)
+ *
+ * Thus all the code elsewhere in libvirt, which *does* have the
+ * -Wdeprecated-declarations warning active, can call g_foo(...) as
+ * normal, without generating warnings. The cost is an extra function
+ * call when using new glib, but this compat code will go away over
+ * time as we update the supported platforms target.
+ */
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+
#undef g_canonicalize_filename
#undef g_fsync
#undef g_strdup_printf
--
2.24.1
1:56 p.m.
New subject: [libvirt] [PATCH 6/7] util: introduce compile time API version checking
Just a small nit:
On 1/6/20 2:26 PM, Daniel P. Berrangé wrote:
GLib header files annotate every API with a version number.
It is possible to define some constants before including
glib.h which will result in useful compile time warnings.
Setting GLIB_VERSION_MIN_REQUIRED will result in a warning
if libvirt uses an API that was deprecated in the declared
version, or before. Such API usage should be rewritten to
use the documented new replacement API.
Setting GLIB_VERSION_MAX_ALLOWED will result in a warning
if libvirt uses an API that was not introduced until a
version of GLib that's newer than our minimum declared
version. This avoids accidentally using functionality
that is not available on some supported platforms.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
config-post.h | 10 ++++++++++
src/util/glibcompat.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/config-post.h b/config-post.h
index 415cc8cc72..de007393da 100644
--- a/config-post.h
+++ b/config-post.h
@@ -49,3 +49,13 @@
#else
# error You either need at least GCC 4.8 or Clang 3.4 or XCode Clang 5.1 to compile
libvirt
#endif
+
+/* Ask for warnings for anything that was marked deprecated in
+ * the defined version, or before. It is a candidate for rewrite.
+ */
+#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_48
+
+/* Ask for warnings if code tries to use function that did not
+ * exist in the defined version. These risk breaking builds
+ */
+#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_48
diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index c6390c5c2e..06fe5b3d33 100644
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -24,6 +24,45 @@
#include "glibcompat.h"
+/*
+ * Note that because of the GLIB_VERSION_MAX_ALLOWED constant in
+ * config-post.h, allowing use of functions from newer GLib via
+ * this compat impl needs a little trickery to prevent warnings
+ * being emitted.
+ *
+ * Consider a function from newer glib-X.Y that we want to use
+ *
+ * int g_foo(const char *wibble)
+ *
+ * We must define a function with the same signature that does
+ * what we need, but with a "vir_" prefix e.g.
+ *
+ * void vir_g_foo(const char *wibble)
+ * {
+ * #if GLIB_CHECK_VERSION(X, Y, 0)
+ * g_foo(wibble)
+ * #else
+ * g_something_equivalent_in_older_glib(wibble);
+ * #endif
+ * }
+ *
+ * The #pragma at the top of this file turns off -Wdeprecated-declarations,
+ * ensuring this wrapper function impl doesn't trigger the compiler
+ * warning about using too new glib APIs. Finally in glibcompat.hu we can
s/glibcompat.hu/glibcompat.h
+ * add
+ *
+ * #define g_foo(a) vir_g_foo(a)
+ *
+ * Thus all the code elsewhere in libvirt, which *does* have the
+ * -Wdeprecated-declarations warning active, can call g_foo(...) as
+ * normal, without generating warnings. The cost is an extra function
+ * call when using new glib, but this compat code will go away over
+ * time as we update the supported platforms target.
+ */
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+
#undef g_canonicalize_filename
#undef g_fsync
#undef g_strdup_printf
11:26 a.m.
New subject: [libvirt] [PATCH 7/7] travis: add build for Debian 9
Debian 9 ships the oldest versions of some of our dependent
packages so can highlight bugs not seen elsewhere. CentOS 7,
despite being quite old, has rebased some packages to much
newer versions.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
.travis.yml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/.travis.yml b/.travis.yml
index e1e8cb4f80..b243e3d5c4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -32,6 +32,13 @@ matrix:
- MAKE_ARGS="syntax-check distcheck"
script:
- make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS"
+ - services:
+ - docker
+ env:
+ - IMAGE="debian-9"
+ - MAKE_ARGS="syntax-check distcheck"
+ script:
+ - make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS"
- services:
- docker
env:
--
2.24.1
1:56 p.m.
On 1/6/20 2:26 PM, Daniel P. Berrangé wrote:
Previous commits to eliminate gnulib modules accidentally
used some glib APIs not available in Debian 9.
GLib has support for compile time version checking of
APIs but we mistakenly havent enabled it in libvirt.
This series fixes the bad API usage, then enables
version checkihng, and finally adds Debian to the
Travis config
Daniel P. Berrangé (7):
util: always replace g_fsync usage with our wrapper
util: keep glib compat methods in alphabetical order
util: fix indent depth for glib compat impls
util: add compat impl of g_canonicalize_filename
src: remove use of g_date_time_new_from_iso8601 function
util: introduce compile time API version checking
travis: add build for Debian 9
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
.travis.yml | 7 ++
config-post.h | 10 ++
src/conf/domain_conf.c | 28 ++++--
src/esx/esx_vi_types.c | 56 +++++++++--
src/libvirt_private.syms | 1 +
src/util/glibcompat.c | 196 ++++++++++++++++++++++++++++++++++-----
src/util/glibcompat.h | 10 +-
src/vz/vz_sdk.c | 24 ++++-
8 files changed, 286 insertions(+), 46 deletions(-)
2:16 a.m.
On Mon, Jan 06, 2020 at 05:26:48PM +0000, Daniel P. Berrangé wrote:
Previous commits to eliminate gnulib modules accidentally
used some glib APIs not available in Debian 9.
GLib has support for compile time version checking of
APIs but we mistakenly havent enabled it in libvirt.
This series fixes the bad API usage, then enables
version checkihng, and finally adds Debian to the
Travis config
Daniel P. Berrangé (7):
util: always replace g_fsync usage with our wrapper
util: keep glib compat methods in alphabetical order
util: fix indent depth for glib compat impls
util: add compat impl of g_canonicalize_filename
src: remove use of g_date_time_new_from_iso8601 function
util: introduce compile time API version checking
travis: add build for Debian 9
Looks good, there are only some minor issues pointed out.
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
1781
days inactive
1782
days old
13 comments
3 participants
participants (3)
-
Daniel Henrique Barboza -
Daniel P. Berrangé -
Pavel Hrdina