9:14 a.m.
Background
==========
We have plenty of features in libvirt, some of which were designed at
a time when the virtualization story was much more straightforward
than the multi-architecture, multi-hypervisor, multi-machine world we
currently live in and, while we have found ways to keep the APIs
chugging along, the result is sometimes somewhat confusing for users
and application developers, as well as requiring libvirt developers
themselves to spend quite a bit of collective time working around
decisions that, in hindsight, turn out to have been less than
fortunate.
Two concrete examples are considered here: one is the
virConnectNumOfDomains() API which, while known to be racy and having
non-racy alternatives, can still be used by developers without
getting any kind of warning in the process; the other one is the
ability to define a domain without specifying the machine type, which
is becoming increasingly problematic now with some x86_64 features
being limited to q35 and downstreams looking to push for its
adoption, as well as being a manifestation of the more general
problem of libvirt's default being sometimes too conservative and at
odds with the existence of slimmed-down QEMU binaries being built
with reducing the total attack surface in mind.
Having a proper deprecation story in libvirt would allow us to point
users and developers towards the recommended solution in each case,
be it using a different API or querying libosinfo for information;
after a generous grace period, we could then remove the problematic
functionality altogether. This would be a more conservative version
of the process we already have in place for dropping support for
older QEMU releases, which recently has allowed us to ax massive
chunks of effectively dead code and simplify parts of libvirt quite
significantly.
This series explores one possible approach to the problem and aims
to spark project-wide discussion around the topic.
Further work
============
* Fix the known issues listed below as well as all not-yet-known
issues that will undoubtably surface through discussion :)
* Introduce a mechanism to catch use of deprecated APIs at build
time, similar to GLib's G_DISABLE_DEPRECATED, to help application
developers proactively move off problematic APIs.
* Create a formal deprecation policy with well-defined rules and
time scales in the spirit of the existing one covering our
relationship with QEMU.
Know issues
===========
* For the more granular (and more interesting) type of deprecation
shown in patch 6/7, warnings are not being reported back to the
client as expected. I believe this is caused by the RPC code
looking for either a failure, in which case the virError is
transmitted, or a success, in which case the actual return value
is: we'll have to figure out a way for the error to travel across
the wire regardless of whether or not the API call was ultimately
successful if we want clients to actually receive warnings when
non-local drivers are involved.
Andrea Bolognani (7):
util: Add 'level' argument to virReportErrorHelper()
util: Introduce virReportWarning()
tools: Print warnings in virsh
util: Introduce VIR_ERR_DEPRECATED_FEATURE
Deprecate virConnectNumOfDomains()
Deprecate missing machine type in virDomainDefineXMLFlags()
tools: Force virsh to use deprecated features
include/libvirt/virterror.h | 1 +
src/access/viraccessdriverpolkit.c | 2 +-
src/access/viraccessmanager.c | 2 +-
src/conf/domain_conf.c | 11 ++++++++---
src/datatypes.h | 30 ++++++++++++++++++++++++++----
src/libvirt-domain.c | 6 ++++++
src/libvirt.c | 1 +
src/util/virbuffer.c | 4 ++--
src/util/virconf.c | 6 ++++--
src/util/virerror.c | 10 +++++++++-
src/util/virerror.h | 15 ++++++++++-----
src/util/virkeyfile.c | 6 ++++--
src/util/virxml.c | 2 +-
tools/virsh-domain-monitor.c | 2 ++
tools/vsh.c | 3 +++
15 files changed, 79 insertions(+), 22 deletions(-)
--
2.17.1
9:14 a.m.
New subject: [libvirt] [RFC 1/7] util: Add 'level' argument to virReportErrorHelper()
Up until now it's been assumed that every single error
reported through the helper would have VIR_ERR_ERROR level,
but this will no longer be the case going forward so we need
to make it a little more flexible.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/access/viraccessdriverpolkit.c | 2 +-
src/access/viraccessmanager.c | 2 +-
src/datatypes.h | 30 ++++++++++++++++++++++++++----
src/libvirt.c | 1 +
src/util/virbuffer.c | 4 ++--
src/util/virconf.c | 6 ++++--
src/util/virerror.c | 4 +++-
src/util/virerror.h | 11 ++++++-----
src/util/virkeyfile.c | 6 ++++--
src/util/virxml.c | 2 +-
10 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
index 6954d74a15..1f87e7e152 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -34,7 +34,7 @@
VIR_LOG_INIT("access.accessdriverpolkit");
#define virAccessError(code, ...) \
- virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ virReportErrorHelper(VIR_FROM_THIS, code, VIR_ERR_ERROR, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
#define VIR_ACCESS_DRIVER_POLKIT_ACTION_PREFIX "org.libvirt.api"
diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index e7b5bf38da..f4230b253a 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -37,7 +37,7 @@
VIR_LOG_INIT("access.accessmanager");
#define virAccessError(code, ...) \
- virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ virReportErrorHelper(VIR_FROM_THIS, code, VIR_ERR_ERROR, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
struct _virAccessManager {
diff --git a/src/datatypes.h b/src/datatypes.h
index e1b38706dc..87d6aa9a80 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -49,7 +49,7 @@ extern virClassPtr virAdmClientClass;
# define virCheckConnectReturn(obj, retval) \
do { \
if (!virObjectIsClass(obj, virConnectClass)) { \
- virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -59,7 +59,7 @@ extern virClassPtr virAdmClientClass;
# define virCheckConnectGoto(obj, label) \
do { \
if (!virObjectIsClass(obj, virConnectClass)) { \
- virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -71,7 +71,7 @@ extern virClassPtr virAdmClientClass;
virDomainPtr _dom = (obj); \
if (!virObjectIsClass(_dom, virDomainClass) || \
!virObjectIsClass(_dom->conn, virConnectClass)) { \
- virReportErrorHelper(VIR_FROM_DOM, VIR_ERR_INVALID_DOMAIN, \
+ virReportErrorHelper(VIR_FROM_DOM, VIR_ERR_INVALID_DOMAIN, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -83,7 +83,7 @@ extern virClassPtr virAdmClientClass;
virDomainPtr _dom = (obj); \
if (!virObjectIsClass(_dom, virDomainClass) || \
!virObjectIsClass(_dom->conn, virConnectClass)) { \
- virReportErrorHelper(VIR_FROM_DOM, VIR_ERR_INVALID_DOMAIN, \
+ virReportErrorHelper(VIR_FROM_DOM, VIR_ERR_INVALID_DOMAIN, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -97,6 +97,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_net->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_NETWORK, \
VIR_ERR_INVALID_NETWORK, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -110,6 +111,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_net->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_NETWORK, \
VIR_ERR_INVALID_NETWORK, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -123,6 +125,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_iface->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_INTERFACE, \
VIR_ERR_INVALID_INTERFACE, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -137,6 +140,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_pool->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_STORAGE, \
VIR_ERR_INVALID_STORAGE_POOL, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -151,6 +155,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_pool->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_STORAGE, \
VIR_ERR_INVALID_STORAGE_POOL, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -164,6 +169,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_vol->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_STORAGE, \
VIR_ERR_INVALID_STORAGE_VOL, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -177,6 +183,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_vol->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_STORAGE, \
VIR_ERR_INVALID_STORAGE_VOL, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -190,6 +197,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_node->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_NODEDEV, \
VIR_ERR_INVALID_NODE_DEVICE, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -204,6 +212,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_dev->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_NODEDEV, \
VIR_ERR_INVALID_NODE_DEVICE, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -217,6 +226,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_secret->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_SECRET, \
VIR_ERR_INVALID_SECRET, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -231,6 +241,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_secret->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_SECRET, \
VIR_ERR_INVALID_SECRET, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -245,6 +256,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_st->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_STREAMS, \
VIR_ERR_INVALID_STREAM, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -258,6 +270,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_st->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_STREAMS, \
VIR_ERR_INVALID_STREAM, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -271,6 +284,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_nw->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_NWFILTER, \
VIR_ERR_INVALID_NWFILTER, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -285,6 +299,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_nw->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_NWFILTER, \
VIR_ERR_INVALID_NWFILTER_BINDING, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -300,6 +315,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_snap->domain->conn, virConnectClass)) { \
virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, \
VIR_ERR_INVALID_DOMAIN_SNAPSHOT, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -358,6 +374,7 @@ extern virClassPtr virAdmClientClass;
do { \
if (!virObjectIsClass(obj, virAdmConnectClass)) { \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -368,6 +385,7 @@ extern virClassPtr virAdmClientClass;
do { \
if (!virObjectIsClass(obj, virAdmConnectClass)) { \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -380,6 +398,7 @@ extern virClassPtr virAdmClientClass;
if (!virObjectIsClass(_srv, virAdmServerClass) || \
!virObjectIsClass(_srv->conn, virAdmConnectClass)) { \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -392,6 +411,7 @@ extern virClassPtr virAdmClientClass;
if (!virObjectIsClass(_srv, virAdmServerClass) || \
!virObjectIsClass(_srv->conn, virAdmConnectClass)) { \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
@@ -405,6 +425,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_clt->srv, virAdmServerClass) || \
!virObjectIsClass(_clt->srv->conn, virAdmConnectClass)) { \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
virDispatchError(NULL); \
@@ -418,6 +439,7 @@ extern virClassPtr virAdmClientClass;
!virObjectIsClass(_clt->srv, virAdmServerClass) || \
!virObjectIsClass(_clt->srv->conn, virAdmConnectClass)) { \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
+ VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
__FUNCTION__); \
goto label; \
diff --git a/src/libvirt.c b/src/libvirt.c
index 0a738aefb1..3e0bb26b9f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -988,6 +988,7 @@ virConnectOpenInternal(const char *name,
#endif
false)) {
virReportErrorHelper(VIR_FROM_NONE, VIR_ERR_CONFIG_UNSUPPORTED,
+ VIR_ERR_ERROR,
__FILE__, __FUNCTION__, __LINE__,
_("libvirt was built without the '%s'
driver"),
ret->uri->scheme);
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 8a2108f2f2..6306b3f072 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -339,8 +339,8 @@ virBufferCheckErrorInternal(const virBuffer *buf,
virReportOOMErrorFull(domcode, filename, funcname, linenr);
errno = ENOMEM;
} else {
- virReportErrorHelper(domcode, VIR_ERR_INTERNAL_ERROR, filename,
- funcname, linenr, "%s",
+ virReportErrorHelper(domcode, VIR_ERR_INTERNAL_ERROR, VIR_ERR_ERROR,
+ filename, funcname, linenr, "%s",
_("Invalid buffer API usage"));
errno = EINVAL;
}
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 88a869517e..8a382e8b6e 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -110,10 +110,12 @@ virConfErrorHelper(const char *file, const char *func, size_t line,
/* Construct the string 'filename:line: info' if we have that. */
if (ctxt && ctxt->filename) {
- virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+ virReportErrorHelper(VIR_FROM_CONF, error, VIR_ERR_ERROR,
+ file, func, line,
_("%s:%d: %s"), ctxt->filename, ctxt->line,
info);
} else {
- virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+ virReportErrorHelper(VIR_FROM_CONF, error, VIR_ERR_ERROR,
+ file, func, line,
"%s", info);
}
}
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 683e51aa19..517e0bde28 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1518,6 +1518,7 @@ virErrorMsg(virErrorNumber error, const char *info)
*
* @domcode: the virErrorDomain indicating where it's coming from
* @errorcode: the virErrorNumber code for the error
+ * @level: the virErrorLevel level for the error
* @filename: Source file error is dispatched from
* @funcname: Function error is dispatched from
* @linenr: Line number error is dispatched from
@@ -1529,6 +1530,7 @@ virErrorMsg(virErrorNumber error, const char *info)
*/
void virReportErrorHelper(int domcode,
int errorcode,
+ virErrorLevel level,
const char *filename,
const char *funcname,
size_t linenr,
@@ -1549,7 +1551,7 @@ void virReportErrorHelper(int domcode,
virerr = virErrorMsg(errorcode, (errorMessage[0] ? errorMessage : NULL));
virRaiseErrorFull(filename, funcname, linenr,
- domcode, errorcode, VIR_ERR_ERROR,
+ domcode, errorcode, level,
virerr, errorMessage, NULL,
-1, -1, virerr, errorMessage);
errno = save_errno;
diff --git a/src/util/virerror.h b/src/util/virerror.h
index 31577c5c8c..6e089db0d0 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -51,11 +51,12 @@ void virRaiseErrorObject(const char *filename,
virErrorPtr err);
void virReportErrorHelper(int domcode, int errcode,
+ virErrorLevel level,
const char *filename,
const char *funcname,
size_t linenr,
const char *fmt, ...)
- ATTRIBUTE_FMT_PRINTF(6, 7);
+ ATTRIBUTE_FMT_PRINTF(7, 8);
void virReportSystemErrorFull(int domcode,
int theerrno,
@@ -160,10 +161,10 @@ void virReportSystemErrorFull(int domcode,
(fmt), __VA_ARGS__)
# define virReportUnsupportedError() \
- virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT, \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_NO_SUPPORT, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, __FUNCTION__)
# define virReportRestrictedError(...) \
- virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_OPERATION_DENIED, \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_OPERATION_DENIED, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, __VA_ARGS__)
/* The sizeof(...) comparison here is a hack to catch typos
* in the name of the enum by triggering a compile error, as well
@@ -171,7 +172,7 @@ void virReportSystemErrorFull(int domcode,
* or struct type, instead of an enum. It should get optimized away
* since sizeof() is known at compile time */
# define virReportEnumRangeError(typname, value) \
- virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR, \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR, VIR_ERR_ERROR, \
__FILE__, __FUNCTION__, __LINE__, \
"Unexpected enum value %d for %s", \
value, sizeof((typname)1) != 0 ? #typname : #typname);
@@ -185,7 +186,7 @@ void virReportOOMErrorFull(int domcode,
virReportOOMErrorFull(VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__)
# define virReportError(code, ...) \
- virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ virReportErrorHelper(VIR_FROM_THIS, code, VIR_ERR_ERROR, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
# define virReportErrorObject(obj) \
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index 1006a88837..4a0a7f5ee8 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -93,10 +93,12 @@ virKeyFileErrorHelper(const char *file, const char *func, size_t
line,
{
/* Construct the string 'filename:line: info' if we have that. */
if (ctxt && ctxt->filename) {
- virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+ virReportErrorHelper(VIR_FROM_CONF, error, VIR_ERR_ERROR,
+ file, func, line,
_("%s:%zu: %s '%s'"), ctxt->filename,
ctxt->line, info, ctxt->cur);
} else {
- virReportErrorHelper(VIR_FROM_CONF, error, file, func, line,
+ virReportErrorHelper(VIR_FROM_CONF, error, VIR_ERR_ERROR,
+ file, func, line,
"%s", info);
}
}
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 998d974882..a5975e6665 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -37,7 +37,7 @@
#define VIR_FROM_THIS VIR_FROM_XML
#define virGenericReportError(from, code, ...) \
- virReportErrorHelper(from, code, __FILE__, \
+ virReportErrorHelper(from, code, VIR_ERR_ERROR, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
/* Internal data to be passed to SAX parser and used by error handler. */
--
2.17.1
9:14 a.m.
New subject: [libvirt] [RFC 2/7] util: Introduce virReportWarning()
This new macro can be used to conveniently report warnings,
just like the existing virReportError() is used for errors.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/util/virerror.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/util/virerror.h b/src/util/virerror.h
index 6e089db0d0..308b49c604 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -189,6 +189,10 @@ void virReportOOMErrorFull(int domcode,
virReportErrorHelper(VIR_FROM_THIS, code, VIR_ERR_ERROR, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
+# define virReportWarning(code, ...) \
+ virReportErrorHelper(VIR_FROM_THIS, code, VIR_ERR_WARNING, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+
# define virReportErrorObject(obj) \
virRaiseErrorObject(__FILE__, __FUNCTION__, __LINE__, obj)
--
2.17.1
9:14 a.m.
New subject: [libvirt] [RFC 3/7] tools: Print warnings in virsh
We delay dealing with actual errors until a later, more
appropriate time in general, but if we wanted to do the same
with warnings we'd have to introduce a lot of duplicated
handling in virsh, so we just print them right away and move
on with the usual processing instead.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
tools/vsh.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/vsh.c b/tools/vsh.c
index 9ea3c4b96a..2ebd11d09f 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -242,6 +242,9 @@ vshErrorHandler(void *opaque ATTRIBUTE_UNUSED,
{
virFreeError(last_error);
last_error = virSaveLastError();
+
+ if (last_error && last_error->level == VIR_ERR_WARNING)
+ fprintf(stderr, "Warning: %s\n", NULLSTR(last_error->message));
}
/* Store a libvirt error that is from a helper API that doesn't raise errors
--
2.17.1
9:14 a.m.
New subject: [libvirt] [RFC 4/7] util: Introduce VIR_ERR_DEPRECATED_FEATURE
This error code will be used whenever the user triggers
execution of code that's been marked as deprecated.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
include/libvirt/virterror.h | 1 +
src/util/virerror.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 57aadb8d16..780270c4f2 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -323,6 +323,7 @@ typedef enum {
VIR_ERR_DEVICE_MISSING = 99, /* fail to find the desired device */
VIR_ERR_INVALID_NWFILTER_BINDING = 100, /* invalid nwfilter binding */
VIR_ERR_NO_NWFILTER_BINDING = 101, /* no nwfilter binding */
+ VIR_ERR_DEPRECATED_FEATURE = 102, /* deprecated feature */
} virErrorNumber;
/**
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 517e0bde28..f1ee47b309 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1509,6 +1509,12 @@ virErrorMsg(virErrorNumber error, const char *info)
else
errmsg = _("Network filter binding not found: %s");
break;
+ case VIR_ERR_DEPRECATED_FEATURE:
+ if (info == NULL)
+ errmsg = _("Deprecated feature");
+ else
+ errmsg = _("Deprecated feature: %s");
+ break;
}
return errmsg;
}
--
2.17.1
9:14 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
In this case we want to deprecate the API wholesale, so we
can simply report a warning in the public entry point.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/libvirt-domain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7690339521..a758539b6d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -97,6 +97,11 @@ virConnectNumOfDomains(virConnectPtr conn)
int ret = conn->driver->connectNumOfDomains(conn);
if (ret < 0)
goto error;
+
+ virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
+ "%s",
+ "virConnectNumOfDomains()");
+ virDispatchError(conn);
return ret;
}
--
2.17.1
9:43 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Tue, Oct 02, 2018 at 16:14:44 +0200, Andrea Bolognani wrote:
In this case we want to deprecate the API wholesale, so we
can simply report a warning in the public entry point.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/libvirt-domain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7690339521..a758539b6d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -97,6 +97,11 @@ virConnectNumOfDomains(virConnectPtr conn)
int ret = conn->driver->connectNumOfDomains(conn);
if (ret < 0)
goto error;
+
+ virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
+ "%s",
+ "virConnectNumOfDomains()");
+ virDispatchError(conn);
I don't think our API contract allows for reporting an error AND
returning success. And even if we didn't specify it it's pretty much
assumed right now, so I'm not in support of this.
return ret;
}
--
2.17.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:25 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Tue, 2018-10-02 at 16:43 +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:14:44 +0200, Andrea Bolognani wrote:
[...]
> @@ -97,6 +97,11 @@ virConnectNumOfDomains(virConnectPtr conn)
> int ret = conn->driver->connectNumOfDomains(conn);
> if (ret < 0)
> goto error;
> +
> + virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
> + "%s",
> + "virConnectNumOfDomains()");
> + virDispatchError(conn);
I don't think our API contract allows for reporting an error AND
returning success. And even if we didn't specify it it's pretty much
assumed right now, so I'm not in support of this.
I'm afraid I don't have a very good answer for this concern, as
for the most part I share it.
This is the best I could come up with on my own; hopefully other
developers such as yourself will contribute to the discussion and
we'll collectively come up with something much better :)
--
Andrea Bolognani / Red Hat / Virtualization
10:28 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Tue, Oct 02, 2018 at 17:25:34 +0200, Andrea Bolognani wrote:
On Tue, 2018-10-02 at 16:43 +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:14:44 +0200, Andrea Bolognani wrote:
[...]
> I don't think our API contract allows for reporting an error
AND
> returning success. And even if we didn't specify it it's pretty much
> assumed right now, so I'm not in support of this.
I'm afraid I don't have a very good answer for this concern, as
for the most part I share it.
This is the best I could come up with on my own; hopefully other
developers such as yourself will contribute to the discussion and
we'll collectively come up with something much better :)
Well, my suggestion is not to do it. That won't contribute much towards
the discussion but will greatly contribute to not breaking of the code.
6:14 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Tue, Oct 02, 2018 at 17:25:34 +0200, Andrea Bolognani wrote:
On Tue, 2018-10-02 at 16:43 +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:14:44 +0200, Andrea Bolognani wrote:
[...]
> > @@ -97,6 +97,11 @@ virConnectNumOfDomains(virConnectPtr conn)
> > int ret = conn->driver->connectNumOfDomains(conn);
> > if (ret < 0)
> > goto error;
> > +
> > + virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
> > + "%s",
> > + "virConnectNumOfDomains()");
> > + virDispatchError(conn);
>
> I don't think our API contract allows for reporting an error AND
> returning success. And even if we didn't specify it it's pretty much
> assumed right now, so I'm not in support of this.
I'm afraid I don't have a very good answer for this concern, as
for the most part I share it.
This is the best I could come up with on my own; hopefully other
developers such as yourself will contribute to the discussion and
we'll collectively come up with something much better :)
Technically you could introduce a new event and report warnings as
events. That would much cleaner IMHO. Of course, apps would have to run
an event loop and register for getting such events, but not doing so is
not too different from just ignoring them when they are forcefully
pushed on clients by some ugly black magic. Your approach only works for
stuff that can be detected at a client library level, but useless in
case you'd want to deprecate something in an XML, for example. Our RPC
can only transfer a success or an error, but not both. A new event would
be useful for all cases and you could even report several warning for a
single API call.
Jirka
6:13 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Wed, 2018-10-03 at 13:14 +0200, Jiri Denemark wrote:
On Tue, Oct 02, 2018 at 17:25:34 +0200, Andrea Bolognani wrote:
> This is the best I could come up with on my own; hopefully other
> developers such as yourself will contribute to the discussion and
> we'll collectively come up with something much better :)
Technically you could introduce a new event and report warnings as
events. That would much cleaner IMHO. Of course, apps would have to run
an event loop and register for getting such events, but not doing so is
not too different from just ignoring them when they are forcefully
pushed on clients by some ugly black magic. Your approach only works for
stuff that can be detected at a client library level, but useless in
case you'd want to deprecate something in an XML, for example. Our RPC
can only transfer a success or an error, but not both. A new event would
be useful for all cases and you could even report several warning for a
single API call.
Yeah, I've run into the RPC issue and mention it in the cover letter
as something that needs to be addressed before this approach can be
realistically considered.
The idea of using events to report warnings sounds neat! I wonder
much of a limitation the fact that you'd need to have an event loop
running would be in practice...
How likely is it that someone will write/has written a useful,
non-helloworld libvirt application that doesn't have one? Could we
detect whether an even loop is running from library code and, much
further down the line, refuse to execute commands unless it is?
Of course then you get into the fun of warning developers that
they're not going to receive warnings :)
--
Andrea Bolognani / Red Hat / Virtualization
6:29 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Thu, Oct 04, 2018 at 13:13:48 +0200, Andrea Bolognani wrote:
On Wed, 2018-10-03 at 13:14 +0200, Jiri Denemark wrote:
> On Tue, Oct 02, 2018 at 17:25:34 +0200, Andrea Bolognani wrote:
> > This is the best I could come up with on my own; hopefully other
> > developers such as yourself will contribute to the discussion and
> > we'll collectively come up with something much better :)
>
> Technically you could introduce a new event and report warnings as
> events. That would much cleaner IMHO. Of course, apps would have to run
> an event loop and register for getting such events, but not doing so is
> not too different from just ignoring them when they are forcefully
> pushed on clients by some ugly black magic. Your approach only works for
> stuff that can be detected at a client library level, but useless in
> case you'd want to deprecate something in an XML, for example. Our RPC
> can only transfer a success or an error, but not both. A new event would
> be useful for all cases and you could even report several warning for a
> single API call.
Yeah, I've run into the RPC issue and mention it in the cover letter
as something that needs to be addressed before this approach can be
realistically considered.
The idea of using events to report warnings sounds neat! I wonder
much of a limitation the fact that you'd need to have an event loop
running would be in practice...
How likely is it that someone will write/has written a useful,
non-helloworld libvirt application that doesn't have one? Could we
detect whether an even loop is running from library code and, much
further down the line, refuse to execute commands unless it is?
So you want to mandate running an event loop? While that would allow
removing some complexity from the RPC code it's not possible to do it
without breaking apps not running the event loop.
Of course then you get into the fun of warning developers that
they're not going to receive warnings :)
I don't think there's a possibility to mandate reception of warnings
since it would break apps which would not use it at the time.
7:53 a.m.
New subject: [libvirt] [RFC 5/7] Deprecate virConnectNumOfDomains()
On Tue, Oct 02, 2018 at 04:43:01PM +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:14:44 +0200, Andrea Bolognani wrote:
> In this case we want to deprecate the API wholesale, so we
> can simply report a warning in the public entry point.
>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> src/libvirt-domain.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 7690339521..a758539b6d 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -97,6 +97,11 @@ virConnectNumOfDomains(virConnectPtr conn)
> int ret = conn->driver->connectNumOfDomains(conn);
> if (ret < 0)
> goto error;
> +
> + virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
> + "%s",
> + "virConnectNumOfDomains()");
> + virDispatchError(conn);
I don't think our API contract allows for reporting an error AND
returning success. And even if we didn't specify it it's pretty much
assumed right now, so I'm not in support of this.
Yep, even though our public header has VIR_ERR_WARNING level flag,
this is impossible for us to use in practice. It was only ever
usable back in the very early days of libvirt, when there was no
RPC and you had an error callback function registered.
No app ever uses the error callbacks, because it is a single callback
per-process making it unusable when a single process has different
bits of code all calling libvirt.
Meanwhile the RPC code won't transport more than one error object
and will only do it when the API fails, so this warning will be
lost for most drivers.
If anything we should just put a comment in virterror.h that
VIR_ERR_WARNING will never be seen, so apps can assume VIR_ERR_ERROR.
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 :|
9:14 a.m.
New subject: [libvirt] [RFC 6/7] Deprecate missing machine type in virDomainDefineXMLFlags()
This time we don't want to deprecate the API itself; instead,
we want warning to be reported whenever some specific condition
is met.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 11 ++++++++---
src/libvirt-domain.c | 1 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9911d56130..da0f1ed025 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19114,9 +19114,14 @@ virDomainDefParseCaps(virDomainDefPtr def,
} else {
if (!def->os.arch)
def->os.arch = capsdata->arch;
- if ((!def->os.machine &&
- VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0))
- goto cleanup;
+ if (!def->os.machine) {
+ virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
+ "%s",
+ _("Missing machine type"));
+
+ if (VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0)
+ goto cleanup;
+ }
}
ret = 0;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index a758539b6d..be1a67d8f6 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -6216,6 +6216,7 @@ virDomainDefineXMLFlags(virConnectPtr conn, const char *xml,
unsigned int flags)
ret = conn->driver->domainDefineXMLFlags(conn, xml, flags);
if (!ret)
goto error;
+ virDispatchError(conn);
return ret;
}
--
2.17.1
9:48 a.m.
New subject: [libvirt] [RFC 6/7] Deprecate missing machine type in virDomainDefineXMLFlags()
On Tue, Oct 02, 2018 at 16:14:45 +0200, Andrea Bolognani wrote:
This time we don't want to deprecate the API itself; instead,
we want warning to be reported whenever some specific condition
is met.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 11 ++++++++---
src/libvirt-domain.c | 1 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9911d56130..da0f1ed025 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19114,9 +19114,14 @@ virDomainDefParseCaps(virDomainDefPtr def,
} else {
if (!def->os.arch)
def->os.arch = capsdata->arch;
- if ((!def->os.machine &&
- VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0))
- goto cleanup;
+ if (!def->os.machine) {
+ virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
+ "%s",
+ _("Missing machine type"));
Soo, and if multiple warnings are present? Since we don't support
multiple error objects this will create one warning and after you fix it
a second one.
Also I doubt that LXC has a machine type.
10:18 a.m.
New subject: [libvirt] [RFC 6/7] Deprecate missing machine type in virDomainDefineXMLFlags()
On Tue, 2018-10-02 at 16:48 +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:14:45 +0200, Andrea Bolognani wrote:
[...]
> @@ -19114,9 +19114,14 @@ virDomainDefParseCaps(virDomainDefPtr
def,
> } else {
> if (!def->os.arch)
> def->os.arch = capsdata->arch;
> - if ((!def->os.machine &&
> - VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0))
> - goto cleanup;
> + if (!def->os.machine) {
> + virReportWarning(VIR_ERR_DEPRECATED_FEATURE,
> + "%s",
> + _("Missing machine type"));
Soo, and if multiple warnings are present? Since we don't support
multiple error objects this will create one warning and after you fix it
a second one.
I don't think that's necessarily a blocker: a similar behavior can
be observed by feeding libvirt a guest definition that contains two
or more errors, in which case you're expected to solve them one at
the time; you could very well do the same for warnings.
Also I doubt that LXC has a machine type.
That could be addressed by reporting the warning from the QEMU
driver instead of the generic code. Doing so would probably require
shuffling the calls around quite a bit, but I don't see why it
wouldn't be possible.
--
Andrea Bolognani / Red Hat / Virtualization
9:14 a.m.
New subject: [libvirt] [RFC 7/7] tools: Force virsh to use deprecated features
Instead of trying newer APIs first and only falling back to
deprecated ones if those are not available, go straight for
the older APIs.
Running 'virsh list' will now cause warnings to be displayed.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
tools/virsh-domain-monitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 3a2636377d..f2f2944a8a 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1609,6 +1609,8 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
int mansave;
virshControlPtr priv = ctl->privData;
+ goto fallback;
+
/* try the list with flags support (0.9.13 and later) */
if ((ret = virConnectListAllDomains(priv->conn, &list->domains,
flags)) >= 0) {
--
2.17.1
9:30 a.m.
New subject: [libvirt] [RFC 7/7] tools: Force virsh to use deprecated features
On Tue, Oct 02, 2018 at 16:14:46 +0200, Andrea Bolognani wrote:
Instead of trying newer APIs first and only falling back to
deprecated ones if those are not available, go straight for
the older APIs.
Running 'virsh list' will now cause warnings to be displayed.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
tools/virsh-domain-monitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 3a2636377d..f2f2944a8a 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1609,6 +1609,8 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
int mansave;
virshControlPtr priv = ctl->privData;
+ goto fallback;
+
Obvious NACK since you did not make the patch look as a reproducer case
to prove the code changes work.
I suggest adding a all-caps warning to the subject and NOT adding a
Sign-off for reproducers not meant to be pushed.
9:43 a.m.
New subject: [libvirt] [RFC 7/7] tools: Force virsh to use deprecated features
On Tue, 2018-10-02 at 16:30 +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:14:46 +0200, Andrea Bolognani wrote:
> Instead of trying newer APIs first and only falling back to
> deprecated ones if those are not available, go straight for
> the older APIs.
>
> Running 'virsh list' will now cause warnings to be displayed.
>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> tools/virsh-domain-monitor.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 3a2636377d..f2f2944a8a 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1609,6 +1609,8 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
> int mansave;
> virshControlPtr priv = ctl->privData;
>
> + goto fallback;
Obvious NACK since you did not make the patch look as a reproducer case
to prove the code changes work.
I suggest adding a all-caps warning to the subject and NOT adding a
Sign-off for reproducers not meant to be pushed.
Eh, nothing in this series is really meant to be pushed :)
But yours is indeed very good advice in general, so I'll keep it in
mind for future patches. Thanks!
--
Andrea Bolognani / Red Hat / Virtualization
10:26 a.m.
New subject: [libvirt] [RFC 7/7] tools: Force virsh to use deprecated features
On Tue, Oct 02, 2018 at 16:43:04 +0200, Andrea Bolognani wrote:
On Tue, 2018-10-02 at 16:30 +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:14:46 +0200, Andrea Bolognani wrote:
> > Instead of trying newer APIs first and only falling back to
> > deprecated ones if those are not available, go straight for
> > the older APIs.
> >
> > Running 'virsh list' will now cause warnings to be displayed.
> >
> > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> > ---
> > tools/virsh-domain-monitor.c | 2 ++
> > 1 file changed, 2 insertions(+)
[...]
> I suggest adding a all-caps warning to the subject and NOT
adding a
> Sign-off for reproducers not meant to be pushed.
Eh, nothing in this series is really meant to be pushed :)
Such approach without any obvious markers is only warranted on 1st of
April.
10:38 a.m.
New subject: [libvirt] [RFC 7/7] tools: Force virsh to use deprecated features
On Tue, 2018-10-02 at 17:26 +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:43:04 +0200, Andrea Bolognani wrote:
[...]
> Eh, nothing in this series is really meant to be pushed :)
Such approach without any obvious markers is only warranted on 1st of
April.
I figured the series being marked as RFC, along with the cover letter
containing a "Known issues" section mentioning how the implementation
completely fails to work in one out of two sample scenarios, would be
enough to tip readers off.
How do you recommend I make it more obvious next time?
--
Andrea Bolognani / Red Hat / Virtualization
10:19 a.m.
On Tue, Oct 02, 2018 at 16:14:39 +0200, Andrea Bolognani wrote:
Background
==========
[...]
Two concrete examples are considered here: one is the
virConnectNumOfDomains() API which, while known to be racy and having
non-racy alternatives, can still be used by developers without
getting any kind of warning in the process; the other one is the
ability to define a domain without specifying the machine type, which
Okay, but for these particular ones we could do a compile time warning.
Not that we ever can remove them though.
is becoming increasingly problematic now with some x86_64 features
being limited to q35 and downstreams looking to push for its
Well so I presume the issue is that we can't change to q35 as the
default which was vigorously discussed.
IMHO the current approach still allows users shoot themselves into the
foot (usually with less spectacular results than when the drivers for
devices don't work anymore) but still:
If you use a libvirt XML without a machine type but with features which
require a certain machine type (e.g. cpu and memory hotplug) and define
it on a host with new-enough qemu everything will work. If you do the
same on a host with older qemu your VM will not start at all (granted,
here you get a libvirt error as this will mostly result into missing
capabilities.
Our documentation states in multiple places that fields not populated by
the user are mostly hypervisor dependant what the default will become.
In my opinion the machine type is similarly hypervisor dependant, and in
this case the "hypervisor" for the libvirt-qemu infrastructure also
involves libvirt's qemu driver.
adoption, as well as being a manifestation of the more general
problem of libvirt's default being sometimes too conservative and at
odds with the existence of slimmed-down QEMU binaries being built
with reducing the total attack surface in mind.
If your qemu binary does not support certain feature, libvirt will know
it. We have capability detection and for that matter we also have
machine type detection (we fill in the default according to the
canonical name). In such case we are very welcome to choose anything
which will satisfy the default.
I'm afraid though that the downstreams you are mentioning can't in fact
fully drop i440fx for some reason and thus are trying to weasel around
by attempting to make us change the default. This I don't support as a
worthy goal. If they want to slim down qemu, they are welcome and we can
pick a suitable different default.
We can also consider using what qemu provides as a default. Users will
get the default they asked for. They always can specify their specific
type if their software is not happy with it. That is in the end the
reason for libvirt recording the machine type in the XML for it's use as
we don't want it to change. Not specifying it is a invitation to change
it.
10:38 a.m.
On Tue, Oct 02, 2018 at 05:19:30PM +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:14:39 +0200, Andrea Bolognani wrote:
> Background
> ==========
[...]
> Two concrete examples are considered here: one is the
> virConnectNumOfDomains() API which, while known to be racy and having
> non-racy alternatives, can still be used by developers without
> getting any kind of warning in the process; the other one is the
> ability to define a domain without specifying the machine type, which
Okay, but for these particular ones we could do a compile time warning.
Not that we ever can remove them though.
Definitely agree with Peter, having a runtime warning for issue that
you cannot change in runtime situation is IMHO wrong. Even though we
cannot remove any API the deprecation warning can be still useful
because it can suggest better API to use instead of the old one.
> is becoming increasingly problematic now with some x86_64
features
> being limited to q35 and downstreams looking to push for its
Well so I presume the issue is that we can't change to q35 as the
default which was vigorously discussed.
IMHO the current approach still allows users shoot themselves into the
foot (usually with less spectacular results than when the drivers for
devices don't work anymore) but still:
If you use a libvirt XML without a machine type but with features which
require a certain machine type (e.g. cpu and memory hotplug) and define
it on a host with new-enough qemu everything will work. If you do the
same on a host with older qemu your VM will not start at all (granted,
here you get a libvirt error as this will mostly result into missing
capabilities.
Our documentation states in multiple places that fields not populated by
the user are mostly hypervisor dependant what the default will become.
In my opinion the machine type is similarly hypervisor dependant, and in
this case the "hypervisor" for the libvirt-qemu infrastructure also
involves libvirt's qemu driver.
> adoption, as well as being a manifestation of the more general
> problem of libvirt's default being sometimes too conservative and at
> odds with the existence of slimmed-down QEMU binaries being built
> with reducing the total attack surface in mind.
If your qemu binary does not support certain feature, libvirt will know
it. We have capability detection and for that matter we also have
machine type detection (we fill in the default according to the
canonical name). In such case we are very welcome to choose anything
which will satisfy the default.
I'm afraid though that the downstreams you are mentioning can't in fact
fully drop i440fx for some reason and thus are trying to weasel around
by attempting to make us change the default. This I don't support as a
worthy goal. If they want to slim down qemu, they are welcome and we can
pick a suitable different default.
We can also consider using what qemu provides as a default. Users will
get the default they asked for. They always can specify their specific
type if their software is not happy with it. That is in the end the
reason for libvirt recording the machine type in the XML for it's use as
we don't want it to change. Not specifying it is a invitation to change
it.
Again I have to agree here. I don't see any strong reason why we
cannot change default machine type. As Peter wrote, if the only reason
would be that downstream want's to remove it, well they should change
it in downstream as well. But in this case I thing that it's time
to move to Q35 as a default machine type, it's supported by majority of
recent OSes and it would be sensible default.
If some application/user depends on a specific machine type they should
ask for it and most likely they are already doing it. We have a lot of
attributes in our XML that are usually filled in by default and that
default already depends on QEMU version or features that QEMU offers.
Pavel
11:55 a.m.
I feel like I've addressed most of your points in my reply to
Peter's message so I won't repeat the same arguments and will
snip quite aggressively, but please let me know if I've missed
anything.
On Tue, 2018-10-02 at 17:38 +0200, Pavel Hrdina wrote:
[...]
But in this case I thing that it's time
to move to Q35 as a default machine type, it's supported by majority of
recent OSes and it would be sensible default.
If some application/user depends on a specific machine type they should
ask for it and most likely they are already doing it.
Even if we agreed that q35 is a better default than i440fx these
days, changing libvirt's default would be, in my opinion, merely
papering over the real issue, which is: there shouldn't have been
a default to begin with.
Applications should either choose a specific machine type because
they know it's the only one the fits their needs (as you suggest)
or, if they don't have such specialized needs, ask libosinfo to
figure one out based on information such as the OS that will
ultimately run in the guest; the same is true for network
interfaces, storage controllers, graphics adapters and the like.
The idea that you can have a single "good" default is one that was
perhaps true years ago, but it's certainly not true today, and
holding on to it only causes pain for users and developers alike.
We have a lot of
attributes in our XML that are usually filled in by default and that
default already depends on QEMU version or features that QEMU offers.
We're actually awfully inconsistent in this, since we base some of
the choices on QEMU features and have hardcoded values for others; in
some particularly neat cases, we might even end up applying *both*
approaches :)
--
Andrea Bolognani / Red Hat / Virtualization
2:13 a.m.
On 10/02/2018 05:38 PM, Pavel Hrdina wrote:
On Tue, Oct 02, 2018 at 05:19:30PM +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:14:39 +0200, Andrea Bolognani wrote:
>> Background
>> ==========
>
> [...]
>
>> Two concrete examples are considered here: one is the
>> virConnectNumOfDomains() API which, while known to be racy and having
>> non-racy alternatives, can still be used by developers without
>> getting any kind of warning in the process; the other one is the
>> ability to define a domain without specifying the machine type, which
>
> Okay, but for these particular ones we could do a compile time warning.
> Not that we ever can remove them though.
Definitely agree with Peter, having a runtime warning for issue that
you cannot change in runtime situation is IMHO wrong. Even though we
cannot remove any API the deprecation warning can be still useful
because it can suggest better API to use instead of the old one.
So two of our biggest consumers use python bindings. I'm failing to see
how a compile time warning would help here. E.g. if an app is using
"conn.conn.numOfDefinedDomains()" we could call
virConnectListAllDomains() with appropriate flags. Except we can't,
because then the app would be unable to talk to older libvirtds where
the procedure doesn't exist.
In other words, if our goal is to make users switch to newer versions of
functions then we might have to find a better solution (than compilte
time warnings). Runtime warnings would work with python though.
Michal
3:44 a.m.
On Wed, Oct 03, 2018 at 09:13:00AM +0200, Michal Privoznik wrote:
On 10/02/2018 05:38 PM, Pavel Hrdina wrote:
> On Tue, Oct 02, 2018 at 05:19:30PM +0200, Peter Krempa wrote:
>> On Tue, Oct 02, 2018 at 16:14:39 +0200, Andrea Bolognani wrote:
>>> Background
>>> ==========
>>
>> [...]
>>
>>> Two concrete examples are considered here: one is the
>>> virConnectNumOfDomains() API which, while known to be racy and having
>>> non-racy alternatives, can still be used by developers without
>>> getting any kind of warning in the process; the other one is the
>>> ability to define a domain without specifying the machine type, which
>>
>> Okay, but for these particular ones we could do a compile time warning.
>> Not that we ever can remove them though.
>
> Definitely agree with Peter, having a runtime warning for issue that
> you cannot change in runtime situation is IMHO wrong. Even though we
> cannot remove any API the deprecation warning can be still useful
> because it can suggest better API to use instead of the old one.
So two of our biggest consumers use python bindings. I'm failing to see
how a compile time warning would help here. E.g. if an app is using
"conn.conn.numOfDefinedDomains()" we could call
virConnectListAllDomains() with appropriate flags. Except we can't,
because then the app would be unable to talk to older libvirtds where
the procedure doesn't exist.
In other words, if our goal is to make users switch to newer versions of
functions then we might have to find a better solution (than compilte
time warnings). Runtime warnings would work with python though.
As John's pointed out in his reply, given our API contract, we're providing
devs of apps built on top of libvirt with certain guarantees, so it's fair to
assume they'd just ignore the warnings, since we can't break our contract and
their product keeps working anyway, so "why bother changing it", right?
Unfortunately, I feel it's a common practice not to change anything unless it
stops working and there might even be very legitimate reasons for such a
practice, like simply not enough man power.
Therefore I myself don't see any mass adoption/switch to the new preferred/safe
versions of our APIs any time soon. The only effect you'd IMHO achieve by having
runtime warnings is log pollution. Having said that, I don't have any better
ideas on how we could achieve this brave goal without using the obvious drastic
measures.
Erik
6:03 a.m.
On Wed, 2018-10-03 at 10:44 +0200, Erik Skultety wrote:
On Wed, Oct 03, 2018 at 09:13:00AM +0200, Michal Privoznik wrote:
> On 10/02/2018 05:38 PM, Pavel Hrdina wrote:
> > Definitely agree with Peter, having a runtime warning for issue that
> > you cannot change in runtime situation is IMHO wrong. Even though we
> > cannot remove any API the deprecation warning can be still useful
> > because it can suggest better API to use instead of the old one.
>
> So two of our biggest consumers use python bindings. I'm failing to see
> how a compile time warning would help here.
The compile time warnings would cause bindings developers to find
out APIs are being deprecated as they compile against newer libvirt
the same way applications developers linking against the C library
would; additionally, they could use that information as a motivation
to deprecate *their* APIs in a language-appropriate manner.
So, as mentioned elsewhere, I believe we should have ways to warn
about deprecated features *both* at compile time and at runtime.
> In other words, if our goal is to make users switch to newer
versions of
> functions then we might have to find a better solution (than compilte
> time warnings). Runtime warnings would work with python though.
As John's pointed out in his reply, given our API contract, we're providing
devs of apps built on top of libvirt with certain guarantees, so it's fair to
assume they'd just ignore the warnings, since we can't break our contract and
their product keeps working anyway, so "why bother changing it", right?
Unfortunately, I feel it's a common practice not to change anything unless it
stops working and there might even be very legitimate reasons for such a
practice, like simply not enough man power.
Therefore I myself don't see any mass adoption/switch to the new preferred/safe
versions of our APIs any time soon. The only effect you'd IMHO achieve by having
runtime warnings is log pollution. Having said that, I don't have any better
ideas on how we could achieve this brave goal without using the obvious drastic
measures.
Deprecation warnings can still help adoption: if tools start spewing
them, users will notice and report bugs, eventually annoying
developers into migrating to non-deprecated APIs :)
Of course this works better for command line applications like
virt-install than for GUI applications like virt-manager, but it's
useful nonetheless.
That said, in order to reap real benefits, deprecating features
should go hand in hand with having a well-defined support policy
that includes a timeline describing how, after a generous grace
period, the functionality will actually be dropped altogether.
--
Andrea Bolognani / Red Hat / Virtualization
6:16 a.m.
On 10/04/2018 01:03 PM, Andrea Bolognani wrote:
On Wed, 2018-10-03 at 10:44 +0200, Erik Skultety wrote:
> On Wed, Oct 03, 2018 at 09:13:00AM +0200, Michal Privoznik wrote:
>> On 10/02/2018 05:38 PM, Pavel Hrdina wrote:
>>> Definitely agree with Peter, having a runtime warning for issue that
>>> you cannot change in runtime situation is IMHO wrong. Even though we
>>> cannot remove any API the deprecation warning can be still useful
>>> because it can suggest better API to use instead of the old one.
>>
>> So two of our biggest consumers use python bindings. I'm failing to see
>> how a compile time warning would help here.
The compile time warnings would cause bindings developers to find
out APIs are being deprecated as they compile against newer libvirt
the same way applications developers linking against the C library
would; additionally, they could use that information as a motivation
to deprecate *their* APIs in a language-appropriate manner.
So, as mentioned elsewhere, I believe we should have ways to warn
about deprecated features *both* at compile time and at runtime.
>> In other words, if our goal is to make users switch to newer versions of
>> functions then we might have to find a better solution (than compilte
>> time warnings). Runtime warnings would work with python though.
>
> As John's pointed out in his reply, given our API contract, we're providing
> devs of apps built on top of libvirt with certain guarantees, so it's fair to
> assume they'd just ignore the warnings, since we can't break our contract
and
> their product keeps working anyway, so "why bother changing it", right?
> Unfortunately, I feel it's a common practice not to change anything unless it
> stops working and there might even be very legitimate reasons for such a
> practice, like simply not enough man power.
> Therefore I myself don't see any mass adoption/switch to the new preferred/safe
> versions of our APIs any time soon. The only effect you'd IMHO achieve by having
> runtime warnings is log pollution. Having said that, I don't have any better
> ideas on how we could achieve this brave goal without using the obvious drastic
> measures.
Deprecation warnings can still help adoption: if tools start spewing
them, users will notice and report bugs, eventually annoying
developers into migrating to non-deprecated APIs :)
This was my reasoning too. We have to have run time warnings. Not all
languages are compiled. Even though I have no statistics, I am confident
to say that oVirt and OpenStack are our biggest consumers and they use
python bindings. Compile time warnings won't help us there, rather the
opposite - put pressure on us when writing them.
Of course this works better for command line applications like
virt-install than for GUI applications like virt-manager, but it's
useful nonetheless.
That said, in order to reap real benefits, deprecating features
should go hand in hand with having a well-defined support policy
that includes a timeline describing how, after a generous grace
period, the functionality will actually be dropped altogether.
/me grabbing some popcorn and soda O:-)
So this was going to be my question. What is the end goal we are trying
to achieve? If it is to advertise we have a better API to do the job, we
can document it - just like we are doing so far. Also the term "better
API" depends on individual use case heavily. If I have a small, one host
virtualization solution, and I know nobody else is starting up/shutting
down domains, I might use virDomainGetNames() just fine.
Worse, if I have a working solution and I upgrade libvirt I'll start
seeing warning all of a sudden. This kind of breaks our promise of
stability.
If the goal is to make mgmt apps switch to APIs we consider better, then
we definitely need runtime warnings. I don't see how apps written in
dynamic, not compiled languages can detect they're using deprecated API
without running themselves. And surely nor developers, nor QEs have 100%
code coverage tests.
And for dropping functionality - we can not do that. Period.
Michal
6:36 a.m.
On Thu, Oct 04, 2018 at 01:16:46PM +0200, Michal Privoznik wrote:
On 10/04/2018 01:03 PM, Andrea Bolognani wrote:
> On Wed, 2018-10-03 at 10:44 +0200, Erik Skultety wrote:
>> On Wed, Oct 03, 2018 at 09:13:00AM +0200, Michal Privoznik wrote:
>>> On 10/02/2018 05:38 PM, Pavel Hrdina wrote:
>>>> Definitely agree with Peter, having a runtime warning for issue that
>>>> you cannot change in runtime situation is IMHO wrong. Even though we
>>>> cannot remove any API the deprecation warning can be still useful
>>>> because it can suggest better API to use instead of the old one.
>>>
>>> So two of our biggest consumers use python bindings. I'm failing to see
>>> how a compile time warning would help here.
>
> The compile time warnings would cause bindings developers to find
> out APIs are being deprecated as they compile against newer libvirt
> the same way applications developers linking against the C library
> would; additionally, they could use that information as a motivation
> to deprecate *their* APIs in a language-appropriate manner.
I completely agree with this.
> So, as mentioned elsewhere, I believe we should have ways to
warn
> about deprecated features *both* at compile time and at runtime.
But definitely not with this paragraph. See below.
>>> In other words, if our goal is to make users switch to
newer versions of
>>> functions then we might have to find a better solution (than compilte
>>> time warnings). Runtime warnings would work with python though.
>>
>> As John's pointed out in his reply, given our API contract, we're
providing
>> devs of apps built on top of libvirt with certain guarantees, so it's fair
to
>> assume they'd just ignore the warnings, since we can't break our
contract and
>> their product keeps working anyway, so "why bother changing it",
right?
>> Unfortunately, I feel it's a common practice not to change anything unless
it
>> stops working and there might even be very legitimate reasons for such a
>> practice, like simply not enough man power.
>> Therefore I myself don't see any mass adoption/switch to the new
preferred/safe
>> versions of our APIs any time soon. The only effect you'd IMHO achieve by
having
>> runtime warnings is log pollution. Having said that, I don't have any
better
>> ideas on how we could achieve this brave goal without using the obvious drastic
>> measures.
>
> Deprecation warnings can still help adoption: if tools start spewing
> them, users will notice and report bugs, eventually annoying
> developers into migrating to non-deprecated APIs :)
This was my reasoning too. We have to have run time warnings. Not all
languages are compiled. Even though I have no statistics, I am confident
to say that oVirt and OpenStack are our biggest consumers and they use
python bindings. Compile time warnings won't help us there, rather the
opposite - put pressure on us when writing them.
You should not mix libvirt and libvirt-python, two separate projects
and two separate issues. Even if that is true, we should still handle
deprecation warnings for C code as well and that should be compile time
only. See bellow.
> Of course this works better for command line applications like
> virt-install than for GUI applications like virt-manager, but it's
> useful nonetheless.
>
> That said, in order to reap real benefits, deprecating features
> should go hand in hand with having a well-defined support policy
> that includes a timeline describing how, after a generous grace
> period, the functionality will actually be dropped altogether.
>
/me grabbing some popcorn and soda O:-)
So this was going to be my question. What is the end goal we are trying
to achieve? If it is to advertise we have a better API to do the job, we
can document it - just like we are doing so far. Also the term "better
API" depends on individual use case heavily. If I have a small, one host
virtualization solution, and I know nobody else is starting up/shutting
down domains, I might use virDomainGetNames() just fine.
Worse, if I have a working solution and I upgrade libvirt I'll start
seeing warning all of a sudden. This kind of breaks our promise of
stability.
If the goal is to make mgmt apps switch to APIs we consider better, then
we definitely need runtime warnings. I don't see how apps written in
dynamic, not compiled languages can detect they're using deprecated API
without running themselves. And surely nor developers, nor QEs have 100%
code coverage tests.
Runtime deprecation warning in C code is wrong in my opinion and even
though there are some project doing it we should not do it.
Deprecation warning is for developers, not for users.
Yes, adding deprecation warning into libvirt will not solve the goal for
oVirt nor OpenStack, but this should be handled in the libvirt-python
binding code using language-specific manner as Andrea mentioned above.
In python it happens to be a runtime warning,
warnings.DeprecatedWarning, however, that warning is by default ignored
unless it's triggered from __main__, so developers using libvirt-python
API would need to enable that warning for their test-suite, that's a
python way how to do it.
One of the reasons why it's not enabled by default is that as user
you have no way how to modify the module code and therefore you should
not get the warning. From user point of view everything works how it
should and I don't care whether my program uses something deprecated
or not. It's unnecessary noise for the user.
Again, we should not introduce runtime deprecation warnings in C code.
Pavel
And for dropping functionality - we can not do that. Period.
+1, for everyone questioning this please read [1], thanks.
Pavel
[1] <https://libvirt.org/support.html>
7:06 a.m.
On Thu, 2018-10-04 at 13:36 +0200, Pavel Hrdina wrote:
Runtime deprecation warning in C code is wrong in my opinion and
even
though there are some project doing it we should not do it.
Deprecation warning is for developers, not for users.
Yes, adding deprecation warning into libvirt will not solve the goal for
oVirt nor OpenStack, but this should be handled in the libvirt-python
binding code using language-specific manner as Andrea mentioned above.
In python it happens to be a runtime warning,
warnings.DeprecatedWarning, however, that warning is by default ignored
unless it's triggered from __main__, so developers using libvirt-python
API would need to enable that warning for their test-suite, that's a
python way how to do it.
One of the reasons why it's not enabled by default is that as user
you have no way how to modify the module code and therefore you should
not get the warning. From user point of view everything works how it
should and I don't care whether my program uses something deprecated
or not. It's unnecessary noise for the user.
All of the above is perfectly reasonable if you only consider cases
where we might want to deprecate an entire API, but fails to take
into account that a large part of libvirt's functionality is exposed
through XML elements and attributes rather than C function calls.
So if we wanted to warn people that not providing a machine type
when defining a guest is a bad idea and they should use libosinfo
to figure out the information instead, there's simply no way we can
do that at compile time.
Not to mention that the existence of virsh somewhat blurs the lines
between users and developers, as it's a very thin wrapper where each
option translates almost directly to the corresponding C function
call and its parameters and flags, but is then installed in $PATH
for the world to use...
> And for dropping functionality - we can not do that. Period.
+1, for everyone questioning this please read [1], thanks.
[1] <https://libvirt.org/support.html>
As mentioned elsewhere, that document defines our current stance on
backwards compatibility, but nothing says we can't change it if we
think doing so will bring benefits to developers and users, which I
strongly believe it would.
We used to have a similar, although not formally documented, stance
on supporting old QEMU versions, but we've since relaxed it[1] which
has benefited libvirt and its developers greatly without, as far as
I'm aware, inconveniencing users.
[1] A change that, I can't help noticing, you signed off on ;)
--
Andrea Bolognani / Red Hat / Virtualization
8:03 a.m.
On Thu, Oct 04, 2018 at 02:06:54PM +0200, Andrea Bolognani wrote:
On Thu, 2018-10-04 at 13:36 +0200, Pavel Hrdina wrote:
> Runtime deprecation warning in C code is wrong in my opinion and even
> though there are some project doing it we should not do it.
> Deprecation warning is for developers, not for users.
>
> Yes, adding deprecation warning into libvirt will not solve the goal for
> oVirt nor OpenStack, but this should be handled in the libvirt-python
> binding code using language-specific manner as Andrea mentioned above.
> In python it happens to be a runtime warning,
> warnings.DeprecatedWarning, however, that warning is by default ignored
> unless it's triggered from __main__, so developers using libvirt-python
> API would need to enable that warning for their test-suite, that's a
> python way how to do it.
>
> One of the reasons why it's not enabled by default is that as user
> you have no way how to modify the module code and therefore you should
> not get the warning. From user point of view everything works how it
> should and I don't care whether my program uses something deprecated
> or not. It's unnecessary noise for the user.
All of the above is perfectly reasonable if you only consider cases
where we might want to deprecate an entire API, but fails to take
into account that a large part of libvirt's functionality is exposed
through XML elements and attributes rather than C function calls.
So if we wanted to warn people that not providing a machine type
when defining a guest is a bad idea and they should use libosinfo
to figure out the information instead, there's simply no way we can
do that at compile time.
Sure, if we want to warn about missing/wrong/deprecated features from
XML we need runtime warnings only for that and that's OK, that is
something that user usually can change.
Not to mention that the existence of virsh somewhat blurs the lines
between users and developers, as it's a very thin wrapper where each
option translates almost directly to the corresponding C function
call and its parameters and flags, but is then installed in $PATH
for the world to use...
Again if it's something XML related user can change that even while
using virsh, if it's API related user should not see it by default.
> > And for dropping functionality - we can not do that.
Period.
>
> +1, for everyone questioning this please read [1], thanks.
>
> [1] <https://libvirt.org/support.html>
As mentioned elsewhere, that document defines our current stance on
backwards compatibility, but nothing says we can't change it if we
think doing so will bring benefits to developers and users, which I
strongly believe it would.
We used to have a similar, although not formally documented, stance
on supporting old QEMU versions, but we've since relaxed it[1] which
has benefited libvirt and its developers greatly without, as far as
I'm aware, inconveniencing users.
I'm not sure whether we had that promise for QEMU versions documented
and it's also slightly different situation. We are dropping support
for old QEMUs in new libvirts with the assumption that user usually
updates both at the same time (through OS distribution channels).
Removing APIs is totally different. We would have to bump our library
so version and that would break every existing application unless you
would rebuild it to use the new library. That would cause a lot of
issues for everyone using libvirt.
Yes, dropping old or broken API would allow us to drop a lot of code,
on the other hand with the deprecation warning in place and deprecation
note in the documentation we can start claiming that these APIs are no
longer supported and any bugs caused be these APIs (maybe except for
security issues) will not be fixed and in the code we could try to
isolate that code as much as possible to make it easier for us.
Pavel
6:38 a.m.
On Thu, 2018-10-04 at 13:16 +0200, Michal Privoznik wrote:
On 10/04/2018 01:03 PM, Andrea Bolognani wrote:
> That said, in order to reap real benefits, deprecating features
> should go hand in hand with having a well-defined support policy
> that includes a timeline describing how, after a generous grace
> period, the functionality will actually be dropped altogether.
/me grabbing some popcorn and soda O:-)
So this was going to be my question. What is the end goal we are trying
to achieve? If it is to advertise we have a better API to do the job, we
can document it - just like we are doing so far. Also the term "better
API" depends on individual use case heavily. If I have a small, one host
virtualization solution, and I know nobody else is starting up/shutting
down domains, I might use virDomainGetNames() just fine.
Until the time you give access to your host to another user, and
suddenly your scripts will fail 10% of the time for no obvious
reason ;)
Using the proper API might be more work upfront, but it will pay
off in the long run.
Worse, if I have a working solution and I upgrade libvirt I'll
start
seeing warning all of a sudden. This kind of breaks our promise of
stability.
If the functionality keeps working, then getting a bunch of warnings
in the process will definitely not break any stability promise. And
if reporting warnings breaks functionality, then we need to find a
different way to report them because applications no longer working
after a libvirt update is simply unacceptable.
And for dropping functionality - we can not do that. Period.
It's *us* who decided that's the case, so it's up to *us* to uphold
that decision. Or, you know, revert it :)
I'm not saying that we should start dropping random features out of
the blue without any rationale, but if we start formally deprecating
functionality that we deem problematic and at the same time
communicate clearly to both developers and users such functionality
will be removed upstream after, say, three years worth of releases,
then I see absolutely no problem with that.
GTK+ does it, Qt does it; and I'm willing to bet there are more
applications linking against either of them than there are linking
against libvirt.
--
Andrea Bolognani / Red Hat / Virtualization
8:02 a.m.
On Thu, Oct 04, 2018 at 01:38:33PM +0200, Andrea Bolognani wrote:
GTK+ does it, Qt does it; and I'm willing to bet there are more
applications linking against either of them than there are linking
against libvirt.
I don't think that's a positive example. As a developer who has
used GTK for multiple apps, I *hate* their frequent API deprecations
during minor updates. It is a constant battle to keep app code
compiling cleanly even within a single releae, no to mention the
even greater pain of trying to do GTK2 and GTK3 in parallel.
Sure, this deprecation and deletion of code benefits the GTK
maintainers, but the cost of creating ongoing pain for every
single application developer. That's not a net win IMHO as the
set of app developers is much larger.
As a counter point, Microsoft Windows never breaks its APIs and
that has many orders of magnitude more app developers than GTK/Qt
The fact that libvirt never breaks API is one of our greatest
selling points.
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 :|
9:08 a.m.
On Thu, 2018-10-04 at 14:02 +0100, Daniel P. Berrangé wrote:
On Thu, Oct 04, 2018 at 01:38:33PM +0200, Andrea Bolognani wrote:
> GTK+ does it, Qt does it; and I'm willing to bet there are more
> applications linking against either of them than there are linking
> against libvirt.
I don't think that's a positive example. As a developer who has
used GTK for multiple apps, I *hate* their frequent API deprecations
during minor updates. It is a constant battle to keep app code
compiling cleanly even within a single releae, no to mention the
even greater pain of trying to do GTK2 and GTK3 in parallel.
Sure, this deprecation and deletion of code benefits the GTK
maintainers, but the cost of creating ongoing pain for every
single application developer. That's not a net win IMHO as the
set of app developers is much larger.
I think keeping up with changes in libraries you consume is simply
due diligence for developers, and while of course that takes up
some of your time and is for the most part hardly a glamorous job,
it also ensures your application is in tip-top shape at any given
time and is taking advantage of all possible improvements in the
underlying tech by periodically reconsidering parts of its design,
which will ultimately result in a better experience for users.
Cleaning up the API from time to time also means developers won't
have to figure out which one of the seemingly equivalent solutions
to a given problem presents which particular pitfalls, which will
reduce frustrations and once again result in better applications.
And libvirt developers themselves not having to tiptoe around what
is in some cases quite literally a decade worth of backwards
compatibility hacks would allow them to focus on features and bug
fixes that actually impact users in a positive way, all the while
keeping the size of the library from growing out of control, with
all the resource usage and attack suface consideration that entails.
As a counter point, Microsoft Windows never breaks its APIs and
that has many orders of magnitude more app developers than GTK/Qt
How many of the bugs and crashes experienced daily by Windows users
could be avoided if applications were forced to adopt newer, safer
APIs over time instead of being allowed to lazily stick to whatever
was available at the time Windows 95 was the new hotness?
Your guess is as good as mine, but I'm pretty sure the answer is
not "zero".
The fact that libvirt never breaks API is one of our greatest
selling points.
I don't have hard data on this, but I suspect the success of
libvirt is driven more by the great features it offers than its
promise to preserve API/ABI compatibility forever; even more so
considering that the majority of Open Source projects don't offer
anything close to the latter, and yet somehow manage to thrive.
--
Andrea Bolognani / Red Hat / Virtualization
9:26 a.m.
On Thu, Oct 04, 2018 at 04:08:31PM +0200, Andrea Bolognani wrote:
On Thu, 2018-10-04 at 14:02 +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 04, 2018 at 01:38:33PM +0200, Andrea Bolognani wrote:
> > GTK+ does it, Qt does it; and I'm willing to bet there are more
> > applications linking against either of them than there are linking
> > against libvirt.
>
> I don't think that's a positive example. As a developer who has
> used GTK for multiple apps, I *hate* their frequent API deprecations
> during minor updates. It is a constant battle to keep app code
> compiling cleanly even within a single releae, no to mention the
> even greater pain of trying to do GTK2 and GTK3 in parallel.
>
> Sure, this deprecation and deletion of code benefits the GTK
> maintainers, but the cost of creating ongoing pain for every
> single application developer. That's not a net win IMHO as the
> set of app developers is much larger.
I think keeping up with changes in libraries you consume is simply
due diligence for developers, and while of course that takes up
some of your time and is for the most part hardly a glamorous job,
it also ensures your application is in tip-top shape at any given
time and is taking advantage of all possible improvements in the
underlying tech by periodically reconsidering parts of its design,
which will ultimately result in a better experience for users.
Changing existing code to use new APIs doesn't imply the app
is in tip-top shape. On the contrary, I can arguably make it
more likely to break in subtle ways by changing something which
is long tested in the field for something new & relatively
untested.
And libvirt developers themselves not having to tiptoe around what
is in some cases quite literally a decade worth of backwards
compatibility hacks would allow them to focus on features and bug
fixes that actually impact users in a positive way, all the while
keeping the size of the library from growing out of control, with
all the resource usage and attack suface consideration that entails.
Maintaining API compatibility is not really the main contributor
to resource usage or attack surface. That is a large architectural
issue, such as the monolithic libvirtd and usage memory unsafe
language for impl. Deprecating & removing features or changing
defaults will have negligible impact.
> As a counter point, Microsoft Windows never breaks its APIs and
> that has many orders of magnitude more app developers than GTK/Qt
How many of the bugs and crashes experienced daily by Windows users
could be avoided if applications were forced to adopt newer, safer
APIs over time instead of being allowed to lazily stick to whatever
was available at the time Windows 95 was the new hotness?
The idea that you can force people to adapt is flawed. Some devs
may adapt. Others simply won't so an app that otherwise worked
fine will now be needlessly broken. Others will simply bundle
the older version of your API in their code getting stuck on
old outdated insecure versions.
Your guess is as good as mine, but I'm pretty sure the answer is
not "zero".
Adapting to new APIs will itself introduce new bugs. Libvirt has felt
this when we've had to adapt to APIs changing in things we use too.
> The fact that libvirt never breaks API is one of our greatest
> selling points.
I don't have hard data on this, but I suspect the success of
libvirt is driven more by the great features it offers than its
promise to preserve API/ABI compatibility forever; even more so
considering that the majority of Open Source projects don't offer
anything close to the latter, and yet somehow manage to thrive.
GTK's API breakage means that we still have applications in Fedora
that are stuck on GTK1, despite fact that GTK3 is current and GTK4
is coming soon. The world of hurt caused by Python's gratuituous
incompatbilities between py2 and py3 is going to continue to harm
the python community for years to come.
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 :|
10:43 a.m.
On Thu, 2018-10-04 at 15:26 +0100, Daniel P. Berrangé wrote:
On Thu, Oct 04, 2018 at 04:08:31PM +0200, Andrea Bolognani wrote:
> I think keeping up with changes in libraries you consume is simply
> due diligence for developers, and while of course that takes up
> some of your time and is for the most part hardly a glamorous job,
> it also ensures your application is in tip-top shape at any given
> time and is taking advantage of all possible improvements in the
> underlying tech by periodically reconsidering parts of its design,
> which will ultimately result in a better experience for users.
Changing existing code to use new APIs doesn't imply the app
is in tip-top shape. On the contrary, I can arguably make it
more likely to break in subtle ways by changing something which
is long tested in the field for something new & relatively
untested.
You don't have to be an early adopter: it's perfectly fine to wait
a few point releases before moving to a new API. Never addressing
the deprecation warnings, though, is hardly going to result in
better code in the long run.
> And libvirt developers themselves not having to tiptoe around
what
> is in some cases quite literally a decade worth of backwards
> compatibility hacks would allow them to focus on features and bug
> fixes that actually impact users in a positive way, all the while
> keeping the size of the library from growing out of control, with
> all the resource usage and attack suface consideration that entails.
Maintaining API compatibility is not really the main contributor
to resource usage or attack surface. That is a large architectural
issue, such as the monolithic libvirtd and usage memory unsafe
language for impl. Deprecating & removing features or changing
defaults will have negligible impact.
It is certainly forcing us to write more complex logic, which is
easier to get wrong and will result in more bugs regardless of
how safe the programming language we use is.
Not that I'm opposing any of the changes you mention, mind you :)
> > As a counter point, Microsoft Windows never breaks its APIs
and
> > that has many orders of magnitude more app developers than GTK/Qt
>
> How many of the bugs and crashes experienced daily by Windows users
> could be avoided if applications were forced to adopt newer, safer
> APIs over time instead of being allowed to lazily stick to whatever
> was available at the time Windows 95 was the new hotness?
The idea that you can force people to adapt is flawed. Some devs
may adapt. Others simply won't so an app that otherwise worked
fine will now be needlessly broken. Others will simply bundle
the older version of your API in their code getting stuck on
old outdated insecure versions.
Are those the kind of applications you'd trust to manage your
virtualization cluster?
If the developers can't be bothered to put out a new release
for literally *years*, then you're better off using pretty much
anything else.
> Your guess is as good as mine, but I'm pretty sure the
answer is
> not "zero".
Adapting to new APIs will itself introduce new bugs. Libvirt has felt
this when we've had to adapt to APIs changing in things we use too.
Adding support for new QEMU features also sometimes results in new
bugs popping up, but that should teach us to write and test our
code more carefully, not to give up on keeping up with the
innovations happening in QEMU.
> > The fact that libvirt never breaks API is one of our
greatest
> > selling points.
>
> I don't have hard data on this, but I suspect the success of
> libvirt is driven more by the great features it offers than its
> promise to preserve API/ABI compatibility forever; even more so
> considering that the majority of Open Source projects don't offer
> anything close to the latter, and yet somehow manage to thrive.
GTK's API breakage means that we still have applications in Fedora
that are stuck on GTK1, despite fact that GTK3 is current and GTK4
is coming soon.
'dnf repoquery --whatrequires gtk+' reports ~30 results, a big chunk
of which are related to XMMS, a media player that's not been updated
in 10+ year; GTK+ 1 itself hasn't seen a release since 2011.
Allowing them to still be just a 'dnf install' away for any user is
borderline irresponsible IMHO.
The world of hurt caused by Python's gratuituous
incompatbilities between py2 and py3 is going to continue to harm
the python community for years to come.
For what it's worth, and that's probably not much, according to the
TIOBE Index Python is more popular now that it's ever been, and it
has climbed back from 5th to 3rd place over the past year, despite
it being arguably one of the worst times for Python programmers who
had been putting off the 2->3 transition until the very last moment.
--
Andrea Bolognani / Red Hat / Virtualization
11:26 a.m.
On Tue, 2018-10-02 at 17:19 +0200, Peter Krempa wrote:
On Tue, Oct 02, 2018 at 16:14:39 +0200, Andrea Bolognani wrote:
[...]
> Two concrete examples are considered here: one is the
> virConnectNumOfDomains() API which, while known to be racy and having
> non-racy alternatives, can still be used by developers without
> getting any kind of warning in the process; the other one is the
> ability to define a domain without specifying the machine type, which
Okay, but for these particular ones we could do a compile time warning.
I believe we should really have both, to address both applications
being rebuilt against newer libvirt all the time, such as open source
projects that are included in our CI or in any Linux distro, and
those that aren't and just get a new libvirt version from time to
time due to OS updates, such as home-grown management tools.
Not that we ever can remove them though.
True, that has been our policy so far. Doesn't mean it cannot ever
change :)
[...]
Our documentation states in multiple places that fields not populated
by
the user are mostly hypervisor dependant what the default will become.
In my opinion the machine type is similarly hypervisor dependant, and in
this case the "hypervisor" for the libvirt-qemu infrastructure also
involves libvirt's qemu driver.
I don't necessarily disagree with you, but it should be noted that
attempts to change libvirt's own defaults have been rejected times
and times again on the basis that existing applications were, despite
that being a very bad idea, relying on them.
> adoption, as well as being a manifestation of the more general
> problem of libvirt's default being sometimes too conservative and at
> odds with the existence of slimmed-down QEMU binaries being built
> with reducing the total attack surface in mind.
If your qemu binary does not support certain feature, libvirt will know
it. We have capability detection and for that matter we also have
machine type detection (we fill in the default according to the
canonical name). In such case we are very welcome to choose anything
which will satisfy the default.
I'm afraid though that the downstreams you are mentioning can't in fact
fully drop i440fx for some reason and thus are trying to weasel around
by attempting to make us change the default. This I don't support as a
worthy goal. If they want to slim down qemu, they are welcome and we can
pick a suitable different default.
q35 is what sparked the discussion, but it's far from the only
offender. For example, if I create a guest using
$ virt-install \
--os-variant fedora28 \
...
then libosinfo will be queried for information about supported
network cards, and the resulting XML will look like
<interface type='network'>
<model type='virtio'/>
...
However, libvirt's own default for x86_64 guests' network devices is
rtl8139, which means that if I later 'virsh edit' the guest or 'virsh
attach-device' a new network interface I will get that model instead
of virtio; to add insult to injury, the above will happen even if my
QEMU binary has rtl8139 compiled out and virtio-net-pci compiled in!
The only way to avoid such issues is to require the user / management
application to provide the model name themselves.
We can also consider using what qemu provides as a default. Users
will
get the default they asked for. They always can specify their specific
type if their software is not happy with it.
Using QEMU's default machine type is exactly what we were doing until
very recently, but we changed that because QEMU's default has been
i440fx for so long that applications have come to rely on it and
would break if q35 suddenly started showing up instead, which goes to
show that they should not have been relying on either QEMU's or
libvirt's default, and they should have been providing the machine
type explicitly, possibly as obtained by querying libosinfo, instead.
Or, looking at it from the other side, that libvirt should have
required them to provide the machine type instead of trying to be
helpful and filling it in if absent. We can't retroactively mandate
applications do that, but we can deprecate such behavior and thus
steer them firmly towards the proper solution.
--
Andrea Bolognani / Red Hat / Virtualization
3:52 a.m.
On Tue, Oct 02, 2018 at 18:26:12 +0200, Andrea Bolognani wrote:
On Tue, 2018-10-02 at 17:19 +0200, Peter Krempa wrote:
> Our documentation states in multiple places that fields not populated by
> the user are mostly hypervisor dependant what the default will become.
>
> In my opinion the machine type is similarly hypervisor dependant, and in
> this case the "hypervisor" for the libvirt-qemu infrastructure also
> involves libvirt's qemu driver.
I don't necessarily disagree with you, but it should be noted that
attempts to change libvirt's own defaults have been rejected times
and times again on the basis that existing applications were, despite
that being a very bad idea, relying on them.
I'm going to talk only about default values which are stored in domain
XML once chosen here as the ones which do not make it into the XML are
much more complicated and we can't easily change them...
I think rejecting changes to libvirt's defaults in general is wrong. The
defaults are not stable and have never been and we should stop
pretending they are. When we select a default value for something we
store it in the domain XML to protect us from future changes to the
default value. Not to mention that defaults may depend on hypervisor
version and we need to make sure we fail to start a domain on newer
hypervisor with different defaults rather than silently starting a
domain with different devices and hope for not breaking the guest OS.
So for example, when a domain XML contains no machine type or even just
an alias, such as "pc", we replace it with something real when the
domain is defined. Thus when the exact same domain XML will result in
different machine type depending on the QEMU version. Of course, the
difference is not so drastic as in "pc" vs. "q35", but the difference
is
real.
That said, I'm not saying we should just change our defaults just
because we think a new value is better. We have have libosinfo for
telling users what values are the best for their guest OS. But in case
QEMU deprecates some device model or even machine type, I think we
should avoid using the deprecated model as a default value to prepare
people for transition. Once the deprecated model is removed, our default
would change anyway, but existing domain would suddenly start to fail in
case we did not change the default when the model was marked as
deprecated.
Of course, this would not help apps or users using transient domains
without default values, but such apps/users are just broken if they rely
on defaults. For persistent domains libvirt takes care of preserving
default values from when a domain was first defined. Users/apps have to
handle this themselves for transient apps if they care. Libvirt updates
the live XML and they can read it and update their definition to make
the defaults persistent. There's nothing more we can do for them and I
don't think this should block us from changing any defaults. We're
already doing so anyway.
Jirka
8:13 a.m.
On Tue, Oct 02, 2018 at 06:26:12PM +0200, Andrea Bolognani wrote:
On Tue, 2018-10-02 at 17:19 +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:14:39 +0200, Andrea Bolognani wrote:
[...]
> > Two concrete examples are considered here: one is the
> > virConnectNumOfDomains() API which, while known to be racy and having
> > non-racy alternatives, can still be used by developers without
> > getting any kind of warning in the process; the other one is the
> > ability to define a domain without specifying the machine type, which
>
> Okay, but for these particular ones we could do a compile time warning.
I believe we should really have both, to address both applications
being rebuilt against newer libvirt all the time, such as open source
projects that are included in our CI or in any Linux distro, and
those that aren't and just get a new libvirt version from time to
time due to OS updates, such as home-grown management tools.
> Not that we ever can remove them though.
True, that has been our policy so far. Doesn't mean it cannot ever
change :)
[...]
> Our documentation states in multiple places that fields not populated by
> the user are mostly hypervisor dependant what the default will become.
>
> In my opinion the machine type is similarly hypervisor dependant, and in
> this case the "hypervisor" for the libvirt-qemu infrastructure also
> involves libvirt's qemu driver.
I don't necessarily disagree with you, but it should be noted that
attempts to change libvirt's own defaults have been rejected times
and times again on the basis that existing applications were, despite
that being a very bad idea, relying on them.
> > adoption, as well as being a manifestation of the more general
> > problem of libvirt's default being sometimes too conservative and at
> > odds with the existence of slimmed-down QEMU binaries being built
> > with reducing the total attack surface in mind.
>
> If your qemu binary does not support certain feature, libvirt will know
> it. We have capability detection and for that matter we also have
> machine type detection (we fill in the default according to the
> canonical name). In such case we are very welcome to choose anything
> which will satisfy the default.
>
> I'm afraid though that the downstreams you are mentioning can't in fact
> fully drop i440fx for some reason and thus are trying to weasel around
> by attempting to make us change the default. This I don't support as a
> worthy goal. If they want to slim down qemu, they are welcome and we can
> pick a suitable different default.
q35 is what sparked the discussion, but it's far from the only
offender. For example, if I create a guest using
$ virt-install \
--os-variant fedora28 \
...
then libosinfo will be queried for information about supported
network cards, and the resulting XML will look like
<interface type='network'>
<model type='virtio'/>
...
However, libvirt's own default for x86_64 guests' network devices is
rtl8139, which means that if I later 'virsh edit' the guest or 'virsh
attach-device' a new network interface I will get that model instead
of virtio; to add insult to injury, the above will happen even if my
QEMU binary has rtl8139 compiled out and virtio-net-pci compiled in!
That's not correct actually. If rtl8139 is missing in QEMU, libvirt
will try e1000, and if that's missing it'll try virtio-net.
> We can also consider using what qemu provides as a default.
Users will
> get the default they asked for. They always can specify their specific
> type if their software is not happy with it.
Using QEMU's default machine type is exactly what we were doing until
very recently, but we changed that because QEMU's default has been
i440fx for so long that applications have come to rely on it and
would break if q35 suddenly started showing up instead, which goes to
show that they should not have been relying on either QEMU's or
libvirt's default, and they should have been providing the machine
type explicitly, possibly as obtained by querying libosinfo, instead.
Or, looking at it from the other side, that libvirt should have
required them to provide the machine type instead of trying to be
helpful and filling it in if absent. We can't retroactively mandate
applications do that, but we can deprecate such behavior and thus
steer them firmly towards the proper solution.
The problem with saying applications were doing it "wrong" is that
this definition of "wrong" changes. Applications were perfectly
justified in not providing a machine type, because the concept
didn't even exist in earlier libvirt. Once it did exist, we still
only supported x86, and there was no q35, so it was still valid to
not specify it.
Even today it is reasonable to not care about machine type in case
where the app only cares about x86.
Our view of "best" way to configure a guest is changing and in many
cases it is becoming increasingly clear that there's no single
"best" way, or no single perfect default.
Taking something that's historically optional and saying it should
be mandatory is a defacto API breakage. Deprecating it doesn't
magically stop it being an API breakage. It is just giving apps
a warning that we're about to hurt them, and I don't consider that
a good thing.
I think we're largely missing the bigger picture here. Configuring
guests, and using libvirt APIs in general, can be very complicated.
We provide basic API contract docs, and a crude XML schema reference,
but this is woefully insufficient. There's very little telling apps
about the big picture way to configure things / implement tasks.
We're missing a good developer guide where you'd give info to apps
devs about how to effectively use libvirt, so it is no surprise that
apps do things that are sub-optimal. Providing better docs to app
devs would be far more useful than deprecation warnings which have
minimal contextual guidance.
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 :|
9:49 a.m.
On Thu, 2018-10-04 at 14:13 +0100, Daniel P. Berrangé wrote:
On Tue, Oct 02, 2018 at 06:26:12PM +0200, Andrea Bolognani wrote:
> However, libvirt's own default for x86_64 guests' network devices is
> rtl8139, which means that if I later 'virsh edit' the guest or 'virsh
> attach-device' a new network interface I will get that model instead
> of virtio; to add insult to injury, the above will happen even if my
> QEMU binary has rtl8139 compiled out and virtio-net-pci compiled in!
That's not correct actually. If rtl8139 is missing in QEMU, libvirt
will try e1000, and if that's missing it'll try virtio-net.
Welp, you're right. Quite embarrassing that I got it wrong, too,
considering that I am the one who implemented this behavior in the
first place O:-)
The larger point still stands, though: you pretty much never want
to see an rtl8139 device popping up in your guest these days, and
yet it's awfully easy for that to happen which, in my opinion, is
a problem we should address.
> > We can also consider using what qemu provides as a default.
Users will
> > get the default they asked for. They always can specify their specific
> > type if their software is not happy with it.
>
> Using QEMU's default machine type is exactly what we were doing until
> very recently, but we changed that because QEMU's default has been
> i440fx for so long that applications have come to rely on it and
> would break if q35 suddenly started showing up instead, which goes to
> show that they should not have been relying on either QEMU's or
> libvirt's default, and they should have been providing the machine
> type explicitly, possibly as obtained by querying libosinfo, instead.
>
> Or, looking at it from the other side, that libvirt should have
> required them to provide the machine type instead of trying to be
> helpful and filling it in if absent. We can't retroactively mandate
> applications do that, but we can deprecate such behavior and thus
> steer them firmly towards the proper solution.
The problem with saying applications were doing it "wrong" is that
this definition of "wrong" changes. Applications were perfectly
justified in not providing a machine type, because the concept
didn't even exist in earlier libvirt. Once it did exist, we still
only supported x86, and there was no q35, so it was still valid to
not specify it.
Sure, that design decision made complete sense in the context it
was made in.
But the context has changed in a way that turned it from an asset
into a liability, and pretending that's not the case will certainly
not solve the issue.
Our view of "best" way to configure a guest is changing and
in many
cases it is becoming increasingly clear that there's no single
"best" way, or no single perfect default.
I completely agree here.
Taking something that's historically optional and saying it
should
be mandatory is a defacto API breakage. Deprecating it doesn't
magically stop it being an API breakage. It is just giving apps
a warning that we're about to hurt them, and I don't consider that
a good thing.
I disagree: for better or worse, features being deprecated and
ultimately removed is one of the realities of software development,
and I see very little motivation for libvirt to try and play by
different rules.
I think we're largely missing the bigger picture here.
Configuring
guests, and using libvirt APIs in general, can be very complicated.
We provide basic API contract docs, and a crude XML schema reference,
but this is woefully insufficient. There's very little telling apps
about the big picture way to configure things / implement tasks.
We're missing a good developer guide where you'd give info to apps
devs about how to effectively use libvirt, so it is no surprise that
apps do things that are sub-optimal. Providing better docs to app
devs would be far more useful than deprecation warnings which have
minimal contextual guidance.
I agree that having better documentation would help, and we should
definitely work towards that goal.
However, I'm fairly confident trying to address issues through
documentation only will not be enough to get applications off
problematic API usage: people mentioned several times elsewhere in
the thread how they think *emitting warnings* itself wouldn't be
enough, and developers would only take notice once the application
breaks for good! How is documentation going to be effective? Who
would look at documentation periodically just to make sure they're
not using features that have since been deprecated? I know I don't
do that for any of libvirt's dependencies, but if I started getting
warnings I'd certainly take notice.
Unless users are nagged about it during usage and developers are
nagged about it during development, usage of problematic APIs will
never go away.
--
Andrea Bolognani / Red Hat / Virtualization
10:04 a.m.
On Thu, Oct 04, 2018 at 04:49:43PM +0200, Andrea Bolognani wrote:
On Thu, 2018-10-04 at 14:13 +0100, Daniel P. Berrangé wrote:
> I think we're largely missing the bigger picture here. Configuring
> guests, and using libvirt APIs in general, can be very complicated.
>
> We provide basic API contract docs, and a crude XML schema reference,
> but this is woefully insufficient. There's very little telling apps
> about the big picture way to configure things / implement tasks.
>
> We're missing a good developer guide where you'd give info to apps
> devs about how to effectively use libvirt, so it is no surprise that
> apps do things that are sub-optimal. Providing better docs to app
> devs would be far more useful than deprecation warnings which have
> minimal contextual guidance.
I agree that having better documentation would help, and we should
definitely work towards that goal.
However, I'm fairly confident trying to address issues through
documentation only will not be enough to get applications off
problematic API usage: people mentioned several times elsewhere in
the thread how they think *emitting warnings* itself wouldn't be
enough, and developers would only take notice once the application
breaks for good! How is documentation going to be effective? Who
would look at documentation periodically just to make sure they're
not using features that have since been deprecated? I know I don't
do that for any of libvirt's dependencies, but if I started getting
warnings I'd certainly take notice.
Unless users are nagged about it during usage and developers are
nagged about it during development, usage of problematic APIs will
never go away.
I'm not concerned if usage doesn't go away entirely, because I believe
in our goal to preserve API compatibility indefinitely. If an application
is using an API or feature & it works for them that is fine. Even if the
API has known design flaws, that isn't a problem unless those flaws are
relevant to the usage scenario of the app
I'm much more interested in how we can help application developers pick
the most effective approaches to using libvirt, and a couple of lines in
a deprecation message are not effective for this.
A large part of this thread is based off the fact that apps are blindly
relying on defaults. This is bad because no single default is a good
choice for all scenarios. What's needed is guidance on how to effectively
configure guests using libosinfo to identify suitable choices. This means
explaining to apps how libosinfo fits into the picture when using libvirt,
and describing the caveats around the choices of machine types, or the
choices of CPU models. There's no escaping the writing of detailed docs
to get across these points.
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 :|
3:59 a.m.
On Thu, 2018-10-04 at 16:04 +0100, Daniel P. Berrangé wrote:
On Thu, Oct 04, 2018 at 04:49:43PM +0200, Andrea Bolognani wrote:
> I agree that having better documentation would help, and we should
> definitely work towards that goal.
>
> However, I'm fairly confident trying to address issues through
> documentation only will not be enough to get applications off
> problematic API usage: people mentioned several times elsewhere in
> the thread how they think *emitting warnings* itself wouldn't be
> enough, and developers would only take notice once the application
> breaks for good! How is documentation going to be effective? Who
> would look at documentation periodically just to make sure they're
> not using features that have since been deprecated? I know I don't
> do that for any of libvirt's dependencies, but if I started getting
> warnings I'd certainly take notice.
>
> Unless users are nagged about it during usage and developers are
> nagged about it during development, usage of problematic APIs will
> never go away.
I'm not concerned if usage doesn't go away entirely, because I believe
in our goal to preserve API compatibility indefinitely.
I don't see that as a goal per se, but rather as a *mean* to
achieve goals such as ensuring applications don't break on users
during upgrade and application developers don't have to work harder
than necessary to keep up with changes in libvirt.
It's not, however, quite a silver bullet because, for all the good
it does, it also introduces several issues, some of which have been
mentioned in this thread.
I believe a better balance would be achieved by guaranteeing *long
term* API/ABI stability, in the order of several years, so that
application developers and users still benefit from not having
changes forced on them too frequently but we also have the chance
to clean up after our mistakes from time to time, which again would
not only help libvirt developers but also, indirectly, users and
application developers too.
If an application
is using an API or feature & it works for them that is fine. Even if the
API has known design flaws, that isn't a problem unless those flaws are
relevant to the usage scenario of the app
Usage scenarios can and will change.
Someone earlier in the thread used the example of a simple
application that works just fine despite using racy APIs because
there's only one person starting and stopping guests: what happens
when a second admin enters the picture? Now you have to spend time
figuring out where the (seldomly occurring) issue comes from and
ultimately fix it by switching to the non-racy API.
Wouldn't it have been much better if using the racy API had raised
warnings from the very beginning, causing the developer to switch
to the non-racy one before hitting any issues?
Also, circling back to the machine type conundrum, what if the
application or deployment assume guests will be i440fx but at some
point a QEMU binary with i440fx compiled out is installed on the
system? Suddenly what used to work doesn't work anymore despite
the application itself sticking to "what works for them".
I'm much more interested in how we can help application
developers pick
the most effective approaches to using libvirt, and a couple of lines in
a deprecation message are not effective for this.
While a short "this is deprecated" message won't give application
developers or users the whole story, it will certainly give them
motivation to look up the details in the documentation.
A large part of this thread is based off the fact that apps are
blindly
relying on defaults. This is bad because no single default is a good
choice for all scenarios. What's needed is guidance on how to effectively
configure guests using libosinfo to identify suitable choices. This means
explaining to apps how libosinfo fits into the picture when using libvirt,
and describing the caveats around the choices of machine types, or the
choices of CPU models. There's no escaping the writing of detailed docs
to get across these points.
Do we expect virsh users to also, unprompted, go out of their way
to read documentation targeted at application developers?
--
Andrea Bolognani / Red Hat / Virtualization
3:34 a.m.
On Thu, Oct 04, 2018 at 14:13:41 +0100, Daniel P. Berrangé wrote:
The problem with saying applications were doing it "wrong"
is that
this definition of "wrong" changes. Applications were perfectly
justified in not providing a machine type, because the concept
didn't even exist in earlier libvirt. Once it did exist, we still
only supported x86, and there was no q35, so it was still valid to
not specify it.
Even today it is reasonable to not care about machine type in case
where the app only cares about x86.
Our view of "best" way to configure a guest is changing and in many
cases it is becoming increasingly clear that there's no single
"best" way, or no single perfect default.
As I tried to explain in another mail in this thread, I definitely agree
we should not change defaults just because we think some value is better
now. That's really a job for libosinfo.
But what if QEMU (or any other hypervisor) marks something (device
model, machine type) as deprecated and we use that deprecated value as
our default. Shouldn't we be able to tell about it to our users (here
runtime warnings could help) when they use such thing explicitly and
choose a different default value? That would help users with the
transition and once hypervisor drops support for it completely, fewer
existing domains will be affected since the recently created ones would
already use non-deprecated defaults.
We already silently do this to some extent with machine types. If a
users specifies "pc" or even nothing, we translate it to the current
QEMU default machine type "pc-i440fx-*".
So taking "q35" as an example, if QEMU marks i440fx machine types as
deprecated (they seem to be thinking about it) I don't see that big
difference[*] in changing the default machine type to "q35". After all
we will automatically change the default once i440fx is removed
completely.
[*] sure some devices don't even exist in q35 machine types some domain
XMLs may not work anymore while with just newer i440fx machine type the
XML would just keep working. But the guest OS will see a different
hardware anyway and may be unhappy about it.
Jirka
3:54 a.m.
On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
On Thu, Oct 04, 2018 at 14:13:41 +0100, Daniel P. Berrangé wrote:
> The problem with saying applications were doing it "wrong" is that
> this definition of "wrong" changes. Applications were perfectly
> justified in not providing a machine type, because the concept
> didn't even exist in earlier libvirt. Once it did exist, we still
> only supported x86, and there was no q35, so it was still valid to
> not specify it.
>
> Even today it is reasonable to not care about machine type in case
> where the app only cares about x86.
>
> Our view of "best" way to configure a guest is changing and in many
> cases it is becoming increasingly clear that there's no single
> "best" way, or no single perfect default.
As I tried to explain in another mail in this thread, I definitely agree
we should not change defaults just because we think some value is better
now. That's really a job for libosinfo.
But what if QEMU (or any other hypervisor) marks something (device
model, machine type) as deprecated and we use that deprecated value as
our default. Shouldn't we be able to tell about it to our users (here
runtime warnings could help) when they use such thing explicitly and
choose a different default value? That would help users with the
transition and once hypervisor drops support for it completely, fewer
existing domains will be affected since the recently created ones would
already use non-deprecated defaults.
QEMU already emits warnings on stderr whenever something that is
deprecated is used, and those end up in the libvirt log file for
that guest. I don't think that we need to duplicate what QEMU
is already reporting again.
It is not the kind of think an application will dynamically take
action on at runtime. It is something application developers need
to be told about so they can adapt future code releases as needed.
So taking "q35" as an example, if QEMU marks i440fx machine
types as
deprecated (they seem to be thinking about it) I don't see that big
difference[*] in changing the default machine type to "q35". After all
we will automatically change the default once i440fx is removed
completely.
There's no active plans to deprecate or remove i440fx upstream in
QEMU. If a downstream did remove it, then clearly that'll impose
some level of pain on applications.
This is something where better documentation would help application
developers. We report on supported machine types in capabilities
XML but few applications do anything useful with this. We're also
intending to add support to libosinfo to report on machine type
compatibility for OS.
We've got nothing that explains to applications how all this
functionality fits together, nor good description of application
relevant differences between i440fx & q35. So it is not surprising
that code apps have around machine types is very sub-par and liable
to break.
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 :|
4:38 a.m.
On Fri, 2018-10-05 at 09:54 +0100, Daniel P. Berrangé wrote:
On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
> But what if QEMU (or any other hypervisor) marks something (device
> model, machine type) as deprecated and we use that deprecated value as
> our default. Shouldn't we be able to tell about it to our users (here
> runtime warnings could help) when they use such thing explicitly and
> choose a different default value? That would help users with the
> transition and once hypervisor drops support for it completely, fewer
> existing domains will be affected since the recently created ones would
> already use non-deprecated defaults.
QEMU already emits warnings on stderr whenever something that is
deprecated is used, and those end up in the libvirt log file for
that guest. I don't think that we need to duplicate what QEMU
is already reporting again.
Warnings printed on stderr -> users and developers will actually
see them, be annoyed by them, eventually cave in and act upon them.
Warnings written to a log -> nobody will notice them, until one day
things suddenly stop working apparently out of the blue.
We might pretend that's not the case, but really, it is.
> So taking "q35" as an example, if QEMU marks i440fx
machine types as
> deprecated (they seem to be thinking about it) I don't see that big
> difference[*] in changing the default machine type to "q35". After all
> we will automatically change the default once i440fx is removed
> completely.
There's no active plans to deprecate or remove i440fx upstream in
QEMU. If a downstream did remove it, then clearly that'll impose
some level of pain on applications.
While i440fx itself has not been deprecated and might possibly
never be, other machine types (x86's pc-0.10 and pc-0.11, ppc's
prep) are in fact already marked as deprecated and will eventually
be dropped entirely from QEMU.
--
Andrea Bolognani / Red Hat / Virtualization
4:56 a.m.
On Fri, Oct 05, 2018 at 11:38:14AM +0200, Andrea Bolognani wrote:
On Fri, 2018-10-05 at 09:54 +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
> > But what if QEMU (or any other hypervisor) marks something (device
> > model, machine type) as deprecated and we use that deprecated value as
> > our default. Shouldn't we be able to tell about it to our users (here
> > runtime warnings could help) when they use such thing explicitly and
> > choose a different default value? That would help users with the
> > transition and once hypervisor drops support for it completely, fewer
> > existing domains will be affected since the recently created ones would
> > already use non-deprecated defaults.
>
> QEMU already emits warnings on stderr whenever something that is
> deprecated is used, and those end up in the libvirt log file for
> that guest. I don't think that we need to duplicate what QEMU
> is already reporting again.
Warnings printed on stderr -> users and developers will actually
see them, be annoyed by them, eventually cave in and act upon them.
Warnings written to a log -> nobody will notice them, until one day
things suddenly stop working apparently out of the blue.
We might pretend that's not the case, but really, it is.
You cannot expect that libvirt will duplicate all the deprecation
warnings from all hypervisors for all models or machine types or other
values that can be set to a lot of different attributes in our XML.
Once we do that for machine type there will be requests to do it for
other attributes as well. I think that this is a big NO and way out
of scope of libvirt.
Pavel
6:23 a.m.
On Fri, 2018-10-05 at 11:56 +0200, Pavel Hrdina wrote:
On Fri, Oct 05, 2018 at 11:38:14AM +0200, Andrea Bolognani wrote:
> Warnings printed on stderr -> users and developers will actually
> see them, be annoyed by them, eventually cave in and act upon them.
>
> Warnings written to a log -> nobody will notice them, until one day
> things suddenly stop working apparently out of the blue.
>
> We might pretend that's not the case, but really, it is.
You cannot expect that libvirt will duplicate all the deprecation
warnings from all hypervisors for all models or machine types or other
values that can be set to a lot of different attributes in our XML.
Once we do that for machine type there will be requests to do it for
other attributes as well. I think that this is a big NO and way out
of scope of libvirt.
Of course not, that would become very silly very fast - although
in many cases we already go out of our way to report a libvirt
error for configurations that would cause QEMU to ultimately bail
out anyway, so it wouldn't be entirely unprecedented either.
What I meant to say is that we should report deprecations of
*libvirt* features to the user directly instead of stuffing them
into a log file where they will possibly never be noticed until
it's way too late.
--
Andrea Bolognani / Red Hat / Virtualization
6:05 a.m.
On Fri, Oct 05, 2018 at 11:38:14AM +0200, Andrea Bolognani wrote:
On Fri, 2018-10-05 at 09:54 +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
> > But what if QEMU (or any other hypervisor) marks something (device
> > model, machine type) as deprecated and we use that deprecated value as
> > our default. Shouldn't we be able to tell about it to our users (here
> > runtime warnings could help) when they use such thing explicitly and
> > choose a different default value? That would help users with the
> > transition and once hypervisor drops support for it completely, fewer
> > existing domains will be affected since the recently created ones would
> > already use non-deprecated defaults.
>
> QEMU already emits warnings on stderr whenever something that is
> deprecated is used, and those end up in the libvirt log file for
> that guest. I don't think that we need to duplicate what QEMU
> is already reporting again.
Warnings printed on stderr -> users and developers will actually
see them, be annoyed by them, eventually cave in and act upon them.
Warnings written to a log -> nobody will notice them, until one day
things suddenly stop working apparently out of the blue.
We might pretend that's not the case, but really, it is.
Unless you're talking about a CLI tool (virt-install, virsh), there
is no difference between those two scenarios. For virt-manager,
virt-viewer, oVirt, OpenStack, KubeVirt, stderr is never going to
be seen, it just ends up in a log file. So I don't find that
distinction to be compelling.
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 :|
6:43 a.m.
On Fri, 2018-10-05 at 12:05 +0100, Daniel P. Berrangé wrote:
On Fri, Oct 05, 2018 at 11:38:14AM +0200, Andrea Bolognani wrote:
> Warnings printed on stderr -> users and developers will actually
> see them, be annoyed by them, eventually cave in and act upon them.
>
> Warnings written to a log -> nobody will notice them, until one day
> things suddenly stop working apparently out of the blue.
>
> We might pretend that's not the case, but really, it is.
Unless you're talking about a CLI tool (virt-install, virsh), there
is no difference between those two scenarios. For virt-manager,
virt-viewer, oVirt, OpenStack, KubeVirt, stderr is never going to
be seen, it just ends up in a log file. So I don't find that
distinction to be compelling.
Sure, I used "stderr" as a shorthand for "whatever reporting method
can be appropriately used by the application to shove a warning to
the user's face" :)
For a GUI application, it might be a dialog popping up or a message
showing scrolling throuhg the status bar or what have you; for
something headless, it might be an email delivered to the admin's
inbox.
The point is that the user should *not* be required to dig through
logs to find out they've been using deprecated features: they should
be *told* that's the case right when they do.
--
Andrea Bolognani / Red Hat / Virtualization
5:33 a.m.
On Fri, Oct 05, 2018 at 09:54:02 +0100, Daniel P. Berrangé wrote:
On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
> On Thu, Oct 04, 2018 at 14:13:41 +0100, Daniel P. Berrangé wrote:
> > The problem with saying applications were doing it "wrong" is that
> > this definition of "wrong" changes. Applications were perfectly
> > justified in not providing a machine type, because the concept
> > didn't even exist in earlier libvirt. Once it did exist, we still
> > only supported x86, and there was no q35, so it was still valid to
> > not specify it.
> >
> > Even today it is reasonable to not care about machine type in case
> > where the app only cares about x86.
> >
> > Our view of "best" way to configure a guest is changing and in many
> > cases it is becoming increasingly clear that there's no single
> > "best" way, or no single perfect default.
>
> As I tried to explain in another mail in this thread, I definitely agree
> we should not change defaults just because we think some value is better
> now. That's really a job for libosinfo.
>
> But what if QEMU (or any other hypervisor) marks something (device
> model, machine type) as deprecated and we use that deprecated value as
> our default. Shouldn't we be able to tell about it to our users (here
> runtime warnings could help) when they use such thing explicitly and
> choose a different default value? That would help users with the
> transition and once hypervisor drops support for it completely, fewer
> existing domains will be affected since the recently created ones would
> already use non-deprecated defaults.
QEMU already emits warnings on stderr whenever something that is
deprecated is used, and those end up in the libvirt log file for
that guest. I don't think that we need to duplicate what QEMU
is already reporting again.
It is not the kind of think an application will dynamically take
action on at runtime. It is something application developers need
to be told about so they can adapt future code releases as needed.
OK, but if it's us choosing some default which QEMU considers
deprecated, it's us who should dealt with it. That is, change the
default IMHO. Sure apps could just stop relying on defaults and choose
something specific which is not deprecated, but that doesn't mean we as
a user of QEMU should continue using default values which got deprecated
and are likely to be removed.
If it's something which is not user/guest visible, we are already trying
to move to more modern interfaces. That is easy since it doesn't change
anything for libvirt users, but I feel like we need to have a plan for
stuff which actually is visible to our users.
> So taking "q35" as an example, if QEMU marks i440fx
machine types as
> deprecated (they seem to be thinking about it) I don't see that big
> difference[*] in changing the default machine type to "q35". After all
> we will automatically change the default once i440fx is removed
> completely.
There's no active plans to deprecate or remove i440fx upstream in
QEMU. If a downstream did remove it, then clearly that'll impose
some level of pain on applications.
I wasn't talking about downstream and I probably gave a wrong example
and mixed several different things, but the point stays valid. We should
have some sort of a policy when and how we change default values rather
than saying "we never do that except when we already did so". One of the
reason why we store chosen defaults in domain XML is to be able to
change the defaults without breaking existing domains. We should
formally touch this area in our "Support guarantees" document
(https://libvirt.org/support.html) so that users/apps know what they can
expect and we don't have to think or fight everytime someone comes up
with an idea to change a default value.
Jirka
6:10 a.m.
On Fri, Oct 05, 2018 at 12:33:57PM +0200, Jiri Denemark wrote:
On Fri, Oct 05, 2018 at 09:54:02 +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
> > On Thu, Oct 04, 2018 at 14:13:41 +0100, Daniel P. Berrangé wrote:
> > > The problem with saying applications were doing it "wrong" is
that
> > > this definition of "wrong" changes. Applications were perfectly
> > > justified in not providing a machine type, because the concept
> > > didn't even exist in earlier libvirt. Once it did exist, we still
> > > only supported x86, and there was no q35, so it was still valid to
> > > not specify it.
> > >
> > > Even today it is reasonable to not care about machine type in case
> > > where the app only cares about x86.
> > >
> > > Our view of "best" way to configure a guest is changing and in
many
> > > cases it is becoming increasingly clear that there's no single
> > > "best" way, or no single perfect default.
> >
> > As I tried to explain in another mail in this thread, I definitely agree
> > we should not change defaults just because we think some value is better
> > now. That's really a job for libosinfo.
> >
> > But what if QEMU (or any other hypervisor) marks something (device
> > model, machine type) as deprecated and we use that deprecated value as
> > our default. Shouldn't we be able to tell about it to our users (here
> > runtime warnings could help) when they use such thing explicitly and
> > choose a different default value? That would help users with the
> > transition and once hypervisor drops support for it completely, fewer
> > existing domains will be affected since the recently created ones would
> > already use non-deprecated defaults.
>
> QEMU already emits warnings on stderr whenever something that is
> deprecated is used, and those end up in the libvirt log file for
> that guest. I don't think that we need to duplicate what QEMU
> is already reporting again.
>
> It is not the kind of think an application will dynamically take
> action on at runtime. It is something application developers need
> to be told about so they can adapt future code releases as needed.
OK, but if it's us choosing some default which QEMU considers
deprecated, it's us who should dealt with it. That is, change the
default IMHO. Sure apps could just stop relying on defaults and choose
something specific which is not deprecated, but that doesn't mean we as
a user of QEMU should continue using default values which got deprecated
and are likely to be removed.
If it's something which is not user/guest visible, we are already trying
to move to more modern interfaces. That is easy since it doesn't change
anything for libvirt users, but I feel like we need to have a plan for
stuff which actually is visible to our users.
If some user visible feature is actually removed, then our hand is
forced - we have to pick something else to replace it. This is what
happened with the default NIC with RHEL stopped building rtl8139,
so we had to have a default fallback.
That change is/was not seemless since there exist OS which supported
rtl8139 but which lacked e1000 / virtio-net drivers. Fortunately apps
already use libosinfo to explicitly set NIC model so the breakage was
not so visible.
If 'i440fx' is ever actually deleted we would have todo something
similar - prefer i440fx for best historical compat, but fallback
to q35 despite knowing it could break.
> > So taking "q35" as an example, if QEMU marks
i440fx machine types as
> > deprecated (they seem to be thinking about it) I don't see that big
> > difference[*] in changing the default machine type to "q35". After
all
> > we will automatically change the default once i440fx is removed
> > completely.
>
> There's no active plans to deprecate or remove i440fx upstream in
> QEMU. If a downstream did remove it, then clearly that'll impose
> some level of pain on applications.
I wasn't talking about downstream and I probably gave a wrong example
and mixed several different things, but the point stays valid. We should
have some sort of a policy when and how we change default values rather
than saying "we never do that except when we already did so". One of the
reason why we store chosen defaults in domain XML is to be able to
change the defaults without breaking existing domains. We should
formally touch this area in our "Support guarantees" document
(https://libvirt.org/support.html) so that users/apps know what they can
expect and we don't have to think or fight everytime someone comes up
with an idea to change a default value.
I guess we should outline the NIC default choice as an example of the
approach we would take.
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 :|
6:28 a.m.
On Fri, Oct 05, 2018 at 12:10:51 +0100, Daniel P. Berrangé wrote:
On Fri, Oct 05, 2018 at 12:33:57PM +0200, Jiri Denemark wrote:
> On Fri, Oct 05, 2018 at 09:54:02 +0100, Daniel P. Berrangé wrote:
> > On Fri, Oct 05, 2018 at 10:34:33AM +0200, Jiri Denemark wrote:
> > > On Thu, Oct 04, 2018 at 14:13:41 +0100, Daniel P. Berrangé wrote:
> > > > The problem with saying applications were doing it "wrong"
is that
> > > > this definition of "wrong" changes. Applications were
perfectly
> > > > justified in not providing a machine type, because the concept
> > > > didn't even exist in earlier libvirt. Once it did exist, we
still
> > > > only supported x86, and there was no q35, so it was still valid to
> > > > not specify it.
> > > >
> > > > Even today it is reasonable to not care about machine type in case
> > > > where the app only cares about x86.
> > > >
> > > > Our view of "best" way to configure a guest is changing and
in many
> > > > cases it is becoming increasingly clear that there's no single
> > > > "best" way, or no single perfect default.
> > >
> > > As I tried to explain in another mail in this thread, I definitely agree
> > > we should not change defaults just because we think some value is better
> > > now. That's really a job for libosinfo.
> > >
> > > But what if QEMU (or any other hypervisor) marks something (device
> > > model, machine type) as deprecated and we use that deprecated value as
> > > our default. Shouldn't we be able to tell about it to our users (here
> > > runtime warnings could help) when they use such thing explicitly and
> > > choose a different default value? That would help users with the
> > > transition and once hypervisor drops support for it completely, fewer
> > > existing domains will be affected since the recently created ones would
> > > already use non-deprecated defaults.
> >
> > QEMU already emits warnings on stderr whenever something that is
> > deprecated is used, and those end up in the libvirt log file for
> > that guest. I don't think that we need to duplicate what QEMU
> > is already reporting again.
> >
> > It is not the kind of think an application will dynamically take
> > action on at runtime. It is something application developers need
> > to be told about so they can adapt future code releases as needed.
>
> OK, but if it's us choosing some default which QEMU considers
> deprecated, it's us who should dealt with it. That is, change the
> default IMHO. Sure apps could just stop relying on defaults and choose
> something specific which is not deprecated, but that doesn't mean we as
> a user of QEMU should continue using default values which got deprecated
> and are likely to be removed.
>
> If it's something which is not user/guest visible, we are already trying
> to move to more modern interfaces. That is easy since it doesn't change
> anything for libvirt users, but I feel like we need to have a plan for
> stuff which actually is visible to our users.
If some user visible feature is actually removed, then our hand is
forced - we have to pick something else to replace it. This is what
happened with the default NIC with RHEL stopped building rtl8139,
so we had to have a default fallback.
That change is/was not seemless since there exist OS which supported
rtl8139 but which lacked e1000 / virtio-net drivers. Fortunately apps
already use libosinfo to explicitly set NIC model so the breakage was
not so visible.
If 'i440fx' is ever actually deleted we would have todo something
similar - prefer i440fx for best historical compat, but fallback
to q35 despite knowing it could break.
We already do so. If QEMU supports "pc", we use that as the default (and
resolve it to some pc-i440fx-* machine type), otherwise we use whatever
QEMU says is the default.
My point is whether we should wait until we're forced or switch the
default once it's deprecated to make sure newly created domains get the
new default early and they will keep working once the old default is
removed.
Of course, changes to the XML or guest configuration may be needed for
the new default to actually work, but I think it's better to do the
changes when creating new domains rather than realizing that suddenly
all my existing domains won't boot anymore because the default value
they got when I created them was removed.
Jirka
7:19 a.m.
On Fri, 2018-10-05 at 12:10 +0100, Daniel P. Berrangé wrote:
On Fri, Oct 05, 2018 at 12:33:57PM +0200, Jiri Denemark wrote:
> OK, but if it's us choosing some default which QEMU considers
> deprecated, it's us who should dealt with it. That is, change the
> default IMHO. Sure apps could just stop relying on defaults and choose
> something specific which is not deprecated, but that doesn't mean we as
> a user of QEMU should continue using default values which got deprecated
> and are likely to be removed.
>
> If it's something which is not user/guest visible, we are already trying
> to move to more modern interfaces. That is easy since it doesn't change
> anything for libvirt users, but I feel like we need to have a plan for
> stuff which actually is visible to our users.
If some user visible feature is actually removed, then our hand is
forced - we have to pick something else to replace it. This is what
happened with the default NIC with RHEL stopped building rtl8139,
so we had to have a default fallback.
That change is/was not seemless since there exist OS which supported
rtl8139 but which lacked e1000 / virtio-net drivers. Fortunately apps
already use libosinfo to explicitly set NIC model so the breakage was
not so visible.
If 'i440fx' is ever actually deleted we would have todo something
similar - prefer i440fx for best historical compat, but fallback
to q35 despite knowing it could break.
I think this is just running in circles around the issue.
Once you've determined that there is no sensible default that works
for everyone, and that whatever works now might not work in the
future, so applications should provide a value instead of relying
on the default if they don't want to break down the line, you've
made the default for all intents and purposes entirely pointless.
Yet you've stopped just short of actually addressing the underlying
issue, which you would do by (after a generous deprecation period)
*requiring* applications to provide an explicit value at all times.
--
Andrea Bolognani / Red Hat / Virtualization
6:09 a.m.
Picking up the thread again from here because it looks like as
sensible a point as any.
On Tue, 2018-10-02 at 18:26 +0200, Andrea Bolognani wrote:
q35 is what sparked the discussion, but it's far from the only
offender. For example, if I create a guest using
$ virt-install \
--os-variant fedora28 \
...
then libosinfo will be queried for information about supported
network cards, and the resulting XML will look like
<interface type='network'>
<model type='virtio'/>
...
However, libvirt's own default for x86_64 guests' network devices is
rtl8139, which means that if I later 'virsh edit' the guest or 'virsh
attach-device' a new network interface I will get that model instead
of virtio; [...]
I'd say at this point it's fair to conclude that the consensus
among libvirt developers is to stick to the existing compatibility
promise and thus never deprecate, let alone drop, features; the
question of whether defaults could ever change hasn't been answered
in such a clear-cut manner, as far as I can tell, but it's also
somewhat less relevant because I think we can all agree that having
a single default value is simply not going to cut it these days.
So applications are supposed to provide explicit values, obtained
through libosinfo, so that they will keep working as other parts
of the stack change around them; this will, however, not be
enforced in any way but rather encouraged through documentation.
This is a sensible enough plan, but it fails to address one
important scenario: people using virsh directly to script changes
and other operations on their guests - see above for an example of
how that can end up not working as expected.
Since virsh itself is basically just exposing raw libvirt APIs,
it doesn't and shouldn't have much logic of its own, plus it falls
under the same stability guarantees as the library itself, so we
can't easily change it.
The obvious solution is to have a *different* command line client
that can alter guests the same way virsh can, but is not shipped
as part of libvirt and can thus link against libosinfo as well as
not having to follow the same strict stability guarantees.
Pavel helpfully pointed out that such a client already exists:
it's called virt-xml and it's part of virt-manager.
It needs a few tweaks before it can really fit the bill, but once
that's been taken care of you should be able to use something like
$ virt-xml \
myguest \
--os-variant fedora28 \
--add-device \
--network network=default
and get a virtio-net device, as you should, instead of rtl8139.
Actually accepting --os-variant is the missing bit, but it should
be fairly easy to do; not only that, but the existing proposal to
store libosinfo metadata in the guest XML during installation has
the potential to make even that entirely unnecessary!
For installation you'll obviously want to use virt-install, but
that's the case already since actually installing a guest from
scratch using 'virsh define' is just madness :) IIUC the use case
of importing an existing guest image without booting the guest at
the same time is not covered, but once again that's only a bugfix
away.
So once we have these changes in place, command line users can be
pretty much completely isolated from libvirt defaults, just like
virt-manager and oVirt and Nova users. Then it will be up to us
to actually advertise these alternatives and push users away from
virsh[1] and towards them.
I wonder if showing a message suggesting to use virt-xml instead
when 'virsh edit' or 'virsh attach-device' are called would be
considered acceptable at that point?
[1] Only where it makes sense, of course: 'virsh start' and
similar, for example, don't really need an alternative.
--
Andrea Bolognani / Red Hat / Virtualization
10:21 a.m.
On Wed, Oct 10, 2018 at 01:09:50PM +0200, Andrea Bolognani wrote:
Pavel helpfully pointed out that such a client already exists:
it's called virt-xml and it's part of virt-manager.
It needs a few tweaks before it can really fit the bill, but once
that's been taken care of you should be able to use something like
$ virt-xml \
myguest \
--os-variant fedora28 \
--add-device \
--network network=default
and get a virtio-net device, as you should, instead of rtl8139.
Actually accepting --os-variant is the missing bit, but it should
be fairly easy to do; not only that, but the existing proposal to
store libosinfo metadata in the guest XML during installation has
the potential to make even that entirely unnecessary!
For installation you'll obviously want to use virt-install, but
that's the case already since actually installing a guest from
scratch using 'virsh define' is just madness :) IIUC the use case
of importing an existing guest image without booting the guest at
the same time is not covered, but once again that's only a bugfix
away.
So once we have these changes in place, command line users can be
pretty much completely isolated from libvirt defaults, just like
virt-manager and oVirt and Nova users. Then it will be up to us
to actually advertise these alternatives and push users away from
virsh[1] and towards them.
I wonder if showing a message suggesting to use virt-xml instead
when 'virsh edit' or 'virsh attach-device' are called would be
considered acceptable at that point?
Depends what you mean by showing a message ? I'd be fine with the
virsh man page referring people to virt-xml as a companion tool.
I would certainly not expect invokation of 'virsh edit' to print
any text on the console, as it will always be valid to want to
use "virsh edit", "virsh atach-device" or any other command
precisely because they are an almost direct passthrough to the
libvirt API without trying to inject clever logic of their own.
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 :|
3:07 p.m.
On 10/12/2018 11:21 AM, Daniel P. Berrangé wrote:
On Wed, Oct 10, 2018 at 01:09:50PM +0200, Andrea Bolognani wrote:
> I wonder if showing a message suggesting to use virt-xml instead
> when 'virsh edit' or 'virsh attach-device' are called would be
> considered acceptable at that point?
Depends what you mean by showing a message ? I'd be fine with the
virsh man page referring people to virt-xml as a companion tool.
I would certainly not expect invokation of 'virsh edit' to print
any text on the console, as it will always be valid to want to
use "virsh edit", "virsh atach-device" or any other command
precisely because they are an almost direct passthrough to the
libvirt API without trying to inject clever logic of their own.
What if it was added to the host's MOTD? :-P
https://bugzilla.redhat.com/show_bug.cgi?id=1635200\
6:41 a.m.
On Fri, 2018-10-12 at 16:21 +0100, Daniel P. Berrangé wrote:
On Wed, Oct 10, 2018 at 01:09:50PM +0200, Andrea Bolognani wrote:
> So once we have these changes in place, command line users can be
> pretty much completely isolated from libvirt defaults, just like
> virt-manager and oVirt and Nova users. Then it will be up to us
> to actually advertise these alternatives and push users away from
> virsh[1] and towards them.
>
> I wonder if showing a message suggesting to use virt-xml instead
> when 'virsh edit' or 'virsh attach-device' are called would be
> considered acceptable at that point?
Depends what you mean by showing a message ? I'd be fine with the
virsh man page referring people to virt-xml as a companion tool.
I would certainly not expect invokation of 'virsh edit' to print
any text on the console, as it will always be valid to want to
use "virsh edit", "virsh atach-device" or any other command
precisely because they are an almost direct passthrough to the
libvirt API without trying to inject clever logic of their own.
Okay, let's forget the runtime messages then: we can mention
virt-xml (and virt-install) in the documentation, write blog posts
about them, and the like.
Does the rest of the plan look reasonable to you?
--
Andrea Bolognani / Red Hat / Virtualization
4:14 a.m.
On Sun, Oct 14, 2018 at 01:41:27PM +0200, Andrea Bolognani wrote:
On Fri, 2018-10-12 at 16:21 +0100, Daniel P. Berrangé wrote:
> On Wed, Oct 10, 2018 at 01:09:50PM +0200, Andrea Bolognani wrote:
> > So once we have these changes in place, command line users can be
> > pretty much completely isolated from libvirt defaults, just like
> > virt-manager and oVirt and Nova users. Then it will be up to us
> > to actually advertise these alternatives and push users away from
> > virsh[1] and towards them.
> >
> > I wonder if showing a message suggesting to use virt-xml instead
> > when 'virsh edit' or 'virsh attach-device' are called would be
> > considered acceptable at that point?
>
> Depends what you mean by showing a message ? I'd be fine with the
> virsh man page referring people to virt-xml as a companion tool.
>
> I would certainly not expect invokation of 'virsh edit' to print
> any text on the console, as it will always be valid to want to
> use "virsh edit", "virsh atach-device" or any other command
> precisely because they are an almost direct passthrough to the
> libvirt API without trying to inject clever logic of their own.
Okay, let's forget the runtime messages then: we can mention
virt-xml (and virt-install) in the documentation, write blog posts
about them, and the like.
Does the rest of the plan look reasonable to you?
Sure, I'm fine with docs updates.
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 :|
4:41 p.m.
On 10/2/18 10:14 AM, Andrea Bolognani wrote:
Background
==========
We have plenty of features in libvirt, some of which were designed at
a time when the virtualization story was much more straightforward
than the multi-architecture, multi-hypervisor, multi-machine world we
currently live in and, while we have found ways to keep the APIs
chugging along, the result is sometimes somewhat confusing for users
and application developers, as well as requiring libvirt developers
themselves to spend quite a bit of collective time working around
decisions that, in hindsight, turn out to have been less than
fortunate.
I've read the other responses and as difficult as it is to type - yeah,
it seems we're stuck with what we have. Hard to undo old decisions as
difficult as they make future life. Being stuck with those decisions
makes future adjustments that much harder unless we can find a better
way to fracture things between legacy and current (which yes has it's
own downsides).
Spending time coding and reviewing on "warnings" serves only those that
keep up with the latest and greatest serves what purpose? Until it stops
working, who notices?
Will someone who develops code that hasn't kept up with the advances in
virtualization see, know, or care? They have something that works and
won't change it. They're probably also holding onto their Commodore
VIC-20 hoping for a comeback, too.
I would think the "bulk" of the scope is less old API's and more the
default value problem. Is this a lazy or uninformed consumer problem?
The default value problem seems to be limited to QEMU machine types and
PCI model/type chosen. If QEMU changed to default to q35 and/or removed
i440fx, then IIUC there'd be lots of problems. No small wonder so many
vendors of physical systems go out of business. Bad defaults. That's not
to say those that are left are providing systems with great defaults,
just better than the others. The whole conversation seems to be a very
circular mess/problem. Libvirt's history may be to not make policy
decisions, but it does seem there was one core policy decision that is
haunting us when trying to move forward.
Two concrete examples are considered here: one is the
virConnectNumOfDomains() API which, while known to be racy and having
non-racy alternatives, can still be used by developers without
getting any kind of warning in the process; the other one is the
ability to define a domain without specifying the machine type, which
is becoming increasingly problematic now with some x86_64 features
being limited to q35 and downstreams looking to push for its
adoption, as well as being a manifestation of the more general
problem of libvirt's default being sometimes too conservative and at
odds with the existence of slimmed-down QEMU binaries being built
with reducing the total attack surface in mind.
I know a moot point now, but...
connectNumOf[Defined]Domains may be used without calling
connectList[Defined]Domains. The latter is actually the one I'd think is
more buggy as opposed to using connectListAllDomains. I think the same
argument can be made for other subsystems with both connectList and
connectListAll (e.g. most of them). Still, since connectListAllDomains
was introduced in 0.9.13 (01-Jul-2012) - I have to imagine anyone
developing code in 2018 would use that instead of the old API's.
Having a proper deprecation story in libvirt would allow us to point
users and developers towards the recommended solution in each case,
be it using a different API or querying libosinfo for information;
after a generous grace period, we could then remove the problematic
functionality altogether. This would be a more conservative version
of the process we already have in place for dropping support for
older QEMU releases, which recently has allowed us to ax massive
chunks of effectively dead code and simplify parts of libvirt quite
significantly.
We cannot deprecate old stuff, but can we declare new stuff comes with
limited lifetimes? IOW: learn from our previous mistakes and not allow
them to happen again. Maybe one of us will still be here in that
lifetime and get to remove something!
Even though we cannot deprecate, we could move stuff around such that
certain things end up on *_legacy.{c,h} files and if problems arise in
them, we could have a general "support" statement that would hopefully
prod the up the stack app to try and enter the modern ages. Perhaps a
time consuming short term with longer term benefits.
This series explores one possible approach to the problem and aims
to spark project-wide discussion around the topic.
Game on ;-)!
This series only 'solves' or 'advises of' the problem for anyone that
would install whatever libvirt version it got into for both client
(virsh specific too) and libvirtd/API. Doesn't help other combinations.
They wouldn't necessarily know the message occurred.
Some random thoughts without putting too much thought into each.
* Can we pull of the python-3 and python-2 magic trick with virsh?
Perhaps creating virsh-legacy that gets fork/exec'd if we cannot find
some new API we create circa 2018-2019 that knows how to only use newer
API's and perhaps (new) API to generate newer/better defaults. I know
that only solves the virsh problem, but one would think virsh-{python,
perl, etc} would be able to pull off similar tricks. Other libvirt API
consumers would be able to use the new API too. Some day soon they are
going to have to solve the same problem especially if they provide
defaults or have configurations using legacy or older options when
something more newer is available.
* Can we come up with a mechanism that allows domain admins to update
defaults automagically? Perhaps some new attribute that indicates such a
desire.
* How about a new domine define flag that says pick the latest and
greatest defaults. Of course that assumes someone is using the later and
greater *Flags API too!
* How about a new tools/virt* image that can be run providing
information about existing [defined only?] domains that would point out
which definitions may use old technology or at least informs a user what
may need to change based on current technology? Thus when a domain is
stopped, you run the tool, it changes (for example) the machine type and
then runs the post parse and verify code and either automatically cleans
up or allows the admin to keep updating until they fix things
themselves. Puts the onus on someone else to make the "policy decision".
* Can/should the Default Machine probing be split into a connect* type
driver function and then used by new apps that care?
* How much of the "default values" code for various machines or
interconnects be split into loadable modules based on machine type?
We're splitting cgroups into v1 and v2 right now. Could we do the same
to have machine specific code in specific modules? Does i440fx use
pci-e? IIRC q35 uses pci-e only.
John
Further work
============
* Fix the known issues listed below as well as all not-yet-known
issues that will undoubtably surface through discussion :)
* Introduce a mechanism to catch use of deprecated APIs at build
time, similar to GLib's G_DISABLE_DEPRECATED, to help application
developers proactively move off problematic APIs.
* Create a formal deprecation policy with well-defined rules and
time scales in the spirit of the existing one covering our
relationship with QEMU.
Know issues
===========
* For the more granular (and more interesting) type of deprecation
shown in patch 6/7, warnings are not being reported back to the
client as expected. I believe this is caused by the RPC code
looking for either a failure, in which case the virError is
transmitted, or a success, in which case the actual return value
is: we'll have to figure out a way for the error to travel across
the wire regardless of whether or not the API call was ultimately
successful if we want clients to actually receive warnings when
non-local drivers are involved.
Andrea Bolognani (7):
util: Add 'level' argument to virReportErrorHelper()
util: Introduce virReportWarning()
tools: Print warnings in virsh
util: Introduce VIR_ERR_DEPRECATED_FEATURE
Deprecate virConnectNumOfDomains()
Deprecate missing machine type in virDomainDefineXMLFlags()
tools: Force virsh to use deprecated features
include/libvirt/virterror.h | 1 +
src/access/viraccessdriverpolkit.c | 2 +-
src/access/viraccessmanager.c | 2 +-
src/conf/domain_conf.c | 11 ++++++++---
src/datatypes.h | 30 ++++++++++++++++++++++++++----
src/libvirt-domain.c | 6 ++++++
src/libvirt.c | 1 +
src/util/virbuffer.c | 4 ++--
src/util/virconf.c | 6 ++++--
src/util/virerror.c | 10 +++++++++-
src/util/virerror.h | 15 ++++++++++-----
src/util/virkeyfile.c | 6 ++++--
src/util/virxml.c | 2 +-
tools/virsh-domain-monitor.c | 2 ++
tools/vsh.c | 3 +++
15 files changed, 79 insertions(+), 22 deletions(-)
9:39 a.m.
On Tue, Oct 02, 2018 at 04:14:39PM +0200, Andrea Bolognani wrote:
Background
==========
We have plenty of features in libvirt, some of which were designed at
a time when the virtualization story was much more straightforward
than the multi-architecture, multi-hypervisor, multi-machine world we
currently live in and, while we have found ways to keep the APIs
chugging along, the result is sometimes somewhat confusing for users
and application developers, as well as requiring libvirt developers
themselves to spend quite a bit of collective time working around
decisions that, in hindsight, turn out to have been less than
fortunate.
Two concrete examples are considered here: one is the
virConnectNumOfDomains() API which, while known to be racy and having
non-racy alternatives, can still be used by developers without
getting any kind of warning in the process; the other one is the
ability to define a domain without specifying the machine type, which
is becoming increasingly problematic now with some x86_64 features
being limited to q35 and downstreams looking to push for its
adoption, as well as being a manifestation of the more general
problem of libvirt's default being sometimes too conservative and at
odds with the existence of slimmed-down QEMU binaries being built
with reducing the total attack surface in mind.
Having a proper deprecation story in libvirt would allow us to point
users and developers towards the recommended solution in each case,
be it using a different API or querying libosinfo for information;
after a generous grace period, we could then remove the problematic
functionality altogether. This would be a more conservative version
of the process we already have in place for dropping support for
older QEMU releases, which recently has allowed us to ax massive
chunks of effectively dead code and simplify parts of libvirt quite
significantly.
I don't think we'll ever be able to remove the functionality.
If informing the app developers is the goal, a "Deprecated Features"
section in news.xml could do the trick.
Jano
2231
days inactive
2244
days old
59 comments
10 participants
participants (10)
-
Andrea Bolognani -
Daniel P. Berrangé -
Erik Skultety -
Jiri Denemark -
John Ferlan -
Ján Tomko -
Laine Stump -
Michal Privoznik -
Pavel Hrdina -
Peter Krempa