Re: [PATCH v3 2/7] migration/dirtyrate: set up framwork of domainGetDirtyRateInfo API
Greate thanks for your suggestions, Daniel. I'll fix that and update my patchset later. BR, Hao On 2020/10/28 3:14, Daniel Henrique Barboza wrote: > > This patch does not compile/test successfully in my env. There are > at least 2 things going wrong here: > >
[PATCH] node_device: fix leak of DIR*
Commit 53aec799fa31 introduced the function udevGetVDPACharDev(),
which scans a directory using virDirOpenIfExists() and
virDirRead(). It unfortunately forgets to close the DIR* when it is
finished with it. This patch fixes that omission.
Signed-off-by: Laine Stump
---
src/node_device/node_device_udev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index b1b8427c05..ec0bf9192b 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1162,6 +1162,7 @@ udevGetVDPACharDev(const char *sysfs_path,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("vDPA chardev path '%s' does not exist"),
chardev);
+VIR_DIR_CLOSE(dir);
return -1;
}
VIR_DEBUG("vDPA chardev is at '%s'", chardev);
@@ -1171,6 +1172,8 @@ udevGetVDPACharDev(const char *sysfs_path,
}
}
+VIR_DIR_CLOSE(dir);
+
if (direrr < 0)
return -1;
--
2.26.2
[PATCH 12/12] remove unnecessary cleanup labels and unused return variables
After converting all DIR* to g_autoptr(DIR), many cleanup: labels
ended up just having "return ret", and every place that set ret would
just immediately goto cleanup. Remove the cleanup label and its
return, and just return the set value immediately, thus eliminating
the need for the return variable itself.
Signed-off-by: Laine Stump
---
src/conf/virnetworkobj.c | 32 +++---
src/qemu/qemu_interop_config.c | 11 +++-
src/storage/storage_util.c | 50 +-
src/util/vircgroup.c | 16 ---
src/util/vircommand.c | 9 ++
src/util/virdevmapper.c| 6 ++--
src/util/virfile.c | 45 --
src/util/virnetdev.c | 10 ++-
src/util/virnuma.c | 21 ++
src/util/virpci.c | 27 ++
src/util/virresctrl.c | 21 ++
src/util/virscsi.c | 26 ++
src/util/virutil.c | 20 --
src/util/virvhba.c | 11 +++-
tests/testutilsqemu.c | 28 +++
15 files changed, 119 insertions(+), 214 deletions(-)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index acf1442f88..8b3eb0f41c 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1091,12 +1091,11 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets,
if (obj &&
virNetworkObjLoadAllPorts(obj, stateDir) < 0) {
virNetworkObjEndAPI(&obj);
-goto cleanup;
+return -1;
}
virNetworkObjEndAPI(&obj);
}
- cleanup:
return ret;
}
@@ -1708,15 +1707,12 @@ virNetworkObjDeleteAllPorts(virNetworkObjPtr net,
g_autoptr(DIR) dh = NULL;
struct dirent *de;
int rc;
-int ret = -1;
if (!(dir = virNetworkObjGetPortStatusDir(net, stateDir)))
-goto cleanup;
+return -1;
-if ((rc = virDirOpenIfExists(&dh, dir)) <= 0) {
-ret = rc;
-goto cleanup;
-}
+if ((rc = virDirOpenIfExists(&dh, dir)) <= 0)
+return rc;
while ((rc = virDirRead(dh, &de, dir)) > 0) {
char *file = NULL;
@@ -1733,10 +1729,7 @@ virNetworkObjDeleteAllPorts(virNetworkObjPtr net,
}
virHashRemoveAll(net->ports);
-
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
@@ -1862,18 +1855,15 @@ virNetworkObjLoadAllPorts(virNetworkObjPtr net,
g_autofree char *dir = NULL;
g_autoptr(DIR) dh = NULL;
struct dirent *de;
-int ret = -1;
int rc;
char uuidstr[VIR_UUID_STRING_BUFLEN];
g_autoptr(virNetworkPortDef) portdef = NULL;
if (!(dir = virNetworkObjGetPortStatusDir(net, stateDir)))
-goto cleanup;
+return -1;
-if ((rc = virDirOpenIfExists(&dh, dir)) <= 0) {
-ret = rc;
-goto cleanup;
-}
+if ((rc = virDirOpenIfExists(&dh, dir)) <= 0)
+return rc;
while ((rc = virDirRead(dh, &de, dir)) > 0) {
g_autofree char *file = NULL;
@@ -1891,12 +1881,10 @@ virNetworkObjLoadAllPorts(virNetworkObjPtr net,
virUUIDFormat(portdef->uuid, uuidstr);
if (virHashAddEntry(net->ports, uuidstr, portdef) < 0)
-goto cleanup;
+return -1;
portdef = NULL;
}
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index f2d83eae93..e1c20aff11 100644
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -40,7 +40,6 @@ qemuBuildFileList(virHashTablePtr files, const char *dir)
g_autoptr(DIR) dirp = NULL;
struct dirent *ent = NULL;
int rc;
-int ret = -1;
if ((rc = virDirOpenIfExists(&dirp, dir)) < 0)
return -1;
@@ -62,24 +61,22 @@ qemuBuildFileList(virHashTablePtr files, const char *dir)
if (stat(path, &sb) < 0) {
virReportSystemError(errno, _("Unable to access %s"), path);
-goto cleanup;
+return -1;
}
if (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode))
continue;
if (virHashUpdateEntry(files, filename, path) < 0)
-goto cleanup;
+return -1;
path = NULL;
}
if (rc < 0)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
static int
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 5a5e517b15..cf1f33f177 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3507,13 +3507,12 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
struct statvfs sb;
struct stat statbuf;
int direrr;
-int ret = -1;
g_autoptr(virStorageVolDef) vol = NULL;
VIR_AUTOCLOSE fd = -1;
g_autoptr(virStorageSource) target = NULL;
if (virDirOpen(&dir, def->target.path) < 0)
-goto cleanup;
+retur
[PATCH 11/12] util: refactor function to simplify and remove label
Once the DIR* in virPCIGetName() was made g_autoptr, the cleanup:
label just had a "return ret;", but the rest of the function was more
compilcated than it needed to be, doing funky things with the value of
ret inside multi-level conditionals and a while loop that might exit
early via a break with ret == 0 or exit early via a goto cleanup with
ret == -1.
It really didn't need to be nearly as complicated. After doing the
trivial replacements of "goto cleanup" with appropriate direct
returns, it became obvious that:
1) the outermost level of the nested conditional at the end of the
function ("if (ret < 0)") was now redundant, since ret is now
*always* < 0 by that point (otherwise the function has returned).
2) by switching the sense of the next level of the conditional (making
it "if (!physPortID)", the "else" (which is now just "return 0;"
becomes the "if", and the new "else" no longer needs to be inside
the conditional.
3) the value of firstEntryName can be moved into *netname with
g_steal_pointer()
Once that is all done, ret is no longer used and can be removed.
Signed-off-by: Laine Stump
---
src/util/virpci.c | 49 +++
1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index b44208e05a..025eb9d91c 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2384,7 +2384,6 @@ virPCIGetNetName(const char *device_link_sysfs_path,
{
g_autofree char *pcidev_sysfs_net_path = NULL;
g_autofree char *firstEntryName = NULL;
-int ret = -1;
g_autoptr(DIR) dir = NULL;
struct dirent *entry = NULL;
size_t i = 0;
@@ -2399,8 +2398,7 @@ virPCIGetNetName(const char *device_link_sysfs_path,
if (virDirOpenQuiet(&dir, pcidev_sysfs_net_path) < 0) {
/* this *isn't* an error - caller needs to check for netname == NULL */
-ret = 0;
-goto cleanup;
+return 0;
}
while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) {
@@ -2411,7 +2409,7 @@ virPCIGetNetName(const char *device_link_sysfs_path,
g_autofree char *thisPhysPortID = NULL;
if (virNetDevGetPhysPortID(entry->d_name, &thisPhysPortID) < 0)
-goto cleanup;
+return -1;
/* if this one doesn't match, keep looking */
if (STRNEQ_NULLABLE(physPortID, thisPhysPortID)) {
@@ -2431,34 +2429,27 @@ virPCIGetNetName(const char *device_link_sysfs_path,
}
*netname = g_strdup(entry->d_name);
-
-ret = 0;
-break;
+return 0;
}
-if (ret < 0) {
-if (physPortID) {
-if (firstEntryName) {
-/* we didn't match the provided phys_port_id, but this
- * is probably because phys_port_id isn't implemented
- * for this NIC driver, so just return the first
- * (probably only) netname we found.
- */
-*netname = firstEntryName;
-firstEntryName = NULL;
-ret = 0;
-} else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not find network device with "
- "phys_port_id '%s' under PCI device at %s"),
- physPortID, device_link_sysfs_path);
-}
-} else {
-ret = 0; /* no netdev at the given index is *not* an error */
-}
+if (!physPortID)
+return 0;
+
+if (firstEntryName) {
+/* we didn't match the provided phys_port_id, but this
+ * is probably because phys_port_id isn't implemented
+ * for this NIC driver, so just return the first
+ * (probably only) netname we found.
+ */
+*netname = g_steal_pointer(&firstEntryName);
+return 0;
}
- cleanup:
-return ret;
+
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find network device with "
+ "phys_port_id '%s' under PCI device at %s"),
+ physPortID, device_link_sysfs_path);
+return -1;
}
int
--
2.26.2
[PATCH 07/12] util: declare g_autoptr cleanup function to auto-close DIR*
Signed-off-by: Laine Stump --- src/util/virfile.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virfile.h b/src/util/virfile.h index 6fde4f88ca..6973fbd54a 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -271,6 +271,7 @@ int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; void virDirClose(DIR *dirp) ATTRIBUTE_NONNULL(1); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(DIR, virDirClose); #define VIR_DIR_CLOSE(dir) virDirClose(dir) int virFileMakePath(const char *path) G_GNUC_WARN_UNUSED_RESULT; -- 2.26.2
[PATCH 06/12] util: change virDirClose to take a DIR* instead of DIR**.
In order to make a usable g_autoptr(DIR), we need to have a close
function that is a NOP when the pointer is NULL, but takes a simple
DIR*. But virDirClose() (candidate to be the g_autoptr cleanup
function) currently takes a DIR**, not DIR*. It does this so that it
can clear the pointer, thus making it safe to call virDirClose on the
same DIR multiple times.
In the past the clearing of the DIR* was essential in a few places,
but those few places have now been changed, so we can modify
virDirClose() to take a DIR*, and remove the side effect of clearing
the DIR*. This will make it directly usable as the g_autoptr cleanup,
and will mean that this:
{
DIR *dirp = NULL;
blah blah ...
VIR_DIR_CLOSE(dirp)
}
is functionally identical to
{
g_autoptr(DIR) dirp = NULL;
blah blah ...
}
which will make conversion to using g_autoptr mechanical and simple to review.
(Note that virDirClose() will still check for NULL before attempting
to close, so that it can always be safely called, as long as the DIR*
was initialized to NULL (another prerequisite of becoming a g_autoptr
cleanup function)
Signed-off-by: Laine Stump
---
src/util/virfile.c | 7 +++
src/util/virfile.h | 4 ++--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 970d4bd234..442d2fab96 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2927,13 +2927,12 @@ int virDirRead(DIR *dirp, struct dirent **ent, const
char *name)
return !!*ent;
}
-void virDirClose(DIR **dirp)
+void virDirClose(DIR *dirp)
{
-if (!*dirp)
+if (!dirp)
return;
-closedir(*dirp); /* exempt from syntax-check */
-*dirp = NULL;
+closedir(dirp); /* exempt from syntax-check */
}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 09488398c5..6fde4f88ca 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -269,9 +269,9 @@ int virDirOpenQuiet(DIR **dirp, const char *dirname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
-void virDirClose(DIR **dirp)
+void virDirClose(DIR *dirp)
ATTRIBUTE_NONNULL(1);
-#define VIR_DIR_CLOSE(dir) virDirClose(&(dir))
+#define VIR_DIR_CLOSE(dir) virDirClose(dir)
int virFileMakePath(const char *path) G_GNUC_WARN_UNUSED_RESULT;
int virFileMakePathWithMode(const char *path,
--
2.26.2
[PATCH 10/12] util: remove unused VIR_DIR_CLOSE() macro
Since every single use of DIR* was converted to use g_autoptr, this function is not currently needed. Even if someone comes up with a usage for a non-g_autoptr DIR* in the future, they can just use virDirClose(), since there is no longer a semantic difference between the two (VIR_DIR_CLOSE() previously had an extra & on the pointer so that it could be transparently passed as a DIR** to virDirClose(), but that was removed several commits back.) Signed-off-by: Laine Stump --- src/util/virfile.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virfile.h b/src/util/virfile.h index 6973fbd54a..ba31a78e58 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -272,7 +272,6 @@ int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname) void virDirClose(DIR *dirp) ATTRIBUTE_NONNULL(1); G_DEFINE_AUTOPTR_CLEANUP_FUNC(DIR, virDirClose); -#define VIR_DIR_CLOSE(dir) virDirClose(dir) int virFileMakePath(const char *path) G_GNUC_WARN_UNUSED_RESULT; int virFileMakePathWithMode(const char *path, -- 2.26.2
[PATCH 05/12] util: manually set dirp to NULL after closing in virCapabilitiesInitCache()
In all uses of VIR_DIR_CLOSE() except one, the DIR* is never
referenced after closing all the way until it goes out of
scope. virCapabilitiesInitCaches(), however, reuses the same DIR* over
and over in a loop, but due to having many error conditions that
result in a goto out of the loop, it's not well suited to reducing the
scope of the variable until we introduce a g_autoptr cleanup function
for DIR*.
In preparation for doing just that, we need to get rid of the side
effect of VIR_DIR_CLOSE() setting the DIR* to NULL, so in this one
case, let's manually set the DIR* to NULL. Then in an upcoming patch
we can safely remove the side effect from VIR_DIR_CLOSE().
This extra/ugly bit of code is only temporary: once we introduce the
g_autoptr cleanup function for DIR*, we will remove this manual
close/clear completely anyway.
Signed-off-by: Laine Stump
---
src/conf/capabilities.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 90ad4e0c13..18b2612d2e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1865,6 +1865,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
path = g_strdup_printf("%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos);
VIR_DIR_CLOSE(dirp);
+dirp = NULL;
rv = virDirOpenIfExists(&dirp, path);
if (rv < 0)
--
2.26.2
[PATCH 08/12] change DIR* int g_autoptr(DIR) where appropriate
All of these conversions are trivial - VIR_DIR_CLOSE() (aka
virDirClose()) is called only once on the DIR*, and it happens just
before going out of scope.
Signed-off-by: Laine Stump
---
src/bhyve/bhyve_capabilities.c | 3 +--
src/conf/virdomainobjlist.c | 3 +--
src/conf/virnetworkobj.c | 12
src/conf/virnwfilterbindingobjlist.c | 3 +--
src/conf/virnwfilterobj.c| 3 +--
src/conf/virsecretobj.c | 3 +--
src/conf/virstorageobj.c | 6 ++
src/openvz/openvz_conf.c | 3 +--
src/qemu/qemu_driver.c | 6 ++
src/qemu/qemu_interop_config.c | 3 +--
src/security/security_selinux.c | 6 ++
src/storage/storage_backend_iscsi.c | 3 +--
src/storage/storage_util.c | 18 +-
src/util/vircgroup.c | 7 ++-
src/util/vircgroupv1.c | 4 +---
src/util/vircommand.c| 3 +--
src/util/virdevmapper.c | 3 +--
src/util/virfile.c | 12
src/util/virhook.c | 8 ++--
src/util/virhostcpu.c| 6 ++
src/util/virnetdev.c | 3 +--
src/util/virnuma.c | 3 +--
src/util/virpci.c| 15 +--
src/util/virprocess.c| 3 +--
src/util/virresctrl.c| 9 +++--
src/util/virscsi.c | 6 ++
src/util/virscsihost.c | 3 +--
src/util/virusb.c| 3 +--
src/util/virutil.c | 6 ++
src/util/virvhba.c | 9 +++--
tests/testutilsqemu.c| 6 ++
tests/virschematest.c| 3 +--
tools/virt-host-validate-common.c| 3 +--
33 files changed, 60 insertions(+), 127 deletions(-)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
index 8a9acf52b0..e9b4e5176d 100644
--- a/src/bhyve/bhyve_capabilities.c
+++ b/src/bhyve/bhyve_capabilities.c
@@ -134,7 +134,7 @@ virBhyveDomainCapsBuild(bhyveConnPtr conn,
{
virDomainCapsPtr caps = NULL;
unsigned int bhyve_caps = 0;
-DIR *dir;
+g_autoptr(DIR) dir = NULL;
struct dirent *entry;
size_t firmwares_alloc = 0;
virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(conn);
@@ -171,7 +171,6 @@ virBhyveDomainCapsBuild(bhyveConnPtr conn,
cleanup:
VIR_FREE(firmwares);
-VIR_DIR_CLOSE(dir);
virObjectUnref(cfg);
return caps;
}
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index e9a4b271df..9c10090b32 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -588,7 +588,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
virDomainLoadConfigNotify notify,
void *opaque)
{
-DIR *dir;
+g_autoptr(DIR) dir = NULL;
struct dirent *entry;
int ret = -1;
int rc;
@@ -633,7 +633,6 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
}
}
-VIR_DIR_CLOSE(dir);
virObjectRWUnlock(doms);
return ret;
}
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 46205b163c..acf1442f88 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1072,7 +1072,7 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets,
const char *stateDir,
virNetworkXMLOptionPtr xmlopt)
{
-DIR *dir;
+g_autoptr(DIR) dir = NULL;
struct dirent *entry;
int ret = -1;
int rc;
@@ -1097,7 +1097,6 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets,
}
cleanup:
-VIR_DIR_CLOSE(dir);
return ret;
}
@@ -1108,7 +1107,7 @@ virNetworkObjLoadAllConfigs(virNetworkObjListPtr nets,
const char *autostartDir,
virNetworkXMLOptionPtr xmlopt)
{
-DIR *dir;
+g_autoptr(DIR) dir = NULL;
struct dirent *entry;
int ret = -1;
int rc;
@@ -1132,7 +1131,6 @@ virNetworkObjLoadAllConfigs(virNetworkObjListPtr nets,
virNetworkObjEndAPI(&obj);
}
-VIR_DIR_CLOSE(dir);
return ret;
}
@@ -1707,7 +1705,7 @@ virNetworkObjDeleteAllPorts(virNetworkObjPtr net,
const char *stateDir)
{
g_autofree char *dir = NULL;
-DIR *dh = NULL;
+g_autoptr(DIR) dh = NULL;
struct dirent *de;
int rc;
int ret = -1;
@@ -1738,7 +1736,6 @@ virNetworkObjDeleteAllPorts(virNetworkObjPtr net,
ret = 0;
cleanup:
-VIR_DIR_CLOSE(dh);
return ret;
}
@@ -1863,7 +1860,7 @@ virNetworkObjLoadAllPorts(virNetworkObjPtr net,
const char *stateDir)
{
g_autofree char *dir = NULL;
-DIR *dh = NULL;
+g_autoptr(DIR) dh = NULL;
struct dirent *de;
int ret = -1;
int rc;
@@ -1901,6 +1898,5 @@ virNetworkObjLoadAllPorts(virNetworkOb
[PATCH 09/12] conf: convert final DIR* to g_autoptr
This use of DIR* was re-using the same function-scope DIR* each time
through a for loop, and due to multiple error gotos in the loop, it
needed to have the scope of the DIR* reduced to just the loop at the
same time as switching to g_autoptr. That's what this patch does.
Signed-off-by: Laine Stump
---
src/conf/capabilities.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 18b2612d2e..425f34113a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1837,7 +1837,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
size_t i = 0;
virBitmapPtr cpus = NULL;
ssize_t pos = -1;
-DIR *dirp = NULL;
int ret = -1;
char *path = NULL;
char *type = NULL;
@@ -1860,13 +1859,11 @@ virCapabilitiesInitCaches(virCapsPtr caps)
while ((pos = virBitmapNextSetBit(cpus, pos)) >= 0) {
int rv = -1;
+g_autoptr(DIR) dirp = NULL;
VIR_FREE(path);
path = g_strdup_printf("%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos);
-VIR_DIR_CLOSE(dirp);
-dirp = NULL;
-
rv = virDirOpenIfExists(&dirp, path);
if (rv < 0)
goto cleanup;
@@ -1971,7 +1968,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
cleanup:
VIR_FREE(type);
VIR_FREE(path);
-VIR_DIR_CLOSE(dirp);
virCapsHostCacheBankFree(bank);
virBitmapFree(cpus);
return ret;
--
2.26.2
[PATCH 03/12] storage: remove extraneous call to VIR_DIR_CLOSE()
VIR_DIR_CLOSE(dir) is called in the middle of virStorageBackendRefreshLocal(), which is okay, but redundant - there is no reference to dir between that call and the end of the function, where VIR_DIR_CLOSE() is called again. Remove the extra call in the middle to simplify the function and make the conversion to g_autoptr trivial/mechanical. Signed-off-by: Laine Stump --- src/storage/storage_util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 21ad54ac54..7eaf899883 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3553,7 +3553,6 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) } if (direrr < 0) goto cleanup; -VIR_DIR_CLOSE(dir); target = virStorageSourceNew(); -- 2.26.2
[PATCH 01/12] consistently use VIR_DIR_CLOSE() instead of virDirClose()
This will make it easier to review upcoming patches that use g_autoptr
to auto-close all DIRs.
Signed-off-by: Laine Stump
---
src/qemu/qemu_interop_config.c | 2 +-
src/security/security_selinux.c | 6 ++
src/util/virdevmapper.c | 2 +-
src/util/virfile.c | 3 +--
tests/testutilsqemu.c | 5 ++---
5 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index 53b251f056..7edca7540a 100644
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -79,7 +79,7 @@ qemuBuildFileList(virHashTablePtr files, const char *dir)
ret = 0;
cleanup:
-virDirClose(&dirp);
+VIR_DIR_CLOSE(dirp);
return ret;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 733bcf23d9..7d9e62a239 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -3487,8 +3487,7 @@ virSecuritySELinuxSetFileLabels(virSecurityManagerPtr mgr,
virReportSystemError(errno, _("Unable to label files under %s"),
path);
-virDirClose(&dir);
-
+VIR_DIR_CLOSE(dir);
return ret;
}
@@ -3532,8 +3531,7 @@ virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr
mgr,
virReportSystemError(errno, _("Unable to restore file labels under
%s"),
path);
-virDirClose(&dir);
-
+VIR_DIR_CLOSE(dir);
return ret;
}
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 4d27c9f104..f26d36832a 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -211,7 +211,7 @@ virDMSanitizepath(const char *path)
}
}
-virDirClose(&dh);
+VIR_DIR_CLOSE(dh);
return ret;
}
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 7d0a40b0fb..970d4bd234 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2983,8 +2983,7 @@ int virFileChownFiles(const char *name,
ret = 0;
cleanup:
-virDirClose(&dir);
-
+VIR_DIR_CLOSE(dir);
return ret;
}
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 4defba0b7b..278587767f 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -558,7 +558,7 @@ testQemuGetLatestCapsForArch(const char *arch,
ret = g_strdup_printf("%s/%s", TEST_QEMU_CAPS_PATH, maxname);
cleanup:
-virDirClose(&dir);
+VIR_DIR_CLOSE(dir);
return ret;
}
@@ -667,8 +667,7 @@ testQemuCapsIterate(const char *suffix,
ret = 0;
cleanup:
-virDirClose(&dir);
-
+VIR_DIR_CLOSE(dir);
return ret;
}
--
2.26.2
[PATCH 02/12] tools: reduce scope of a DIR* in virHostValidateIOMMU()
This will make the trivial nature of a conversion to g_autoptr (in a
later patch) more obvious.
Signed-off-by: Laine Stump
---
tools/virt-host-validate-common.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/virt-host-validate-common.c
b/tools/virt-host-validate-common.c
index 9779eb7b3b..61284ae4da 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -336,7 +336,6 @@ int virHostValidateIOMMU(const char *hvname,
bool isAMD = false, isIntel = false;
virArch arch = virArchFromHost();
struct dirent *dent;
-DIR *dir;
int rc;
flags = virHostValidateGetCPUFlags();
@@ -375,6 +374,8 @@ int virHostValidateIOMMU(const char *hvname,
} else if (ARCH_IS_PPC64(arch)) {
/* Empty Block */
} else if (ARCH_IS_S390(arch)) {
+DIR *dir;
+
/* On s390x, we skip the IOMMU check if there are no PCI
* devices (which is quite usual on s390x). If there are
* no PCI devices the directory is still there but is
--
2.26.2
[PATCH 04/12] util: reduce scope of a DIR * in virCgroupV1SetOwner()
DIR *dh is being re-used each time through the for loop of this
function, so it must be closed and then re-opened, which means we
can't convert it to g_autoptr. By moving the definition of dh inside
the for loop, we make it possible to trivially convert to g_autoptr
(which will happen in a subsequent patch)
NB: VIR_DIR_CLOSE() is already called at the bottom of the for loop,
so removing the VIR_DIR_CLOSE() at the end of the function is *not*
creating a leak of a DIR*!
Signed-off-by: Laine Stump
---
src/util/vircgroupv1.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 91c1c4d4b1..67b35c1b9a 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -875,12 +875,12 @@ virCgroupV1SetOwner(virCgroupPtr cgroup,
{
int ret = -1;
size_t i;
-DIR *dh = NULL;
int direrr;
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
g_autofree char *base = NULL;
struct dirent *de;
+DIR *dh = NULL;
if (!((1 << i) & controllers))
continue;
@@ -922,7 +922,6 @@ virCgroupV1SetOwner(virCgroupPtr cgroup,
ret = 0;
cleanup:
-VIR_DIR_CLOSE(dh);
return ret;
}
--
2.26.2
[PATCH 00/12] use g_autoptr for all DIR*
I don't even remember why I started looking at this. I think that again I was considering changing some function, and making the DIR* in that function autoclose was a prerequisite for a reasonably clean addition to the function. I eventually gave up on the other plan (probably because it was a bad idea), but decided that the DIR*'s really did need to autoclose. In the end, all uses of DIR* could be easily converted to use g_autoptr. Laine Stump (12): consistently use VIR_DIR_CLOSE() instead of virDirClose() tools: reduce scope of a DIR* in virHostValidateIOMMU() storage: remove extraneous call to VIR_DIR_CLOSE() util: reduce scope of a DIR * in virCgroupV1SetOwner() util: manually set dirp to NULL after closing in virCapabilitiesInitCache() util: change virDirClose to take a DIR* instead of DIR**. util: declare g_autoptr cleanup function to auto-close DIR* change DIR* int g_autoptr(DIR) where appropriate conf: convert final DIR* to g_autoptr util: remove unused VIR_DIR_CLOSE() macro util: refactor function to simplify and remove label remove unnecessary cleanup labels and unused return variables src/bhyve/bhyve_capabilities.c | 3 +- src/conf/capabilities.c | 5 +- src/conf/virdomainobjlist.c | 3 +- src/conf/virnetworkobj.c | 44 +- src/conf/virnwfilterbindingobjlist.c | 3 +- src/conf/virnwfilterobj.c| 3 +- src/conf/virsecretobj.c | 3 +- src/conf/virstorageobj.c | 6 +- src/openvz/openvz_conf.c | 3 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_interop_config.c | 14 ++--- src/security/security_selinux.c | 8 +-- src/storage/storage_backend_iscsi.c | 3 +- src/storage/storage_util.c | 69 - src/util/vircgroup.c | 23 +++ src/util/vircgroupv1.c | 5 +- src/util/vircommand.c| 12 ++-- src/util/virdevmapper.c | 9 +-- src/util/virfile.c | 65 src/util/virfile.h | 4 +- src/util/virhook.c | 8 +-- src/util/virhostcpu.c| 6 +- src/util/virnetdev.c | 13 ++-- src/util/virnuma.c | 24 +++- src/util/virpci.c| 91 +++- src/util/virprocess.c| 3 +- src/util/virresctrl.c| 30 - src/util/virscsi.c | 32 -- src/util/virscsihost.c | 3 +- src/util/virusb.c| 3 +- src/util/virutil.c | 26 +++- src/util/virvhba.c | 20 +++--- tests/testutilsqemu.c| 35 --- tests/virschematest.c| 3 +- tools/virt-host-validate-common.c| 4 +- 35 files changed, 206 insertions(+), 386 deletions(-) -- 2.26.2
[PULL 06/12] nbd: Update qapi to support exporting multiple bitmaps
Since 'block-export-add' is new to 5.2, we can still tweak the
interface; there, allowing 'bitmaps':['str'] is nicer than
'bitmap':'str'. This wires up the qapi and qemu-nbd changes to permit
passing multiple bitmaps as distinct metadata contexts that the NBD
client may request, but the actual support for more than one will
require a further patch to the server.
Note that there are no changes made to the existing deprecated
'nbd-server-add' command; this required splitting the QAPI type
BlockExportOptionsNbd, which fortunately does not affect QMP
introspection.
Signed-off-by: Eric Blake
Message-Id: <20201027050556.269064-5-ebl...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Peter Krempa
---
docs/system/deprecated.rst | 3 ++-
qapi/block-export.json | 41 +++---
blockdev-nbd.c | 6 +-
nbd/server.c | 19 --
qemu-nbd.c | 18 -
5 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 0ebce37a1919..32a0e620dbb9 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -257,7 +257,8 @@ the 'wait' field, which is only applicable to sockets in
server mode
Use the more generic commands ``block-export-add`` and ``block-export-del``
-instead.
+instead. As part of this deprecation, where ``nbd-server-add`` used a
+single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
Human Monitor Protocol (HMP) commands
-
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 480c497690b0..c4125f4d2104 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -63,10 +63,10 @@
'*max-connections': 'uint32' } }
##
-# @BlockExportOptionsNbd:
+# @BlockExportOptionsNbdBase:
#
-# An NBD block export (options shared between nbd-server-add and the NBD branch
-# of block-export-add).
+# An NBD block export (common options shared between nbd-server-add and
+# the NBD branch of block-export-add).
#
# @name: Export name. If unspecified, the @device parameter is used as the
#export name. (Since 2.12)
@@ -74,15 +74,27 @@
# @description: Free-form description of the export, up to 4096 bytes.
# (Since 5.0)
#
-# @bitmap: Also export the dirty bitmap reachable from @device, so the
-# NBD client can use NBD_OPT_SET_META_CONTEXT with
-# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
-#
# Since: 5.0
##
+{ 'struct': 'BlockExportOptionsNbdBase',
+ 'data': { '*name': 'str', '*description': 'str' } }
+
+##
+# @BlockExportOptionsNbd:
+#
+# An NBD block export (distinct options used in the NBD branch of
+# block-export-add).
+#
+# @bitmaps: Also export each of the named dirty bitmaps reachable from
+# @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with
+# the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect
+# each bitmap.
+#
+# Since: 5.2
+##
{ 'struct': 'BlockExportOptionsNbd',
- 'data': { '*name': 'str', '*description': 'str',
-'*bitmap': 'str' } }
+ 'base': 'BlockExportOptionsNbdBase',
+ 'data': { '*bitmaps': ['str'] } }
##
# @BlockExportOptionsVhostUserBlk:
@@ -106,19 +118,24 @@
##
# @NbdServerAddOptions:
#
-# An NBD block export.
+# An NBD block export, per legacy nbd-server-add command.
#
# @device: The device name or node name of the node to be exported
#
# @writable: Whether clients should be able to write to the device via the
#NBD connection (default false).
#
+# @bitmap: Also export a single dirty bitmap reachable from @device, so the
+# NBD client can use NBD_OPT_SET_META_CONTEXT with the metadata
+# context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap
+# (since 4.0).
+#
# Since: 5.0
##
{ 'struct': 'NbdServerAddOptions',
- 'base': 'BlockExportOptionsNbd',
+ 'base': 'BlockExportOptionsNbdBase',
'data': { 'device': 'str',
-'*writable': 'bool' } }
+'*writable': 'bool', '*bitmap': 'str' } }
##
# @nbd-server-add:
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index cee9134b12eb..d8443d235b73 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -209,8 +209,12 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error
**errp)
.has_writable = arg->has_writable,
.writable = arg->writable,
};
-QAPI_CLONE_MEMBERS(BlockExportOptionsNbd, &export_opts->u.nbd,
+QAPI_CLONE_MEMBERS(BlockExportOptionsNbdBase, &export_opts->u.nbd,
qapi_NbdServerAddOptions_base(arg));
+if (arg->has_bitmap) {
+export_opts->u.nbd.has_bitmaps = true;
+QAPI_LIST_PREPEND(export_opts->u.nbd.bitmaps, g_strdup(arg->bitmap));
+}
/*
* nbd-server-add doesn't complain when a r
Re: [PATCH v3 2/7] migration/dirtyrate: set up framwork of domainGetDirtyRateInfo API
On 10/27/20 9:47 AM, Hao Wang wrote:
Introduce DomainGetDirtyRateInfo API for domain's memory dirty rate
calculation and query.
Signed-off-by: Hao Wang
Signed-off-by: Zhou Yimin
Reviewed-by: Chuan Zheng
---
This patch does not compile/test successfully in my env. There are
at least 2 things going wrong here:
- the stub you added in qemu_driver.c is generating compile warnings:
../src/qemu/qemu_driver.c: In function ‘qemuDomainGetDirtyRateInfo’:
../src/qemu/qemu_driver.c:20127:41: error: unused parameter ‘dom’
[-Werror=unused-parameter]
20127 | qemuDomainGetDirtyRateInfo(virDomainPtr dom,
|~^~~
../src/qemu/qemu_driver.c:20128:54: error: unused parameter ‘info’
[-Werror=unused-parameter]
20128 |virDomainDirtyRateInfoPtr info,
|~~^~~~
../src/qemu/qemu_driver.c:20129:38: error: unused parameter ‘sec’
[-Werror=unused-parameter]
20129 |long long sec,
|~~^~~
../src/qemu/qemu_driver.c:20130:32: error: unused parameter ‘flags’
[-Werror=unused-parameter]
20130 |
This is the code:
static int
qemuDomainGetDirtyRateInfo(virDomainPtr dom,
virDomainDirtyRateInfoPtr info,
long long sec,
int flags)
{
/* TODO */
return 0;
}
I don't think you need this placeholder here. You're not using the
domainGetDirtyRateInfo
API anywhere in this patch, so just move this code to patch 03 where the
function is implemented.
After fixing it myself and compiling it again I found another issue:
--- command ---
17:56:04 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /usr/bin/python3
/home/danielhb/kvm-project/libvirt/scripts/check-remote-protocol.py
remote_protocol virt_remote_driver
/home/danielhb/kvm-project/libvirt/build/src/remote/libvirt_remote_driver.a
/usr/bin/pdwtags
/home/danielhb/kvm-project/libvirt/build/../src/remote_protocol-structs
--- stdout ---
--- /home/danielhb/kvm-project/libvirt/build/../src/remote_protocol-structs
2020-10-23 08:48:53.871557535 -0300
+++ - 2020-10-27 14:56:04.587438336 -0300
@@ -3142,6 +3142,17 @@
struct remote_domain_backup_get_xml_desc_ret {
remote_nonnull_string xml;
};
+struct remote_domain_get_dirty_rate_info_args {
+remote_nonnull_domain dom;
+int64_tsec;
+intflags;
+};
+struct remote_domain_get_dirty_rate_info_ret {
+intstatus;
+int64_tdirtyRate;
+int64_tstartTime;
+int64_tcalcTime;
+};
enum remote_procedure {
REMOTE_PROC_CONNECT_OPEN = 1,
REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3566,4 +3577,5 @@
REMOTE_PROC_DOMAIN_BACKUP_BEGIN = 421,
REMOTE_PROC_DOMAIN_BACKUP_GET_XML_DESC = 422,
REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423,
+REMOTE_PROC_DOMAIN_GET_DIRTY_RATE_INFO = 424,
};
---
152/307 libvirt:syntax-check / sc_flags_usage
FAIL 2.75s (exit status 2)
--- command ---
17:56:11 /usr/bin/make -C /home/danielhb/kvm-project/libvirt/build/build-aux
sc_flags_usage
--- stdout ---
make: Entering directory '/home/danielhb/kvm-project/libvirt/build/build-aux'
flags_usage
/home/danielhb/kvm-project/libvirt/include/libvirt/libvirt-domain.h:5126:
int flags);
/home/danielhb/kvm-project/libvirt/src/driver-hypervisor.h:1394:
int flags);
/home/danielhb/kvm-project/libvirt/src/libvirt-domain.c:12778:
int flags)
/home/danielhb/kvm-project/libvirt/src/remote/remote_protocol.x:3785:int
flags;
make: Leaving directory '/home/danielhb/kvm-project/libvirt/build/build-aux'
--- stderr ---
build-aux/syntax-check.mk: flags should be unsigned
make: *** [/home/danielhb/kvm-project/libvirt/build-aux/syntax-check.mk:342:
sc_flags_usage] Error 1
---
From the error message I believe it has to do with 'flags' being an int
instead of unsigned int. This changes goes back to patch 1, where you introduced
the virDomainDirtyRateInfo. You'll need to change the flag type to unsigned
int in both patches 1 and 2, regenerated the protocol files and see if you
get the tests passed.
And for the record, what I'm doing to compile/test the patches is
$ ninja -C build test
Thanks,
DHB
include/libvirt/libvirt-domain.h | 5
src/driver-hypervisor.h | 7 +
src/libvirt-domain.c | 46
src/libvirt_public.syms | 5
src/qemu/qemu_driver.c | 12 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 21 ++-
7 files changed, 96 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/li
Re: [PATCH] re-add cscope db files to .gitignore
On 10/27/20 5:39 PM, Laine Stump wrote: Commit f7114e61db removed these lines as being "old and obsolete". cscope may be old, but it is still very much useful and used (i.e. *not* obsolete), and having the files show up as "Untracked files:" every time git status is run is annoying. Signed-off-by: Laine Stump --- I'm not sure why I didn't notice this omission until now, but a couple weeks ago it resulted in my accidentally committing my cscope.out and cscope.files to git (fortunately I noticed it before I posted the patch!) It could be argued (as abologna did on IRC) that users should just add these to their private user-global .gitignore, but if that's the case, then the vim and emacs-related files should also be removed from libvirt's .gitignore (also suggested by abologna :-)). However if you do that, then any time you checkout libvirt on some random new machine where you haven't imported your personal home directory template will end up showing these by-product files. Also, somebody newly entering the project will potentially have the files showing up and bothering them. Anyway, I'm not necessarily advocating for re-adding the cscope files, but sending a patch seemed to be like the most useful way of discussing it. .gitignore | 6 ++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index 6d44a50061..feb2bcb017 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,12 @@ .#* *~ +# cscope related ignores +cscope.files +cscope.in.out +cscope.out +cscope.po.out + # git related ignores *.rej *.orig Since this is not a technical question but a matter of preference, I won't put my ACK rather than +1. We already have vim/emacs files ignored, tags (marked as "libvirt related ignores" - wut?), /build/ (why, if I need to provide a builddir every time? and just so you know, I'm providing _build). If some of us use cscope we should add these files. Michal
[PATCH] re-add cscope db files to .gitignore
Commit f7114e61db removed these lines as being "old and obsolete". cscope may be old, but it is still very much useful and used (i.e. *not* obsolete), and having the files show up as "Untracked files:" every time git status is run is annoying. Signed-off-by: Laine Stump --- I'm not sure why I didn't notice this omission until now, but a couple weeks ago it resulted in my accidentally committing my cscope.out and cscope.files to git (fortunately I noticed it before I posted the patch!) It could be argued (as abologna did on IRC) that users should just add these to their private user-global .gitignore, but if that's the case, then the vim and emacs-related files should also be removed from libvirt's .gitignore (also suggested by abologna :-)). However if you do that, then any time you checkout libvirt on some random new machine where you haven't imported your personal home directory template will end up showing these by-product files. Also, somebody newly entering the project will potentially have the files showing up and bothering them. Anyway, I'm not necessarily advocating for re-adding the cscope files, but sending a patch seemed to be like the most useful way of discussing it. .gitignore | 6 ++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index 6d44a50061..feb2bcb017 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,12 @@ .#* *~ +# cscope related ignores +cscope.files +cscope.in.out +cscope.out +cscope.po.out + # git related ignores *.rej *.orig -- 2.26.2
[PATCH v2] cpu_ppc64: compare CPU function is ignoring return value
Function to compare CPU on 64-bits PowerPC is ignoring the flag to avoid failure
in case of CPUs (host and guest) are incompatible. Basically, the function is
returning -1 even if it is set to continue.
Signed-off-by: Julio Faracco
---
src/cpu/cpu_ppc64.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 2fedcd25da..555eeecbe7 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -524,11 +524,11 @@ virCPUppc64Compare(virCPUDefPtr host,
if (failIncompatible) {
virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
_("unknown host CPU"));
-} else {
-VIR_WARN("unknown host CPU");
-ret = VIR_CPU_COMPARE_INCOMPATIBLE;
+return VIR_CPU_COMPARE_ERROR;
}
-return -1;
+
+VIR_WARN("unknown host CPU");
+return VIR_CPU_COMPARE_INCOMPATIBLE;
}
ret = ppc64Compute(host, cpu, NULL, &message);
--
2.25.1
Re: [PATCH] cpu_ppc64: compare CPU function is ignoring return value
Hi Jano,
Your suggestion makes more sense.
Let me send a V2 then.
Tks :-)
--
Julio Cesar Faracco
Em ter., 27 de out. de 2020 às 12:24, Ján Tomko escreveu:
>
> On a Tuesday in 2020, Julio Faracco wrote:
> >Function to compare CPU on 64-bit PowerPC is ignoring the flag to avoid
> >failure in case of CPUs (host and guest) are incompatible. Basically, the
> >function is returning -1 even if it is set to continue.
> >
> >Signed-off-by: Julio Faracco
> >---
> > src/cpu/cpu_ppc64.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
> >index 2fedcd25da..23ea5a6a3e 100644
> >--- a/src/cpu/cpu_ppc64.c
> >+++ b/src/cpu/cpu_ppc64.c
> >@@ -517,7 +517,7 @@ virCPUppc64Compare(virCPUDefPtr host,
> >virCPUDefPtr cpu,
> >bool failIncompatible)
> > {
> >-virCPUCompareResult ret;
> >+virCPUCompareResult ret = -1;
> > g_autofree char *message = NULL;
> >
> > if (!host || !host->model) {
> >@@ -528,7 +528,7 @@ virCPUppc64Compare(virCPUDefPtr host,
> > VIR_WARN("unknown host CPU");
> > ret = VIR_CPU_COMPARE_INCOMPATIBLE;
>
> This would be easier to read with each of the if/else
> branches having their own return with the constant.
>
> No need to assing to ret just to return it in the next
> statement.
>
> > }
> >-return -1;
>
> VIR_CPU_COMPARE_ERROR has the value of -1
>
> Jano
>
> >+return ret;
> > }
> >
> > ret = ppc64Compute(host, cpu, NULL, &message);
> >--
> >2.25.1
> >
Re: [PATCH] cpu_ppc64: compare CPU function is ignoring return value
On a Tuesday in 2020, Julio Faracco wrote:
Function to compare CPU on 64-bit PowerPC is ignoring the flag to avoid
failure in case of CPUs (host and guest) are incompatible. Basically, the
function is returning -1 even if it is set to continue.
Signed-off-by: Julio Faracco
---
src/cpu/cpu_ppc64.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 2fedcd25da..23ea5a6a3e 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -517,7 +517,7 @@ virCPUppc64Compare(virCPUDefPtr host,
virCPUDefPtr cpu,
bool failIncompatible)
{
-virCPUCompareResult ret;
+virCPUCompareResult ret = -1;
g_autofree char *message = NULL;
if (!host || !host->model) {
@@ -528,7 +528,7 @@ virCPUppc64Compare(virCPUDefPtr host,
VIR_WARN("unknown host CPU");
ret = VIR_CPU_COMPARE_INCOMPATIBLE;
This would be easier to read with each of the if/else
branches having their own return with the constant.
No need to assing to ret just to return it in the next
statement.
}
-return -1;
VIR_CPU_COMPARE_ERROR has the value of -1
Jano
+return ret;
}
ret = ppc64Compute(host, cpu, NULL, &message);
--
2.25.1
signature.asc
Description: PGP signature
Re: [PATCH v6 04/11] nbd: Update qapi to support exporting multiple bitmaps
27.10.2020 08:05, Eric Blake wrote: Since 'block-export-add' is new to 5.2, we can still tweak the interface; there, allowing 'bitmaps':['str'] is nicer than 'bitmap':'str'. This wires up the qapi and qemu-nbd changes to permit passing multiple bitmaps as distinct metadata contexts that the NBD client may request, but the actual support for more than one will require a further patch to the server. Note that there are no changes made to the existing deprecated 'nbd-server-add' command; this required splitting the QAPI type BlockExportOptionsNbd, which fortunately does not affect QMP introspection. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
[PATCH] cpu_ppc64: compare CPU function is ignoring return value
Function to compare CPU on 64-bit PowerPC is ignoring the flag to avoid
failure in case of CPUs (host and guest) are incompatible. Basically, the
function is returning -1 even if it is set to continue.
Signed-off-by: Julio Faracco
---
src/cpu/cpu_ppc64.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 2fedcd25da..23ea5a6a3e 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -517,7 +517,7 @@ virCPUppc64Compare(virCPUDefPtr host,
virCPUDefPtr cpu,
bool failIncompatible)
{
-virCPUCompareResult ret;
+virCPUCompareResult ret = -1;
g_autofree char *message = NULL;
if (!host || !host->model) {
@@ -528,7 +528,7 @@ virCPUppc64Compare(virCPUDefPtr host,
VIR_WARN("unknown host CPU");
ret = VIR_CPU_COMPARE_INCOMPATIBLE;
}
-return -1;
+return ret;
}
ret = ppc64Compute(host, cpu, NULL, &message);
--
2.25.1
Re: [PATCH] util: Avoid double free in virProcessSetAffinity
On Tue, Oct 27, 2020 at 13:59:17 +0100, Martin Kletzander wrote: > The cpu mask was free()'d immediately on any error and at the end of the > function, where it was expected that it would either error out and return or > goto another allocation if the code was to fail. However since commit > 9514e24984ee the error path did not return in one new case which caused > double-free in such situation. In order to make the code more straightforward > just free the mask after it's been used even before checking the return code > of > the call. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819801 > > Signed-off-by: Martin Kletzander > --- > src/util/virprocess.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Peter Krempa
Re: [PATCH] news: Mention nodedev support for CSS on S390
On Tue, 2020-10-27 at 13:52 +0100, Boris Fiuczynski wrote: > Signed-off-by: Boris Fiuczynski > --- > NEWS.rst | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Andrea Bolognani and pushed. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] news: Mention nodedev support for CSS on S390
On Tue, 27 Oct 2020 13:52:04 +0100 Boris Fiuczynski wrote: > Signed-off-by: Boris Fiuczynski > --- > NEWS.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 5bd8ed1c91..2fef3f706c 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -13,6 +13,12 @@ v6.9.0 (unreleased) > > * **New features** > > + * nodedev: Add support for channel subsystem (CSS) devices on S390 > + > +A CSS device is represented as a parent device of a CCW device. > +This support allows to create vfio-ccw mediated devices with > +``virNodeDeviceCreateXML()``. > + >* qemu: Implement memory failure event > > New event is implemented that is emitted whenever a guest encounters a Reviewed-by: Cornelia Huck
[PATCHv2 10/13] util: hash: Reimplement virHashTable using GHashTable
Glib's hash table provides basically the same functionality as our hash
table.
In most cases the only thing that remains in the virHash* wrappers is
NULL-checks of '@table' argument as glib's hash functions don't tolerate
NULL.
In case of iterators, we adapt the existing API of iterators to glibs to
prevent having rewrite all callers at this point.
Signed-off-by: Peter Krempa
---
v2:
- don't drop hashcode.c
- use our hash function instead, including random seed
src/util/virhash.c | 432 +
src/util/virhash.h | 4 +-
2 files changed, 129 insertions(+), 307 deletions(-)
diff --git a/src/util/virhash.c b/src/util/virhash.c
index a5ab48dbf5..6d03d4035f 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -21,42 +21,15 @@
#include "virerror.h"
#include "virhash.h"
-#include "viralloc.h"
#include "virlog.h"
#include "virhashcode.h"
#include "virrandom.h"
-#include "virstring.h"
#include "virobject.h"
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("util.hash");
-#define MAX_HASH_LEN 8
-
-/* #define DEBUG_GROW */
-
-/*
- * A single entry in the hash table
- */
-typedef struct _virHashEntry virHashEntry;
-typedef virHashEntry *virHashEntryPtr;
-struct _virHashEntry {
-struct _virHashEntry *next;
-char *name;
-void *payload;
-};
-
-/*
- * The entire hash table
- */
-struct _virHashTable {
-virHashEntryPtr *table;
-uint32_t seed;
-size_t size;
-size_t nbElems;
-virHashDataFree dataFree;
-};
struct _virHashAtomic {
virObjectLockable parent;
@@ -76,11 +49,28 @@ static int virHashAtomicOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virHashAtomic);
-static size_t
-virHashComputeKey(const virHashTable *table, const char *name)
+
+/**
+ * Our hash function uses a random seed to provide uncertainity from run to run
+ * to prevent pre-crafting of colliding hash keys.
+ */
+static uint32_t virHashTableSeed;
+
+static int virHashTableSeedOnceInit(void)
+{
+virHashTableSeed = virRandomBits(32);
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virHashTableSeed);
+
+
+static unsigned int
+virHashTableStringKey(const void *vkey)
{
-uint32_t value = virHashCodeGen(name, strlen(name), table->seed);
-return value % table->size;
+const char *key = vkey;
+
+return virHashCodeGen(key, strlen(key), virHashTableSeed);
}
@@ -95,18 +85,9 @@ virHashComputeKey(const virHashTable *table, const char
*name)
virHashTablePtr
virHashNew(virHashDataFree dataFree)
{
-virHashTablePtr table = NULL;
-
-table = g_new0(virHashTable, 1);
-
-table->seed = virRandomBits(32);
-table->size = 32;
-table->nbElems = 0;
-table->dataFree = dataFree;
+ignore_value(virHashTableSeedInitialize());
-table->table = g_new0(virHashEntryPtr, table->size);
-
-return table;
+return g_hash_table_new_full(virHashTableStringKey, g_str_equal, g_free,
dataFree);
}
@@ -138,66 +119,6 @@ virHashAtomicDispose(void *obj)
}
-/**
- * virHashGrow:
- * @table: the hash table
- * @size: the new size of the hash table
- *
- * resize the hash table
- *
- * Returns 0 in case of success, -1 in case of failure
- */
-static int
-virHashGrow(virHashTablePtr table, size_t size)
-{
-size_t oldsize, i;
-virHashEntryPtr *oldtable;
-
-#ifdef DEBUG_GROW
-size_t nbElem = 0;
-#endif
-
-if (table == NULL)
-return -1;
-if (size < 8)
-return -1;
-if (size > 8 * 2048)
-return -1;
-
-oldsize = table->size;
-oldtable = table->table;
-if (oldtable == NULL)
-return -1;
-
-table->table = g_new0(virHashEntryPtr, size);
-table->size = size;
-
-for (i = 0; i < oldsize; i++) {
-virHashEntryPtr iter = oldtable[i];
-while (iter) {
-virHashEntryPtr next = iter->next;
-size_t key = virHashComputeKey(table, iter->name);
-
-iter->next = table->table[key];
-table->table[key] = iter;
-
-#ifdef DEBUG_GROW
-nbElem++;
-#endif
-iter = next;
-}
-}
-
-VIR_FREE(oldtable);
-
-#ifdef DEBUG_GROW
-VIR_DEBUG("virHashGrow : from %d to %d, %ld elems", oldsize,
- size, nbElem);
-#endif
-
-return 0;
-}
-
/**
* virHashFree:
* @table: the hash table
@@ -208,76 +129,12 @@ virHashGrow(virHashTablePtr table, size_t size)
void
virHashFree(virHashTablePtr table)
{
-size_t i;
-
if (table == NULL)
return;
-for (i = 0; i < table->size; i++) {
-virHashEntryPtr iter = table->table[i];
-while (iter) {
-virHashEntryPtr next = iter->next;
-
-if (table->dataFree)
-table->dataFree(iter->payload);
-g_free(iter->name);
-VIR_FREE(iter);
-iter = next;
-}
-}
-
-VIR_FREE(table->table);
-VIR_FREE(table);
+g_hash_table_unref(table);
}
-static int
-virHashAddOrUpdateEntry(virHashTablePtr table, const char *name,
-void *user
Re: [PATCH] Revert "spec: Simplify setting features off by default"
On Tue, 2020-10-27 at 14:05 +0100, Andrea Bolognani wrote:
> I'm not convinced reverting this was the right call.
>
> The way RPM conditional macros work is that, if you have
>
> %{!?macro:value}
>
> that will expand to 'value' if 'macro' is *not* defined, and to
> nothing otherwise. So if you have for example
>
> %define with_fuse 0%{!?_without_fuse:0}
>
> that means that, if you pass
>
> --define '_without_fuse 1'
>
> to rpmbuild the line will expand to
>
> %define with_fuse 0
>
> and if you don't pass the extra option to rpmbuild it will instead
> expand to
>
> %define with_fuse 00
>
> The two are clearly equivalent, so there is no point in keeping the
> conditional macro - it merely obfuscates the logic.
>
> Of course things would be different if you had
>
> %define with_fuse 0%{!?_without_fuse:1}
>
> instead: in this case, the line would expand to
>
> %define with_fuse 0
>
> and
>
> %define with_fuse 01
>
> respectively, which means the feature would be enabled by default but
> could optionally be disabled by passing the correct argument to
> rpmbuild, as expected. We have plenty similar macros in libvirt.spec,
> and since they work just as intended 31d687a3218c didn't touch them.
>
> Note that in this case I've removed
>
> # fuse is used to provide virtualized /proc for LXC
> %if %{with_lxc}
> %define with_fuse 0%{!?_without_fuse:1}
> %endif
>
> from the spec to make sure that the value for the 'fuse' option
> passed to Meson depended solely on the value of the _without_fuse
> macro, and then checked the rpmbuild output to compare.
Also note that I'm aware you want to eventually push for adoption of
the standard bcond macros, and I fully stand behind that desire! If
this patch had been the first in a series that introduced bcond
support and was clearing the path for that, I would have zero
problems with it. As it is, however, you're simply reintroducing some
of the obfuscation we had recently managed to get rid of, without
getting anything in return.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] Revert "spec: Simplify setting features off by default"
On Tue, 2020-10-27 at 11:07 +0100, Michal Privoznik wrote:
> On 10/26/20 10:53 PM, Neal Gompa wrote:
> > As it turns out, the rather complicated structure that is
> > currently used for enabling or disabling features in the libvirt
> > build does not cleanly map well to RPM's bcond feature.
> >
> > Consequently, we need these back in order to support trivially
> > activating these features through extra macros as build inputs.
> >
> > This reverts commit 31d687a3218c9072d7050dd608e013e063ca180f.
> >
> > Signed-off-by: Neal Gompa
> > ---
> > libvirt.spec.in | 16
> > 1 file changed, 8 insertions(+), 8 deletions(-)
>
> Reviewed-by: Michal Privoznik
>
> and pushed.
I'm not convinced reverting this was the right call.
The way RPM conditional macros work is that, if you have
%{!?macro:value}
that will expand to 'value' if 'macro' is *not* defined, and to
nothing otherwise. So if you have for example
%define with_fuse 0%{!?_without_fuse:0}
that means that, if you pass
--define '_without_fuse 1'
to rpmbuild the line will expand to
%define with_fuse 0
and if you don't pass the extra option to rpmbuild it will instead
expand to
%define with_fuse 00
The two are clearly equivalent, so there is no point in keeping the
conditional macro - it merely obfuscates the logic.
Of course things would be different if you had
%define with_fuse 0%{!?_without_fuse:1}
instead: in this case, the line would expand to
%define with_fuse 0
and
%define with_fuse 01
respectively, which means the feature would be enabled by default but
could optionally be disabled by passing the correct argument to
rpmbuild, as expected. We have plenty similar macros in libvirt.spec,
and since they work just as intended 31d687a3218c didn't touch them.
Note that in this case I've removed
# fuse is used to provide virtualized /proc for LXC
%if %{with_lxc}
%define with_fuse 0%{!?_without_fuse:1}
%endif
from the spec to make sure that the value for the 'fuse' option
passed to Meson depended solely on the value of the _without_fuse
macro, and then checked the rpmbuild output to compare.
--
Andrea Bolognani / Red Hat / Virtualization
[PATCH] util: Avoid double free in virProcessSetAffinity
The cpu mask was free()'d immediately on any error and at the end of the
function, where it was expected that it would either error out and return or
goto another allocation if the code was to fail. However since commit
9514e24984ee the error path did not return in one new case which caused
double-free in such situation. In order to make the code more straightforward
just free the mask after it's been used even before checking the return code of
the call.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819801
Signed-off-by: Martin Kletzander
---
src/util/virprocess.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 9366f0630c42..37413796b3f6 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -447,6 +447,7 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool
quiet)
int numcpus = 1024;
size_t masklen;
cpu_set_t *mask;
+int rv = -1;
VIR_DEBUG("Set process affinity on %lld", (long long)pid);
@@ -472,8 +473,10 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map,
bool quiet)
CPU_SET_S(i, masklen, mask);
}
-if (sched_setaffinity(pid, masklen, mask) < 0) {
-CPU_FREE(mask);
+rv = sched_setaffinity(pid, masklen, mask);
+CPU_FREE(mask);
+
+if (rv < 0) {
if (errno == EINVAL &&
numcpus < (1024 << 8)) { /* 262144 cpus ought to be enough for
anyone */
numcpus = numcpus << 2;
@@ -488,7 +491,6 @@ int virProcessSetAffinity(pid_t pid, virBitmapPtr map, bool
quiet)
return -1;
}
}
-CPU_FREE(mask);
return 0;
}
--
2.28.0
[PATCH] news: Mention nodedev support for CSS on S390
Signed-off-by: Boris Fiuczynski --- NEWS.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 5bd8ed1c91..2fef3f706c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,12 @@ v6.9.0 (unreleased) * **New features** + * nodedev: Add support for channel subsystem (CSS) devices on S390 + +A CSS device is represented as a parent device of a CCW device. +This support allows to create vfio-ccw mediated devices with +``virNodeDeviceCreateXML()``. + * qemu: Implement memory failure event New event is implemented that is emitted whenever a guest encounters a -- 2.25.1
[PATCH v3 2/7] migration/dirtyrate: set up framwork of domainGetDirtyRateInfo API
Introduce DomainGetDirtyRateInfo API for domain's memory dirty rate
calculation and query.
Signed-off-by: Hao Wang
Signed-off-by: Zhou Yimin
Reviewed-by: Chuan Zheng
---
include/libvirt/libvirt-domain.h | 5
src/driver-hypervisor.h | 7 +
src/libvirt-domain.c | 46
src/libvirt_public.syms | 5
src/qemu/qemu_driver.c | 12 +
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 21 ++-
7 files changed, 96 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 145f517068..1c63191baa 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5120,4 +5120,9 @@ struct _virDomainDirtyRateInfo {
typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr;
+int virDomainGetDirtyRateInfo(virDomainPtr domain,
+ virDomainDirtyRateInfoPtr info,
+ long long sec,
+ int flags);
+
#endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index bce023017d..dc2aefa910 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1387,6 +1387,12 @@ typedef char *
(*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain,
unsigned int flags);
+typedef int
+(*virDrvDomainGetDirtyRateInfo)(virDomainPtr domain,
+virDomainDirtyRateInfoPtr info,
+long long sec,
+int flags);
+
typedef struct _virHypervisorDriver virHypervisorDriver;
typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1650,4 +1656,5 @@ struct _virHypervisorDriver {
virDrvDomainAgentSetResponseTimeout domainAgentSetResponseTimeout;
virDrvDomainBackupBegin domainBackupBegin;
virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
+virDrvDomainGetDirtyRateInfo domainGetDirtyRateInfo;
};
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3c5f55176a..8714c1ca93 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12758,3 +12758,49 @@ virDomainBackupGetXMLDesc(virDomainPtr domain,
virDispatchError(conn);
return NULL;
}
+
+
+/**
+ * virDomainGetDirtyRateInfo:
+ * @domain: a domain object.
+ * @info: return value of current domain's memory dirty rate info.
+ * @sec: show dirty rate within specified seconds.
+ * @flags: the flags of getdirtyrate action -- calculate and/or query.
+ *
+ * Get the current domain's memory dirty rate (in MB/s).
+ *
+ * Returns 0 in case of success, -1 otherwise.
+ */
+int
+virDomainGetDirtyRateInfo(virDomainPtr domain,
+ virDomainDirtyRateInfoPtr info,
+ long long sec,
+ int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+
+virCheckNonNullArgGoto(info, error);
+virCheckReadOnlyGoto(conn->flags, error);
+
+if (info)
+memset(info, 0, sizeof(*info));
+
+if (conn->driver->domainGetDirtyRateInfo) {
+if (conn->driver->domainGetDirtyRateInfo(domain, info, sec, flags) < 0)
+goto error;
+VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
+return 0;
+}
+
+virReportUnsupportedError();
+ error:
+virDispatchError(conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 539d2e3943..11864f48b1 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -873,4 +873,9 @@ LIBVIRT_6.0.0 {
virDomainBackupGetXMLDesc;
} LIBVIRT_5.10.0;
+LIBVIRT_6.9.0 {
+global:
+virDomainGetDirtyRateInfo;
+} LIBVIRT_6.0.0;
+
# define new API here using predicted next version number
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f1191c1210..513290c934 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20123,6 +20123,17 @@ qemuDomainAgentSetResponseTimeout(virDomainPtr dom,
}
+static int
+qemuDomainGetDirtyRateInfo(virDomainPtr dom,
+ virDomainDirtyRateInfoPtr info,
+ long long sec,
+ int flags)
+{
+/* TODO */
+return 0;
+}
+
+
static virHypervisorDriver qemuHypervisorDriver = {
.name = QEMU_DRIVER_NAME,
.connectURIProbe = qemuConnectURIProbe,
@@ -20362,6 +20373,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainAgentSetResponseTimeout = qemuDomainAgentSetResponseTimeout, /*
5.10.0 */
.domainBackupBegin = qemuDomainBackupBegin, /* 6.0.0 */
.domainBackupGetXMLDesc = qemuDomainBackupGetXMLDesc, /* 6.0.0 */
+.domainGetDirtyRateInfo = qemuDomainGetDirtyRateInfo, /* 6.9.0 */
};
diff --gi
migration/dirtyrate: Introduce APIs for getting domain memory dirty rate
V2 -> V3: reorganize patchset to fix compile warning V1 -> V2: replace QEMU_JOB_ASYNC with QEMU_JOB_QUERY Sometimes domain's memory dirty rate is expected by user in order to decide whether it's proper to be migrated out or not. We have already completed the QEMU part of the capability: https://patchew.org/QEMU/1600237327-33618-1-git-send-email-zhengch...@huawei.com/ And this serial of patches introduce the corresponding LIBVIRT part -- DomainGetDirtyRateInfo API and corresponding virsh api -- "getdirtyrate". instructions: bash# virsh getdirtyrate --help NAME getdirtyrate - Get a vm's memory dirty rate SYNOPSIS getdirtyrate [--seconds ] [--calculate] [--query] DESCRIPTION Get memory dirty rate of a domain in order to decide whether it's proper to be migrated out or not. OPTIONS [--domain] domain name, id or uuid --seconds calculate memory dirty rate within specified seconds, a valid range of values is [1, 60], and would default to 1s. --calculate calculate dirty rate only, can be used together with --query, either or both is expected, otherwise would default to both. --query query dirty rate only, can be used together with --calculate, either or both is expected, otherwise would default to both. example: bash# virsh getdirtyrate --calculate --query --domain vm0 --seconds 1 status:measured startTime: 820148 calcTime: 1 s dirtyRate: 6 MB/s Hao Wang (7): migration/dirtyrate: Introduce virDomainDirtyRateInfo structure migration/dirtyrate: set up framwork of domainGetDirtyRateInfo API migration/dirtyrate: Implement qemuDomainGetDirtyRateInfo migration/dirtyrate: Implement qemuDomainCalculateDirtyRate migration/dirtyrate: Implement qemuDomainCalculateDirtyRate migration/dirtyrate: Implement qemuMonitorJSONExtractDirtyRateInfo migration/dirtyrate: Introduce getdirtyrate virsh api include/libvirt/libvirt-domain.h | 56 src/driver-hypervisor.h | 7 ++ src/libvirt-domain.c | 46 + src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 67 ++ src/qemu/qemu_migration.c| 59 src/qemu/qemu_migration.h| 10 +++ src/qemu/qemu_monitor.c | 24 +++ src/qemu/qemu_monitor.h | 8 +++ src/qemu/qemu_monitor_json.c | 97 ++ src/qemu/qemu_monitor_json.h | 8 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 21 +- tools/virsh-domain.c | 112 +++ 14 files changed, 520 insertions(+), 1 deletion(-) -- 2.23.0
[PATCH v3 3/7] migration/dirtyrate: Implement qemuDomainGetDirtyRateInfo
Implement qemuDomainGetDirtyRateInfo:
using flags to control behaviors -- calculate and/or query dirtyrate.
Signed-off-by: Hao Wang
Reviewed-by: Chuan Zheng
---
include/libvirt/libvirt-domain.h | 11 +++
src/qemu/qemu_driver.c | 51 ++--
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 1c63191baa..5801ca0f86 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5096,6 +5096,17 @@ int virDomainBackupBegin(virDomainPtr domain,
char *virDomainBackupGetXMLDesc(virDomainPtr domain,
unsigned int flags);
+/**
+ * virDomainDirtyRateFlags:
+ *
+ * Details on the flags used by getdirtyrate api.
+ */
+
+typedef enum {
+VIR_DOMAIN_DIRTYRATE_CALC = 1 << 0, /* calculate domain's dirtyrate */
+VIR_DOMAIN_DIRTYRATE_QUERY = 1 << 1, /* query domain's dirtyrate */
+} virDomainDirtyRateFlags;
+
/**
* virDomainDirtyRateInfo:
*
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 513290c934..a93f99f28a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20123,14 +20123,61 @@ qemuDomainAgentSetResponseTimeout(virDomainPtr dom,
}
+#define MIN_DIRTYRATE_CALCULATION_PERIOD1/* 1s */
+#define MAX_DIRTYRATE_CALCULATION_PERIOD60 /* 60s */
+
static int
qemuDomainGetDirtyRateInfo(virDomainPtr dom,
virDomainDirtyRateInfoPtr info,
long long sec,
int flags)
{
-/* TODO */
-return 0;
+virDomainObjPtr vm = NULL;
+virQEMUDriverPtr driver = dom->conn->privateData;
+int ret = -1;
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+return ret;
+
+if (virDomainGetDirtyRateInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
+if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0))
+goto endjob;
+
+if (flags & VIR_DOMAIN_DIRTYRATE_CALC) {
+if (sec < MIN_DIRTYRATE_CALCULATION_PERIOD || sec >
MAX_DIRTYRATE_CALCULATION_PERIOD) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "seconds=%lld is invalid, please choose value
within [1, 60].", sec);
+goto endjob;
+}
+
+/* TODO: call calc-dirty-rate for dirtyrate calculating */
+}
+
+if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) {
+if (flags & VIR_DOMAIN_DIRTYRATE_CALC) {
+struct timespec ts = { .tv_sec = sec, .tv_nsec = 50 * 1000 *
1000ull };
+
+virObjectUnlock(vm);
+nanosleep(&ts, NULL);
+virObjectLock(vm);
+}
+
+/* TODO: call query-dirty-rate for dirtyrate querying */
+}
+
+ret = 0;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
}
--
2.23.0
[PATCH v3 6/7] migration/dirtyrate: Implement qemuMonitorJSONExtractDirtyRateInfo
Implement qemuMonitorJSONExtractDirtyRateInfo to deal with the return from
qmp "query-dirty-rate", and store them in virDomainDirtyRateInfo.
Signed-off-by: Hao Wang
Reviewed-by: Chuan Zheng
---
include/libvirt/libvirt-domain.h | 16 +
src/qemu/qemu_monitor_json.c | 58 ++--
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 5801ca0f86..ad5755cfed 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5107,6 +5107,22 @@ typedef enum {
VIR_DOMAIN_DIRTYRATE_QUERY = 1 << 1, /* query domain's dirtyrate */
} virDomainDirtyRateFlags;
+/**
+ * virDomainDirtyRateStatus:
+ *
+ * Details on the cause of a dirtyrate calculation status.
+ */
+
+typedef enum {
+VIR_DOMAIN_DIRTYRATE_UNSTARTED = 0, /* the dirtyrate calculation has not
been started */
+VIR_DOMAIN_DIRTYRATE_MEASURING = 1, /* the dirtyrate calculation is
measuring */
+VIR_DOMAIN_DIRTYRATE_MEASURED = 2, /* the dirtyrate calculation is
completed */
+
+# ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_DIRTYRATE_LAST
+# endif
+} virDomainDirtyRateStatus;
+
/**
* virDomainDirtyRateInfo:
*
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a0616eae2c..ca7d8d23c0 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9659,12 +9659,61 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
}
+VIR_ENUM_DECL(qemuDomainDirtyRateStatus);
+VIR_ENUM_IMPL(qemuDomainDirtyRateStatus,
+ VIR_DOMAIN_DIRTYRATE_LAST,
+ "unstarted",
+ "measuring",
+ "measured");
+
+static int
+qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data,
+virDomainDirtyRateInfoPtr info)
+{
+const char *status;
+int statusID;
+
+if (!(status = virJSONValueObjectGetString(data, "status"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'status' data"));
+return -1;
+}
+
+if ((statusID = qemuDomainDirtyRateStatusTypeFromString(status)) < 0) {
+return -1;
+}
+info->status = statusID;
+
+if ((info->status == VIR_DOMAIN_DIRTYRATE_MEASURED) &&
+(virJSONValueObjectGetNumberLong(data, "dirty-rate",
&(info->dirtyRate)) < 0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'dirty-rate'
data"));
+return -1;
+}
+
+if (virJSONValueObjectGetNumberLong(data, "start-time",
&(info->startTime)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'start-time'
data"));
+return -1;
+}
+
+if (virJSONValueObjectGetNumberLong(data, "calc-time", &(info->calcTime))
< 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'calc-time'
data"));
+return -1;
+}
+
+return 0;
+}
+
+
int
qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
virDomainDirtyRateInfoPtr info)
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+virJSONValuePtr data = NULL;
if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
return -1;
@@ -9675,6 +9724,11 @@ qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
return -1;
-/* TODO: extract dirtyrate data from reply and store in
virDomainDirtyRateInfoPtr */
-return 0;
+if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-dirty-rate reply was missing 'return' data"));
+return -1;
+}
+
+return qemuMonitorJSONExtractDirtyRateInfo(data, info);
}
--
2.23.0
[PATCH v3 4/7] migration/dirtyrate: Implement qemuDomainCalculateDirtyRate
Implement qemuDomainCalculateDirtyRate which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".
Signed-off-by: Hao Wang
Signed-off-by: Zhou Yimin
Reviewed-by: Chuan Zheng
---
src/qemu/qemu_driver.c | 6 +-
src/qemu/qemu_migration.c| 28
src/qemu/qemu_migration.h| 5 +
src/qemu/qemu_monitor.c | 12
src/qemu/qemu_monitor.h | 4
src/qemu/qemu_monitor_json.c | 22 ++
src/qemu/qemu_monitor_json.h | 4
7 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a93f99f28a..c84bd17b73 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20155,7 +20155,11 @@ qemuDomainGetDirtyRateInfo(virDomainPtr dom,
goto endjob;
}
-/* TODO: call calc-dirty-rate for dirtyrate calculating */
+if (qemuDomainCalculateDirtyRate(dom, vm, sec) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("can't calculate domain's dirty rate"));
+goto endjob;
+}
}
if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f764b0c73..8029e24415 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5836,3 +5836,31 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
virHashFree(blockinfo);
return 0;
}
+
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ long long sec)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+return ret;
+}
+
+priv = vm->privateData;
+
+VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorCalculateDirtyRate(priv->mon, sec);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index fd9eb7cab0..0522b375c0 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -258,3 +258,8 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
qemuDomainJobInfoPtr jobInfo);
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ long long sec);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5481bd99a0..06603b8691 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4769,3 +4769,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
return qemuMonitorJSONTransactionBackup(actions, device, jobname, target,
bitmap, syncmode);
}
+
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+ long long sec)
+{
+VIR_DEBUG("seconds=%lld", sec);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONCalculateDirtyRate(mon, sec);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 54fbb41ef7..afb97780cf 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
const char *target,
const char *bitmap,
qemuMonitorTransactionBackupSyncMode syncmode);
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+ long long sec);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 843a555952..65691522fb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9635,3 +9635,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
migratable);
}
+
+
+int
+qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
+ long long sec)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
+ "I:calc-time", (long)sec,
+ NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 38e23ef3c5..
[PATCH v3 5/7] migration/dirtyrate: Implement qemuDomainCalculateDirtyRate
Implement qemuDomainCalculateDirtyRate which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".
Signed-off-by: Hao Wang
Signed-off-by: Zhou Yimin
Reviewed-by: Chuan Zheng
---
src/qemu/qemu_driver.c | 6 +-
src/qemu/qemu_migration.c| 31 +++
src/qemu/qemu_migration.h| 5 +
src/qemu/qemu_monitor.c | 12
src/qemu/qemu_monitor.h | 4
src/qemu/qemu_monitor_json.c | 21 +
src/qemu/qemu_monitor_json.h | 4
7 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c84bd17b73..3d2a18a21f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20171,7 +20171,11 @@ qemuDomainGetDirtyRateInfo(virDomainPtr dom,
virObjectLock(vm);
}
-/* TODO: call query-dirty-rate for dirtyrate querying */
+if (qemuDomainQueryDirtyRate(dom, vm, info) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("can't query domain's dirty rate"));
+goto endjob;
+}
}
ret = 0;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8029e24415..3d07ba3ac4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5864,3 +5864,34 @@ qemuDomainCalculateDirtyRate(virDomainPtr dom,
return ret;
}
+
+
+int
+qemuDomainQueryDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainDirtyRateInfoPtr info)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+qemuDomainObjPrivatePtr priv;
+int ret = -1;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+return ret;
+}
+
+priv = vm->privateData;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorQueryDirtyRate(priv->mon, info);
+if (ret < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ "%s", _("get vm's dirty rate failed."));
+}
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 0522b375c0..8baae512b7 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -263,3 +263,8 @@ int
qemuDomainCalculateDirtyRate(virDomainPtr dom,
virDomainObjPtr vm,
long long sec);
+
+int
+qemuDomainQueryDirtyRate(virDomainPtr dom,
+ virDomainObjPtr vm,
+ virDomainDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 06603b8691..2fa6879467 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4781,3 +4781,15 @@ qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
return qemuMonitorJSONCalculateDirtyRate(mon, sec);
}
+
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+ virDomainDirtyRateInfoPtr info)
+{
+VIR_DEBUG("info=%p", info);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONQueryDirtyRate(mon, info);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index afb97780cf..25105c3ad9 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1531,3 +1531,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
int
qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
long long sec);
+
+int
+qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
+ virDomainDirtyRateInfoPtr info);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 65691522fb..a0616eae2c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9657,3 +9657,24 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
return 0;
}
+
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+ virDomainDirtyRateInfoPtr info)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+/* TODO: extract dirtyrate data from reply and store in
virDomainDirtyRateInfoPtr */
+return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index c9556fc19a..487f2e6e58 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -714,3 +714,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
int
qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
long long sec);
+
+int
+qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
+
[PATCH v3 1/7] migration/dirtyrate: Introduce virDomainDirtyRateInfo structure
Introduce virDomainDirtyRateInfo structure used for domain's memory dirty rate
query.
Signed-off-by: Hao Wang
Reviewed-by: Chuan Zheng
---
include/libvirt/libvirt-domain.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index b3310729bf..145f517068 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5096,4 +5096,28 @@ int virDomainBackupBegin(virDomainPtr domain,
char *virDomainBackupGetXMLDesc(virDomainPtr domain,
unsigned int flags);
+/**
+ * virDomainDirtyRateInfo:
+ *
+ * a virDomainDirtyRateInfo is a structure filled by virDomainGetDirtyRate()
and
+ * extracting dirty rate infomation for a given active Domain.
+ */
+
+typedef struct _virDomainDirtyRateInfo virDomainDirtyRateInfo;
+
+struct _virDomainDirtyRateInfo {
+int status; /* the status of dirtyrate calculation */
+long long dirtyRate;/* the dirtyrate in MB/s */
+long long startTime;/* the start time of dirtyrate calculation */
+long long calcTime; /* the period of dirtyrate calculation */
+};
+
+/**
+ * virDomainDirtyRateInfoPtr:
+ *
+ * a virDomainDirtyRateInfoPtr is a pointer to a virDomainDirtyRateInfo
structure.
+ */
+
+typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr;
+
#endif /* LIBVIRT_DOMAIN_H */
--
2.23.0
[PATCH v3 7/7] migration/dirtyrate: Introduce getdirtyrate virsh api
Signed-off-by: Hao Wang
Signed-off-by: Zhou Yimin
Reviewed-by: Chuan Zheng
---
tools/virsh-domain.c | 112 +++
1 file changed, 112 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ef347585e8..c1361ea6a9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14374,6 +14374,112 @@ cmdGuestInfo(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+/*
+ * "querydirtyrate" command
+ */
+static const vshCmdInfo info_getdirtyrate[] = {
+{.name = "help",
+ .data = N_("Get a vm's memory dirty rate")
+},
+{.name = "desc",
+ .data = N_("Get memory dirty rate of a domain in order to decide"
+" whether it's proper to be migrated out or not.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_getdirtyrate[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+{.name = "seconds",
+ .type = VSH_OT_INT,
+ .help = N_("calculate memory dirty rate within specified seconds,"
+" a valid range of values is [1, 60], and would default to
1s.")
+},
+{.name = "calculate",
+ .type = VSH_OT_BOOL,
+ .help = N_("calculate dirty rate only, can be used together with --query,"
+" either or both is expected, otherwise would default to
both.")
+},
+{.name = "query",
+ .type = VSH_OT_BOOL,
+ .help = N_("query dirty rate only, can be used together with --calculate,"
+" either or both is expected, otherwise would default to
both.")
+},
+{.name = NULL}
+};
+
+static bool
+cmdGetDirtyRateInfo(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+virDomainDirtyRateInfo info;
+long long sec = 0;
+const char *status = NULL;
+unsigned int flags = 0;
+int rc;
+bool ret = false;
+bool calc = vshCommandOptBool(cmd, "calculate");
+bool query = vshCommandOptBool(cmd, "query");
+
+if (calc)
+flags |= VIR_DOMAIN_DIRTYRATE_CALC;
+if (query)
+flags |= VIR_DOMAIN_DIRTYRATE_QUERY;
+
+/* if flag option is missing, default to both --calculate and --query */
+if (!calc && !query)
+flags |= VIR_DOMAIN_DIRTYRATE_CALC | VIR_DOMAIN_DIRTYRATE_QUERY;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+rc = vshCommandOptLongLong(ctl, cmd, "seconds", &sec);
+if (rc < 0)
+goto done;
+
+/* if --seconds option is missing, default to 1s */
+if (!rc)
+sec = 1;
+
+if (virDomainGetDirtyRateInfo(dom, &info, sec, flags) < 0) {
+vshError(ctl, "%s", _("Get memory dirty-rate failed."));
+goto done;
+}
+
+if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) {
+switch (info.status) {
+case VIR_DOMAIN_DIRTYRATE_UNSTARTED:
+status = _("unstarted");
+break;
+case VIR_DOMAIN_DIRTYRATE_MEASURING:
+status = _("measuring");
+break;
+case VIR_DOMAIN_DIRTYRATE_MEASURED:
+status = _("measured");
+break;
+default:
+status = _("unknown");
+}
+
+vshPrint(ctl, _("status:%s\n"), status);
+vshPrint(ctl, _("start time:%lld\n"), info.startTime);
+vshPrint(ctl, _("calc time: %lld s\n"), info.calcTime);
+
+if (info.status == VIR_DOMAIN_DIRTYRATE_MEASURED)
+vshPrint(ctl, _("dirty rate:%lld MB/s\n"), info.dirtyRate);
+else
+vshPrint(ctl, _("dirty rate:the calculation is %s, please
query results later\n"),
+ status);
+} else {
+vshPrint(ctl, _("Memory dirty rate is calculating, use --query option
to display results.\n"));
+}
+
+ret = true;
+ done:
+virDomainFree(dom);
+return ret;
+}
+
const vshCmdDef domManagementCmds[] = {
{.name = "attach-device",
.handler = cmdAttachDevice,
@@ -15001,5 +15107,11 @@ const vshCmdDef domManagementCmds[] = {
.info = info_guestinfo,
.flags = 0
},
+{.name = "getdirtyrate",
+ .handler = cmdGetDirtyRateInfo,
+ .opts = opts_getdirtyrate,
+ .info = info_getdirtyrate,
+ .flags = 0
+},
{.name = NULL}
};
--
2.23.0
Re: [PATCH v6 04/11] nbd: Update qapi to support exporting multiple bitmaps
On Tue, Oct 27, 2020 at 00:05:49 -0500, Eric Blake wrote: > Since 'block-export-add' is new to 5.2, we can still tweak the > interface; there, allowing 'bitmaps':['str'] is nicer than > 'bitmap':'str'. This wires up the qapi and qemu-nbd changes to permit > passing multiple bitmaps as distinct metadata contexts that the NBD > client may request, but the actual support for more than one will > require a further patch to the server. > > Note that there are no changes made to the existing deprecated > 'nbd-server-add' command; this required splitting the QAPI type > BlockExportOptionsNbd, which fortunately does not affect QMP > introspection. > > Signed-off-by: Eric Blake > --- > docs/system/deprecated.rst | 3 ++- > qapi/block-export.json | 41 +++--- > blockdev-nbd.c | 6 +- > nbd/server.c | 19 -- > qemu-nbd.c | 18 - > 5 files changed, 58 insertions(+), 29 deletions(-) Reviewed-by: Peter Krempa
Re: [PATCH 10/13] util: hash: Reimplement virHashTable using GHashTable
On Tue, Oct 27, 2020 at 01:09:38PM +0100, Peter Krempa wrote: > On Tue, Oct 27, 2020 at 10:04:33 +, Daniel Berrange wrote: > > On Tue, Oct 27, 2020 at 10:53:12AM +0100, Peter Krempa wrote: > > > On Mon, Oct 26, 2020 at 16:08:34 +, Daniel Berrange wrote: > > > > On Mon, Oct 26, 2020 at 04:45:50PM +0100, Peter Krempa wrote: > > > > > Glib's hash table provides basically the same functionality as our > > > > > hash > > > > > table. > > > > > > > > > > In most cases the only thing that remains in the virHash* wrappers is > > > > > NULL-checks of '@table' argument as glib's hash functions don't > > > > > tolerate > > > > > NULL. > > > > > > > > > > In case of iterators, we adapt the existing API of iterators to glibs > > > > > to > > > > > prevent having rewrite all callers at this point. > > > > > > > > > > Signed-off-by: Peter Krempa > > > > > --- > > > > > src/libvirt_private.syms | 4 - > > > > > src/util/meson.build | 1 - > > > > > src/util/virhash.c | 416 > > > > > ++- > > > > > src/util/virhash.h | 4 +- > > > > > src/util/virhashcode.c | 125 > > > > > src/util/virhashcode.h | 33 > > > > > > > > Our hash code impl uses Murmurhash which makes some efforts to be > > > > robust against malicious inputs triggering collisons, notably using > > > > a random seed. > > > > > > > > The new code uses g_str_hash which is much weaker, and the API > > > > docs explicitly recommend against using it if the input can be from > > > > an untrusted user. > > > > > > Yes, I've noticed that, but didn't consider it to be that much of a > > > problem as any untrusted input which is stored in a hash table (so that > > > the attacker can use crafted keys) must be in the first place > > > safeguarded against OOM condition by limiting the input count/size. > > > > The problem isn't OOM, rather it is algorithmic complexity. With malicious > > hash collisions the runtime lookup performance degrades to O(n) which can > > cause scalability concerns in some cases. > > I was pointing out that limiting the input size needed for OOM limit > conveniently limits the size of 'n'. > > The worst case for a malicious actor that I can see is the block device > statistics code, where the worst case input would be based on 2 * 10 MiB > of json, where based on 200 bytes per entry you could achieve 100k hash > comparisons. > > As noted though, I think we can use the better hash function we have. > > The only difference will be probably that the seed will be global and > not per-table since glibs table doesn't support that. If that's not > acceptable we need to keep all the code since glibs hash table's hash > function prototype is: A one-time seed is likely fine. 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 :|
Re: [PATCH 10/13] util: hash: Reimplement virHashTable using GHashTable
On Tue, Oct 27, 2020 at 10:04:33 +, Daniel Berrange wrote: > On Tue, Oct 27, 2020 at 10:53:12AM +0100, Peter Krempa wrote: > > On Mon, Oct 26, 2020 at 16:08:34 +, Daniel Berrange wrote: > > > On Mon, Oct 26, 2020 at 04:45:50PM +0100, Peter Krempa wrote: > > > > Glib's hash table provides basically the same functionality as our hash > > > > table. > > > > > > > > In most cases the only thing that remains in the virHash* wrappers is > > > > NULL-checks of '@table' argument as glib's hash functions don't tolerate > > > > NULL. > > > > > > > > In case of iterators, we adapt the existing API of iterators to glibs to > > > > prevent having rewrite all callers at this point. > > > > > > > > Signed-off-by: Peter Krempa > > > > --- > > > > src/libvirt_private.syms | 4 - > > > > src/util/meson.build | 1 - > > > > src/util/virhash.c | 416 ++- > > > > src/util/virhash.h | 4 +- > > > > src/util/virhashcode.c | 125 > > > > src/util/virhashcode.h | 33 > > > > > > Our hash code impl uses Murmurhash which makes some efforts to be > > > robust against malicious inputs triggering collisons, notably using > > > a random seed. > > > > > > The new code uses g_str_hash which is much weaker, and the API > > > docs explicitly recommend against using it if the input can be from > > > an untrusted user. > > > > Yes, I've noticed that, but didn't consider it to be that much of a > > problem as any untrusted input which is stored in a hash table (so that > > the attacker can use crafted keys) must be in the first place > > safeguarded against OOM condition by limiting the input count/size. > > The problem isn't OOM, rather it is algorithmic complexity. With malicious > hash collisions the runtime lookup performance degrades to O(n) which can > cause scalability concerns in some cases. I was pointing out that limiting the input size needed for OOM limit conveniently limits the size of 'n'. The worst case for a malicious actor that I can see is the block device statistics code, where the worst case input would be based on 2 * 10 MiB of json, where based on 200 bytes per entry you could achieve 100k hash comparisons. As noted though, I think we can use the better hash function we have. The only difference will be probably that the seed will be global and not per-table since glibs table doesn't support that. If that's not acceptable we need to keep all the code since glibs hash table's hash function prototype is: guint (*GHashFunc) (gconstpointer key);
Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik wrote: > > On 10/26/20 11:08 PM, Neal Gompa wrote: > > Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH > > based on the changelog entry date stamp. In scenarios where it already > > is defined, we do not want to redefine it. > > > > This part is okay. > > > Additionally, when building the libvirt package in an Open Build Service > > instance, the spec file is not present in %_specdir, but instead in > > %_sourcedir. > > > > But this looks fishy. Is the %_specdir defined in that case? > It is (that comes from RPM itself), however the directory is empty. -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH 0/6] Hyper-V code cleanup
On 10/22/20 6:38 PM, Matt Coleman wrote: Here's a draft GitLab MR if you'd prefer to review the changes there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs Matt Coleman (6): hyperv: reformat WQL query strings hyperv: remove duplicate function hypervGetVSSDFromUUID() hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId() hyperv: remove unneeded braces in hypervDomainGetInfo() and hypervDomainGetXMLDesc() hyperv: reduce duplicate code for Msvm_ComputerSystem lookups hyperv: do not overwrite errors from hypervInvokeMethod() src/hyperv/hyperv_driver.c | 169 + src/hyperv/hyperv_wmi.c| 57 +++-- src/hyperv/hyperv_wmi.h| 4 + 3 files changed, 75 insertions(+), 155 deletions(-) Reviewed-by: Michal Privoznik but I don't think these qualify for the freeze, so let me queue them locally and push after the release is over. Michal
Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
On 10/26/20 11:08 PM, Neal Gompa wrote: Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH based on the changelog entry date stamp. In scenarios where it already is defined, we do not want to redefine it. This part is okay. Additionally, when building the libvirt package in an Open Build Service instance, the spec file is not present in %_specdir, but instead in %_sourcedir. But this looks fishy. Is the %_specdir defined in that case? Michal
Re: [PATCH] Revert "spec: Simplify setting features off by default"
On 10/26/20 10:53 PM, Neal Gompa wrote: As it turns out, the rather complicated structure that is currently used for enabling or disabling features in the libvirt build does not cleanly map well to RPM's bcond feature. Consequently, we need these back in order to support trivially activating these features through extra macros as build inputs. This reverts commit 31d687a3218c9072d7050dd608e013e063ca180f. Signed-off-by: Neal Gompa --- libvirt.spec.in | 16 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Michal Privoznik and pushed. Michal
Re: [PATCH 10/13] util: hash: Reimplement virHashTable using GHashTable
On Tue, Oct 27, 2020 at 10:53:12AM +0100, Peter Krempa wrote: > On Mon, Oct 26, 2020 at 16:08:34 +, Daniel Berrange wrote: > > On Mon, Oct 26, 2020 at 04:45:50PM +0100, Peter Krempa wrote: > > > Glib's hash table provides basically the same functionality as our hash > > > table. > > > > > > In most cases the only thing that remains in the virHash* wrappers is > > > NULL-checks of '@table' argument as glib's hash functions don't tolerate > > > NULL. > > > > > > In case of iterators, we adapt the existing API of iterators to glibs to > > > prevent having rewrite all callers at this point. > > > > > > Signed-off-by: Peter Krempa > > > --- > > > src/libvirt_private.syms | 4 - > > > src/util/meson.build | 1 - > > > src/util/virhash.c | 416 ++- > > > src/util/virhash.h | 4 +- > > > src/util/virhashcode.c | 125 > > > src/util/virhashcode.h | 33 > > > > Our hash code impl uses Murmurhash which makes some efforts to be > > robust against malicious inputs triggering collisons, notably using > > a random seed. > > > > The new code uses g_str_hash which is much weaker, and the API > > docs explicitly recommend against using it if the input can be from > > an untrusted user. > > Yes, I've noticed that, but didn't consider it to be that much of a > problem as any untrusted input which is stored in a hash table (so that > the attacker can use crafted keys) must be in the first place > safeguarded against OOM condition by limiting the input count/size. The problem isn't OOM, rather it is algorithmic complexity. With malicious hash collisions the runtime lookup performance degrades to O(n) which can cause scalability concerns in some cases. 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 :|
Re: [PATCH 10/13] util: hash: Reimplement virHashTable using GHashTable
On Mon, Oct 26, 2020 at 16:08:34 +, Daniel Berrange wrote: > On Mon, Oct 26, 2020 at 04:45:50PM +0100, Peter Krempa wrote: > > Glib's hash table provides basically the same functionality as our hash > > table. > > > > In most cases the only thing that remains in the virHash* wrappers is > > NULL-checks of '@table' argument as glib's hash functions don't tolerate > > NULL. > > > > In case of iterators, we adapt the existing API of iterators to glibs to > > prevent having rewrite all callers at this point. > > > > Signed-off-by: Peter Krempa > > --- > > src/libvirt_private.syms | 4 - > > src/util/meson.build | 1 - > > src/util/virhash.c | 416 ++- > > src/util/virhash.h | 4 +- > > src/util/virhashcode.c | 125 > > src/util/virhashcode.h | 33 > > Our hash code impl uses Murmurhash which makes some efforts to be > robust against malicious inputs triggering collisons, notably using > a random seed. > > The new code uses g_str_hash which is much weaker, and the API > docs explicitly recommend against using it if the input can be from > an untrusted user. Yes, I've noticed that, but didn't consider it to be that much of a problem as any untrusted input which is stored in a hash table (so that the attacker can use crafted keys) must be in the first place safeguarded against OOM condition by limiting the input count/size. Apart from insertion we do have at least one place where lookup is based on untrusted data so there is a possibility for bigger scope of impact. Said that I don't really believe that the complexity vs impact ratio makes it worth even for a completely static hashing function, but when we can do better, we should. > IOW, I don't think we should be removing our virhashcode impl. If > anything we should upgrade our hash code impl to use SipHash which > is used by perl, python, ruby, rust and more. Okay, I'll leave virhashcode in. Regardless whether we have a "cool" hashing function or not, devs need to be vigilant of use of any data structure based on untrusted data though.
Re: [libvirt PATCH 0/2] rpc: Fix virt-ssh-helper detection
On Tue, Oct 27, 2020 at 12:28:43AM +0100, Andrea Bolognani wrote: > See patch 1 for details. > > Andrea Bolognani (2): > rpc: Fix virt-ssh-helper detection > news: Mention virt-ssh-helper detection fix Reviewed-by: Daniel P. Berrangé 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 :|
Re: [PATCH] util: Add phys_port_name support on virPCIGetNetName
On Mon, Sep 28, 2020 at 12:56:12PM +0300, Dmytro Linkin wrote: > On Tue, Sep 22, 2020 at 08:31:15AM -0400, Laine Stump wrote: > > On 8/28/20 6:53 AM, Dmytro Linkin wrote: > > >Current virPCIGetNetName() logic is to get net device name by checking > > >it's phys_port_id, if caller provide it, or by it's index (eg, by it's > > >position at sysfs net directory). This approach worked fine up until > > >linux kernel version 5.8, where NVIDIA Mellanox driver implemented > > >linking of VFs' representors to PCI device in switchdev mode. This mean > > >that device's sysfs net directory will hold multiple net devices. Ex.: > > > > > >$ ls '/sys/bus/pci/devices/:82:00.0/net' > > >ens1f0 eth0 eth1 > > > > > >Most switch devices support phys_port_name instead of phys_port_id, so > > >virPCIGetNetName() will try to get PF name by it's index - 0. The > > >problem here is that the PF nedev entry may not be the first. > > > > > >To fix that, for switch devices, we introduce a new logic to select the > > >PF uplink netdev according to the content of phys_port_name. Extend > > >virPCIGetNetName() with physPortNameRegex variable to get proper device > > >by it's phys_port_name scheme, for ex., "p[0-9]+$" to get PF, > > >"pf[0-9]+vf[0-9]+$" to get VF or "p1$" to get exact net device. So now > > >virPCIGetNetName() logic work in following sequence: > > > - filter by phys_port_id, if it's provided, > > > or > > > - filter by phys_port_name, if it's regex provided, > > > or > > > - get net device by it's index (position) in sysfs net directory. > > >Also, make getting content of iface sysfs files more generic. > > > > > >Signed-off-by: Dmytro Linkin > > >Reviewed-by: Adrian Chiris > > > > > > [...] > > > > > > > > > >+/* Represents format of PF's phys_port_name in switchdev mode: > > >+ * 'p%u' or 'p%us%u'. New line checked since value is readed from sysfs > > >file. > > >+ */ > > >+# define VIR_PF_PHYS_PORT_NAME_REGEX ((char > > >*)"(p[0-9]+$)|(p[0-9]+s[0-9]+$)") > > >+ > > > > > > I've come back to look at this patch several times since it was > > posted (sorry for the extreme delay in responding), but just can't > > figure out what it's doing with this regex and why the regex is > > necessary. Not having access to the hardware that it works with is a > > bit of a problem, but perhaps I could get a better idea if you gave > > a full example of sysfs contents? My concern with using a regex is > > that it might work just fine when using one method for net device > > naming, but break if that was changed. Also, it seems > > counterintuitive that it would be necessary to look for a device > > with a name matching a specific pattern; why isn't there simply a > > single symbolic link somewhere in the sysfs tree for the net device > > that just directly points at its physical port? That would be so > > much simpler and more reliable (or at least it would give the > > *perception* of being more reliable). > > > > I'll emphasize that regex is being used for phys_port_name, NOT net > device name (they are not the same). Anyway, I'll give an example. > > Lets look how virPCIGetNetName() currently work. > At first it try to read phys_port_id of every net device in net > folder, like: > $ cd '/sys/bus/pci/devices/:80:02.0/:82:00.0/' > $ cat net/ens1f0/phys_port_id > > Imagine we have single pci dual port nic (I hope named it right), > eg. net folder holds net devices for both ports, so read operation > will be successfull and function will return name of first or second > port. > For single port or dual pci nics (or for drivers which didn't > implemented phys_port_id) read fails and function fallback to > picking up device by it's index, which not really fine (I'll describe > it later) but work 'cause net folder usualy contains only one net > device. > > But kernel patch brought third case which not work. > > Here are ifaces of ConnectX-5 NIC: > $ ls -l /sys/class/net | cut -d ' ' -f 9- > ens1f0 -> ../../devices/pci:80/:80:02.0/:82:00.0/net/ens1f0 > ens1f1 -> ../../devices/pci:80/:80:02.0/:82:00.1/net/ens1f1 > ens1f2 -> ../../devices/pci:80/:80:02.0/:82:00.2/net/ens1f2 > ens1f3 -> ../../devices/pci:80/:80:02.0/:82:00.3/net/ens1f3 > ens1f0_0 -> ../../devices/pci:80/:80:02.0/:82:00.0/net/ens1f0_0 > ens1f0_1 -> ../../devices/pci:80/:80:02.0/:82:00.0/net/ens1f0_1 > > ens1f0 & ens1f1 - PFs; > ens1f2 & ens1f3 - VFs created on 1st PF and.. > ens1f0_0 & ens1f0_1 - corresponding representors. > > Here is content of PFs' pci net folders (for comparison): > $ ls '/sys/bus/pci/devices/:80:02.0/:82:00.0/net' > ens1f0 ens1f0_0 ens1f0_1 > $ ls '/sys/bus/pci/devices/:80:02.0/:82:00.1/net' > ens1f1 > > When virPCIGetNetName() called for 2nd PF, for ex., which is in legacy > mode, phys_port_id read fails (Operation not supported), function > falling back to index and return name of first and only net device. > Fine here. > When fun
