[libvirt] [PATCH v2 0/4] Remove VIR_ERR_DEPRECATED & associated pieces
A deprecation is inherantly a warning, not an error, so adding an error with this name makes no conceptual sense. Features in drivers may come & go at any time and we've always use the VIR_ERR_NO_SUPPORT to indicate when a hypervisor driver does not support a particular API. Changed in v2: - Don't remove the driver API entry point entirely - leave it set to NULL, so we can document version number range. Daniel P. Berrangé (4): qemu: delete methods which are no longer supported docs: update QEMU driver docs to replace deprecated with deleted Revert "news: Mention VIR_ERR_DEPRECATED in improvements" Revert "error: Add VIR_ERR_DEPRECATED error code" docs/drvqemu.html.in | 2 +- docs/hvsupport.pl | 25 +++++++++++++++---------- docs/libvirt.css | 4 ++-- docs/news.xml | 10 ---------- include/libvirt/virterror.h | 1 - src/qemu/qemu_driver.c | 33 ++------------------------------- src/util/virerror.c | 4 ---- 7 files changed, 20 insertions(+), 59 deletions(-) -- 2.21.0
The public API entry points will report VIR_ERR_NO_SUPPORT to the caller when a driver does not provide an implementation of a particular method. When deleting methods, leaving the driver API entry point explicitly set to NULL with an version range comment, allows the hvsupport.html page to document when the AP was removed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/hvsupport.pl | 25 +++++++++++++++---------- docs/libvirt.css | 2 +- src/qemu/qemu_driver.c | 33 ++------------------------------- 3 files changed, 18 insertions(+), 42 deletions(-) diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index 2ea245e83a..6023aab222 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl @@ -234,17 +234,22 @@ foreach my $src (@srcs) { } } else { - if ($line =~ m!\s*\.(\w+)\s*=\s*(\w+)\s*,?\s*(?:/\*\s*(\d+\.\d+\.\d+)\s*(?:\(deprecated:\s*(\d+\.\d+\.\d+)\))?\s*\*/\s*)?$!) { + if ($line =~ m!\s*\.(\w+)\s*=\s*(\w+)\s*,?\s*(?:/\*\s*(\d+\.\d+\.\d+)\s*(?:-\s*(\d+\.\d+\.\d+))?\s*\*/\s*)?$!) { my $api = $1; my $meth = $2; my $vers = $3; - my $depre = $4; + my $deleted = $4; next if $api eq "no" || $api eq "name"; - die "Method $meth in $src is missing version" unless defined $vers || $api eq "connectURIProbe"; + if ($meth eq "NULL" && !defined $deleted) { + die "Method impl for $api is NULL, but no deleted version is provided"; + } + if ($meth ne "NULL" && defined $deleted) { + die "Method impl for $api is non-NULL, but deleted version is provided"; + } - die "Driver method for $api is NULL in $src" if $meth eq "NULL"; + die "Method $meth in $src is missing version" unless defined $vers || $api eq "connectURIProbe"; if (!exists($groups{$ingrp}->{apis}->{$api})) { next if $api =~ /\w(Open|Close|URIProbe)/; @@ -254,7 +259,7 @@ foreach my $src (@srcs) { $groups{$ingrp}->{drivers}->{$impl}->{$api} = {}; $groups{$ingrp}->{drivers}->{$impl}->{$api}->{vers} = $vers; - $groups{$ingrp}->{drivers}->{$impl}->{$api}->{depre} = $depre; + $groups{$ingrp}->{drivers}->{$impl}->{$api}->{deleted} = $deleted; if ($api eq "domainMigratePrepare" || $api eq "domainMigratePrepare2" || $api eq "domainMigratePrepare3") { @@ -351,9 +356,9 @@ print <<EOF; <p> This page documents which <a href="html/">libvirt calls</a> work on which libvirt drivers / hypervisors, and which version the API appeared -in. If a hypervisor driver deprecated the API, the version when it -was removed is also mentioned (highlighted in -<span class="deprecatedhv">dark red</span>). +in. If a hypervisor driver later dropped support for the API, the version +when it was removed is also mentioned (highlighted in +<span class="deletedhv">dark red</span>). </p> EOF @@ -411,8 +416,8 @@ EOF if ($groups{$grp}->{drivers}->{$drv}->{$field}->{vers}) { print $groups{$grp}->{drivers}->{$drv}->{$field}->{vers}; } - if ($groups{$grp}->{drivers}->{$drv}->{$field}->{depre}) { - print " - <span class=\"deprecatedhv\">", $groups{$grp}->{drivers}->{$drv}->{$field}->{depre}, "</span>"; + if ($groups{$grp}->{drivers}->{$drv}->{$field}->{deleted}) { + print " - <span class=\"deletedhv\">", $groups{$grp}->{drivers}->{$drv}->{$field}->{deleted}, "</span>"; } } print "</td>\n"; diff --git a/docs/libvirt.css b/docs/libvirt.css index 6639b1df64..8309f7a386 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -588,7 +588,7 @@ td.enumvalue { display: inline; } -.deprecatedhv { +.deletedhv { color: darkred; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d6ab134196..ef2e980216 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7316,22 +7316,6 @@ static char } -static char * -qemuConnectDomainXMLFromNative(virConnectPtr conn, - const char *format ATTRIBUTE_UNUSED, - const char *config ATTRIBUTE_UNUSED, - unsigned int flags) -{ - virCheckFlags(0, NULL); - - if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0) - return NULL; - - virReportError(VIR_ERR_DEPRECATED, "%s", - _("converting arbitrary QEMU command lines to libvirt domain XML is no longer supported")); - return NULL; -} - static char *qemuConnectDomainXMLToNative(virConnectPtr conn, const char *format, const char *xmlData, @@ -16772,19 +16756,6 @@ static int qemuDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, } -static virDomainPtr -qemuDomainQemuAttach(virConnectPtr conn ATTRIBUTE_UNUSED, - unsigned int pid_value ATTRIBUTE_UNUSED, - unsigned int flags) -{ - virCheckFlags(0, NULL); - - virReportError(VIR_ERR_DEPRECATED, "%s", - _("attaching to a QEMU process started outside of libvirt is no longer supported")); - return NULL; -} - - static int qemuDomainOpenConsole(virDomainPtr dom, const char *dev_name, @@ -22271,7 +22242,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */ - .connectDomainXMLFromNative = qemuConnectDomainXMLFromNative, /* 0.6.4 (deprecated: 5.5.0) */ + .connectDomainXMLFromNative = NULL, /* 0.6.4 - 5.5.0 */ .connectDomainXMLToNative = qemuConnectDomainXMLToNative, /* 0.6.4 */ .connectListDefinedDomains = qemuConnectListDefinedDomains, /* 0.2.0 */ .connectNumOfDefinedDomains = qemuConnectNumOfDefinedDomains, /* 0.2.0 */ @@ -22356,7 +22327,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */ .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */ .domainQemuMonitorCommand = qemuDomainQemuMonitorCommand, /* 0.8.3 */ - .domainQemuAttach = qemuDomainQemuAttach, /* 0.9.4 (deprecated: 5.5.0) */ + .domainQemuAttach = NULL, /* 0.9.4 - 5.5.0 */ .domainQemuAgentCommand = qemuDomainQemuAgentCommand, /* 0.10.0 */ .connectDomainQemuMonitorEventRegister = qemuConnectDomainQemuMonitorEventRegister, /* 1.2.3 */ .connectDomainQemuMonitorEventDeregister = qemuConnectDomainQemuMonitorEventDeregister, /* 1.2.3 */ -- 2.21.0
On Thu, Jun 27, 2019 at 10:06:02 +0100, Daniel Berrange wrote:
The public API entry points will report VIR_ERR_NO_SUPPORT to the caller when a driver does not provide an implementation of a particular method.
When deleting methods, leaving the driver API entry point explicitly set to NULL with an version range comment, allows the hvsupport.html page to document when the AP was removed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/hvsupport.pl | 25 +++++++++++++++---------- docs/libvirt.css | 2 +- src/qemu/qemu_driver.c | 33 ++------------------------------- 3 files changed, 18 insertions(+), 42 deletions(-)
diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index 2ea245e83a..6023aab222 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl
[...]
@@ -351,9 +356,9 @@ print <<EOF; <p> This page documents which <a href="html/">libvirt calls</a> work on which libvirt drivers / hypervisors, and which version the API appeared -in. If a hypervisor driver deprecated the API, the version when it -was removed is also mentioned (highlighted in -<span class="deprecatedhv">dark red</span>). +in. If a hypervisor driver later dropped support for the API, the version +when it was removed is also mentioned (highlighted in +<span class="deletedhv">dark red</span>). </p>
EOF
[...]
diff --git a/docs/libvirt.css b/docs/libvirt.css index 6639b1df64..8309f7a386 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -588,7 +588,7 @@ td.enumvalue { display: inline; }
-.deprecatedhv { +.deletedhv {
I'd prefer "removed" as used in the text paragraph above.
color: darkred; }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d6ab134196..ef2e980216 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
[...]
@@ -22271,7 +22242,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */ - .connectDomainXMLFromNative = qemuConnectDomainXMLFromNative, /* 0.6.4 (deprecated: 5.5.0) */ + .connectDomainXMLFromNative = NULL, /* 0.6.4 - 5.5.0 */
/home/pipo/libvirt/src/qemu/qemu_driver.c:245020 Bad prefix 'NULL' for API 'connectDomainXMLFromNative', expecting 'qemu' /home/pipo/libvirt/src/qemu/qemu_driver.c:245020 Bad impl name 'NULL' for API 'connectDomainXMLFromNative', expecting 'qemuConnectDomainXMLFromNative' /home/pipo/libvirt/src/qemu/qemu_driver.c:245105 Bad prefix 'NULL' for API 'domainQemuAttach', expecting 'qemu' /home/pipo/libvirt/src/qemu/qemu_driver.c:245105 Bad impl name 'NULL' for API 'domainQemuAttach', expecting 'qemuDomainQemuAttach' make[3]: *** [Makefile:15016: check-driverimpls] Error 1
.connectDomainXMLToNative = qemuConnectDomainXMLToNative, /* 0.6.4 */ .connectListDefinedDomains = qemuConnectListDefinedDomains, /* 0.2.0 */ .connectNumOfDefinedDomains = qemuConnectNumOfDefinedDomains, /* 0.2.0 */
ACK if you make sure that make check/sytax-check passes.
On Thu, Jun 27, 2019 at 12:45:05PM +0200, Peter Krempa wrote:
On Thu, Jun 27, 2019 at 10:06:02 +0100, Daniel Berrange wrote:
The public API entry points will report VIR_ERR_NO_SUPPORT to the caller when a driver does not provide an implementation of a particular method.
When deleting methods, leaving the driver API entry point explicitly set to NULL with an version range comment, allows the hvsupport.html page to document when the AP was removed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/hvsupport.pl | 25 +++++++++++++++---------- docs/libvirt.css | 2 +- src/qemu/qemu_driver.c | 33 ++------------------------------- 3 files changed, 18 insertions(+), 42 deletions(-)
diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index 2ea245e83a..6023aab222 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl
[...]
@@ -351,9 +356,9 @@ print <<EOF; <p> This page documents which <a href="html/">libvirt calls</a> work on which libvirt drivers / hypervisors, and which version the API appeared -in. If a hypervisor driver deprecated the API, the version when it -was removed is also mentioned (highlighted in -<span class="deprecatedhv">dark red</span>). +in. If a hypervisor driver later dropped support for the API, the version +when it was removed is also mentioned (highlighted in +<span class="deletedhv">dark red</span>). </p>
EOF
[...]
diff --git a/docs/libvirt.css b/docs/libvirt.css index 6639b1df64..8309f7a386 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -588,7 +588,7 @@ td.enumvalue { display: inline; }
-.deprecatedhv { +.deletedhv {
I'd prefer "removed" as used in the text paragraph above.
Sure, fine with me.
- .connectDomainXMLFromNative = qemuConnectDomainXMLFromNative, /* 0.6.4 (deprecated: 5.5.0) */ + .connectDomainXMLFromNative = NULL, /* 0.6.4 - 5.5.0 */
/home/pipo/libvirt/src/qemu/qemu_driver.c:245020 Bad prefix 'NULL' for API 'connectDomainXMLFromNative', expecting 'qemu' /home/pipo/libvirt/src/qemu/qemu_driver.c:245020 Bad impl name 'NULL' for API 'connectDomainXMLFromNative', expecting 'qemuConnectDomainXMLFromNative' /home/pipo/libvirt/src/qemu/qemu_driver.c:245105 Bad prefix 'NULL' for API 'domainQemuAttach', expecting 'qemu' /home/pipo/libvirt/src/qemu/qemu_driver.c:245105 Bad impl name 'NULL' for API 'domainQemuAttach', expecting 'qemuDomainQemuAttach' make[3]: *** [Makefile:15016: check-driverimpls] Error 1
Opps, will fix.
.connectDomainXMLToNative = qemuConnectDomainXMLToNative, /* 0.6.4 */ .connectListDefinedDomains = qemuConnectListDefinedDomains, /* 0.2.0 */ .connectNumOfDefinedDomains = qemuConnectNumOfDefinedDomains, /* 0.2.0 */
ACK if you make sure that make check/sytax-check passes.
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 :|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/drvqemu.html.in | 2 +- docs/libvirt.css | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index aa61ee5ced..6c922ab019 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -427,7 +427,7 @@ mount -t cgroup none /dev/cgroup -o devices <h3><a id="xmlimport">Converting from QEMU args to domain XML</a></h3> <p> - <b>Note:</b> this operation is <span class="deprecated"> deprecated as of + <b>Note:</b> this operation is <span class="deleted"> deleted as of 5.5.0</span> and will return an error. </p> <p> diff --git a/docs/libvirt.css b/docs/libvirt.css index 8309f7a386..a8c5d6e807 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -274,7 +274,7 @@ span.since { font-weight: bold; } -span.deprecated { +span.deleted { color: darkred; font-style: italic; font-weight: bold; -- 2.21.0
On Thu, Jun 27, 2019 at 10:06:03 +0100, Daniel Berrange wrote:
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/drvqemu.html.in | 2 +- docs/libvirt.css | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index aa61ee5ced..6c922ab019 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -427,7 +427,7 @@ mount -t cgroup none /dev/cgroup -o devices <h3><a id="xmlimport">Converting from QEMU args to domain XML</a></h3>
<p> - <b>Note:</b> this operation is <span class="deprecated"> deprecated as of + <b>Note:</b> this operation is <span class="deleted"> deleted as of 5.5.0</span> and will return an error. </p> <p> diff --git a/docs/libvirt.css b/docs/libvirt.css index 8309f7a386..a8c5d6e807 100644 --- a/docs/libvirt.css +++ b/docs/libvirt.css @@ -274,7 +274,7 @@ span.since { font-weight: bold; }
-span.deprecated { +span.deleted { color: darkred; font-style: italic; font-weight: bold;
As in previous patch, IMO 'removed' will be better than 'deleted'. ACK
This reverts commit 3026f6d9d986ad63c4b4a1c57589e6d05b71bd70. --- docs/news.xml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/docs/news.xml b/docs/news.xml index 216134cd77..cada389092 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -64,16 +64,6 @@ </change> </section> <section title="Improvements"> - <change> - <summary> - Add error code for deprecated operations - </summary> - <description> - The new error code VIR_ERR_DEPRECATED allows for more reliable checks - of scenarios where libvirt deprecated a certain operation or - configuration. - </description> - </change> </section> <section title="Bug fixes"> </section> -- 2.21.0
On Thu, Jun 27, 2019 at 10:06:04 +0100, Daniel Berrange wrote:
This reverts commit 3026f6d9d986ad63c4b4a1c57589e6d05b71bd70. --- docs/news.xml | 10 ---------- 1 file changed, 10 deletions(-)
ACK
This reverts commit 226094fbc483128c8888f4171c353aed738b8346. A deprecation is a warning to something that use of a feature is being discouraged. By definition it is not an error condition to continue to use a deprecated feature. A VIR_ERR_DEPRECATED constant thus makes no conceptual sense. For features which are entirely absent we already document that the VIR_ERR_NO_SUPPORT code will be used. There is no need to distinguish between a feature which never existed and a feature which previously existed and was since removed. Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/virterror.h | 1 - src/util/virerror.c | 4 ---- 2 files changed, 5 deletions(-) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 22bc3c2d27..102a2573bf 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -329,7 +329,6 @@ typedef enum { VIR_ERR_INVALID_NETWORK_PORT = 105, /* invalid network port object */ VIR_ERR_NETWORK_PORT_EXIST = 106, /* the network port already exist */ VIR_ERR_NO_NETWORK_PORT = 107, /* network port not found */ - VIR_ERR_DEPRECATED = 108, /* configuration or operation is no longer supported */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST diff --git a/src/util/virerror.c b/src/util/virerror.c index 26f14ddd29..dfba8c5712 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1235,10 +1235,6 @@ const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = { [VIR_ERR_NO_NETWORK_PORT] = { N_("network port not found"), N_("network port not found: %s") }, - [VIR_ERR_DEPRECATED] = { - N_("operation or configuration no longer supported"), - "%s", - }, }; -- 2.21.0
On Thu, Jun 27, 2019 at 10:06:05 +0100, Daniel Berrange wrote:
This reverts commit 226094fbc483128c8888f4171c353aed738b8346.
A deprecation is a warning to something that use of a feature is being discouraged. By definition it is not an error condition to continue to use a deprecated feature.
A VIR_ERR_DEPRECATED constant thus makes no conceptual sense. For features which are entirely absent we already document that the VIR_ERR_NO_SUPPORT code will be used. There is no need to distinguish between a feature which never existed and a feature which previously existed and was since removed.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> ---
ACK
participants (2)
-
Daniel P. Berrangé -
Peter Krempa