[PATCH v2 03/16] migration/qapi: Rename MigrationStats to MigrationRAMStats
This stats is only about RAM, make it accurate. This paves way for statistics for all devices. Thanks to Markus, who pointed out that docs/devel/qapi-code-gen.rst has a section "Compatibility considerations" stated: Since type names are not visible in the Client JSON Protocol, types may be freely renamed. Even certain refactorings are invisible, such as splitting members from one type into a common base type. Hence this change is not ABI violation according to the document. While at it, touch up the lines to make it read better, correct the restriction on migration status being 'active' or 'completed': over time we grew too many new status that will also report "ram" section. Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: devel@lists.libvirt.org Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Juraj Marcin <jmarcin@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> --- docs/about/removed-features.rst | 2 +- qapi/migration.json | 10 +++++----- migration/migration-stats.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index e75db08410..626162022a 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -699,7 +699,7 @@ was superseded by ``sections``. ``query-migrate`` return value member ``skipped`` (removed in 9.1) '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -Member ``skipped`` of the ``MigrationStats`` struct hasn't been used +Member ``skipped`` of the ``MigrationRAMStats`` struct hasn't been used for more than 10 years. Removed with no replacement. ``migrate`` command option ``inc`` (removed in 9.1) diff --git a/qapi/migration.json b/qapi/migration.json index 7134d4ce47..e3ad3f0604 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -12,7 +12,7 @@ { 'include': 'sockets.json' } ## -# @MigrationStats: +# @MigrationRAMStats: # # Detailed migration status. # @@ -64,7 +64,7 @@ # # Since: 0.14 ## -{ 'struct': 'MigrationStats', +{ 'struct': 'MigrationRAMStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , 'duplicate': 'int', 'normal': 'int', @@ -209,8 +209,8 @@ # If this field is not returned, no migration process has been # initiated # -# @ram: `MigrationStats` containing detailed migration status, only -# returned if status is 'active' or 'completed'(since 1.2) +# @ram: Detailed migration RAM statistics, only returned if migration +# is in progress or completed (since 1.2) # # @xbzrle-cache: `XBZRLECacheStats` containing detailed XBZRLE # migration statistics, only returned if XBZRLE feature is on and @@ -309,7 +309,7 @@ # Since: 0.14 ## { 'struct': 'MigrationInfo', - 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', + 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationRAMStats', '*vfio': 'VfioStats', '*xbzrle-cache': 'XBZRLECacheStats', '*total-time': 'int', diff --git a/migration/migration-stats.h b/migration/migration-stats.h index c0f50144c9..1153520f7a 100644 --- a/migration/migration-stats.h +++ b/migration/migration-stats.h @@ -27,7 +27,7 @@ /* * These are the ram migration statistic counters. It is loosely - * based on MigrationStats. + * based on MigrationRAMStats. */ typedef struct { /* -- 2.53.0
Peter Xu <peterx@redhat.com> writes:
This stats is only about RAM, make it accurate. This paves way for statistics for all devices.
Thanks to Markus, who pointed out that docs/devel/qapi-code-gen.rst has a section "Compatibility considerations" stated:
Since type names are not visible in the Client JSON Protocol, types may be freely renamed. Even certain refactorings are invisible, such as splitting members from one type into a common base type.
Hence this change is not ABI violation according to the document.
While at it, touch up the lines to make it read better, correct the restriction on migration status being 'active' or 'completed': over time we grew too many new status that will also report "ram" section.
Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: devel@lists.libvirt.org Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Juraj Marcin <jmarcin@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> --- docs/about/removed-features.rst | 2 +- qapi/migration.json | 10 +++++----- migration/migration-stats.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index e75db08410..626162022a 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -699,7 +699,7 @@ was superseded by ``sections``. ``query-migrate`` return value member ``skipped`` (removed in 9.1) ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-Member ``skipped`` of the ``MigrationStats`` struct hasn't been used +Member ``skipped`` of the ``MigrationRAMStats`` struct hasn't been used for more than 10 years. Removed with no replacement.
``migrate`` command option ``inc`` (removed in 9.1)
docs/about/removed-features.rst and docs/about/deprecated.rst are meant for consumers of external interfaces. Since QAPI types are not relevant there, I try to avoid mentioning them. Your patch is just fine as is. "Member ``skipped`` of the return value" would also be fine. [...] Reviewed-by: Markus Armbruster <armbru@redhat.com>
When modules.alias is not available (e.g. monolithic kernel), virPCIDeviceFindBestVFIOVariant() would fail, causing the entire PCI device detach to abort. Instead, log a warning and return success with no variant found, allowing the caller to fall back to the generic vfio-pci driver. Signed-off-by: Baptiste Daroussin <baptiste.daroussin@ovhcloud.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virpci.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/util/virpci.c b/src/util/virpci.c index d43fa1ef54..fa04264a95 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1459,6 +1459,12 @@ virPCIDeviceFindBestVFIOVariant(virPCIDevice *dev, uname(&unameInfo); modulesAliasPath = g_strdup_printf("/lib/modules/%s/modules.alias", unameInfo.release); + if (!virFileExists(modulesAliasPath)) { + /* on monolithic kernel this file does not exist */ + VIR_DEBUG("modules.alias not available (%s), skipping VFIO variant detection", + modulesAliasPath); + return 0; + } if (virFileReadAll(modulesAliasPath, 8 * 1024 * 1024, &modulesAliasContent) < 0) return -1; -- 2.43.0
On monolythic kernel /sys/modules/*/drivers may not exist (ENOENT) On kernels with enhanced security (e.g. grsecurity), it might not be accessible: EACCESS or EPERM. Directly try to open if it fails with any of those errors, we fallback on the module name. Signed-off-by: Baptiste Daroussin <baptiste.daroussin@ovhcloud.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virpci.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index fa04264a95..ec22eac7c9 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1287,8 +1287,21 @@ virPCIDeviceFindDriver(virPCIDevice *dev) moduleDriversDir = g_strdup_printf("/sys/module/%s/drivers", moduleName); - if (virDirOpen(&dir, moduleDriversDir) < 0) + if (virDirOpenQuiet(&dir, moduleDriversDir) < 0) { + /* with monolithic kernels this directory will not exist: ENOENT + * with hardened system like grsecurity or any MAC frameowrk it might be be accessible: + * EACCESS or EPERM. + */ + if (errno == ENOENT || errno == EACCES || errno == EPERM) { + VIR_DEBUG("driver directory not available (%s), using module name as driver name", + moduleDriversDir); + g_free(dev->stubDriverName); + dev->stubDriverName = g_steal_pointer(&moduleName); + return 0; + } + virReportSystemError(errno, _("cannot open directory '%1$s'"), moduleDriversDir); return -1; + } while (virDirRead(dir, &ent, moduleDriversDir) > 0) { -- 2.43.0
On 6/8/26 10:10, Baptiste Daroussin wrote:
On monolythic kernel /sys/modules/*/drivers may not exist (ENOENT) On kernels with enhanced security (e.g. grsecurity), it might not be accessible: EACCESS or EPERM.
Directly try to open if it fails with any of those errors, we fallback on the module name.
Signed-off-by: Baptiste Daroussin <baptiste.daroussin@ovhcloud.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virpci.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Congratulations on your first libvirt contribution! Michal
On 6/8/26 10:10, Baptiste Daroussin wrote:
When modules.alias is not available (e.g. monolithic kernel), virPCIDeviceFindBestVFIOVariant() would fail, causing the entire PCI device detach to abort.
Instead, log a warning and return success with no variant found, allowing the caller to fall back to the generic vfio-pci driver.
Signed-off-by: Baptiste Daroussin <baptiste.daroussin@ovhcloud.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virpci.c | 6 ++++++ 1 file changed, 6 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (4)
-
Baptiste Daroussin -
Markus Armbruster -
Michal Prívozník -
Peter Xu