Re: [PATCH] libxl: use b_info->{acpi,acpi} when available
On 9/18/20 9:51 AM, Daniel P. Berrangé wrote:
On Fri, Sep 18, 2020 at 05:41:21PM +0200, Marek Marczykowski-Górecki wrote:
On Fri, Sep 18, 2020 at 05:12:52PM +0200, Michal Prívozník wrote:
On 9/18/20 1:31 PM, Daniel P. Berrangé wrote:
On Wed, Sep 16, 2020 at 11:09:31AM +0200, Michal Privoznik wrote:
On 9/10/20 6:18 AM, Marek Marczykowski-Górecki wrote:
b_info->u.hvm.{acpi,apic} are deprecated. But also, on recent libxl
version (4.14) the old one seems to be broken. While libxl part should
be fixed too, update the usage here and at some point drop support for
the old version.
b_info->acpi was added in Xen 4.8
b_info->apic was added in Xen 4.10
Xen 4.10 is the oldest version that still has security support (until
December 2020).
Signed-off-by: Marek Marczykowski-Górecki
---
src/libxl/libxl_conf.c | 13 +
tests/libxlxml2domconfigdata/basic-hvm.json | 4 ++--
tests/libxlxml2domconfigdata/cpu-shares-hvm.json| 4 ++--
.../libxlxml2domconfigdata/fullvirt-acpi-slic.json | 4 ++--
.../fullvirt-cpuid-legacy-nest.json | 4 ++--
tests/libxlxml2domconfigdata/fullvirt-cpuid.json| 4 ++--
.../max-eventchannels-hvm.json | 4 ++--
tests/libxlxml2domconfigdata/max-gntframes-hvm.json | 4 ++--
tests/libxlxml2domconfigdata/moredevs-hvm.json | 4 ++--
.../libxlxml2domconfigdata/variable-clock-hvm.json | 4 ++--
.../vnuma-hvm-legacy-nest.json | 4 ++--
tests/libxlxml2domconfigdata/vnuma-hvm.json | 4 ++--
12 files changed, 35 insertions(+), 22 deletions(-)
This looks good to me.
Reviewed-by: Michal Privoznik
I'll wait a bit with pushing it though in case Jim wants to chime in.
This broke the build on Ubuntu 1804 due to tests failing
TEST: libxlxml2domconfigtest
.. 10 FAIL
Oh, Ubuntu 18.04 has libxen-dev-4.9.2 and I'm not sure about FreeBSD, but
probably something old too. So we can't use xen 4.10 APIs even though it was
released 3 years ago.
Unfortunately, we will have to support Ubuntu 18.04 for quite some time
because 20.04 was released only a while ago and we have this two year
transition period:
https://libvirt.org/platforms.html
Marek, are you okay with me reverting the patch?
Technically, the driver code itself should work on both versions (there
is an #ifdef for that). It's only an issue with tests, where we don't have
#ifdef inside json files...
One idea would be to duplicate those affected json files and pick the
right one based on the libxenlight version, but it doesn't sound nice...
Any alternative ideas?
I think just duplicating the JSON files is the best solution because it
is simple, low overhead and keeps test coverage in both versions.
I'm catching up on libvirt mail and could have missed it, but I didn't see a
follow-up patch to fix the test.
Marek, do you have time for it? If not I'll get to it tomorrow.
Regards,
Jim
[PATCH] Remove redundant check when storage pool is mounted
virFileComparePaths just return 0 or 1 after commit 7b48bb8
so break while after virFileComparePaths return 1
Signed-off-by: Yi Li
---
src/storage/storage_backend_fs.c | 8 ++--
src/util/virfile.c | 1 -
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 536e5cf952..30c2367df4 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -245,7 +245,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr
pool)
FILE *mtab;
struct mntent ent;
char buf[1024];
-int rc1, rc2;
g_autofree char *src = NULL;
if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) {
@@ -262,11 +261,8 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr
pool)
/* compare both mount destinations and sources to be sure the mounted
* FS pool is really the one we're looking for
*/
-if ((rc1 = virFileComparePaths(ent.mnt_dir, def->target.path)) < 0 ||
-(rc2 = virFileComparePaths(ent.mnt_fsname, src)) < 0)
-goto cleanup;
-
-if (rc1 && rc2) {
+if (virFileComparePaths(ent.mnt_dir, def->target.path) &&
+virFileComparePaths(ent.mnt_fsname, src)) {
ret = 1;
goto cleanup;
}
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 61d2c16072..e120d277d0 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3884,7 +3884,6 @@ virFileCopyACLs(const char *src,
* Returns:
* 1 : Equal
* 0 : Non-Equal
- * -1 : Error
*/
int
virFileComparePaths(const char *p1, const char *p2)
--
2.25.3
[PATCH v2] util: support PCI passthrough net device stats collection
Collect PCI passthrough net device stats from kernel by netlink
API.
Currently, libvirt can not get PCI passthrough net device stats,
run command:
#virsh domifstat instance --interface=52:54:00:2d:b2:35
error: Failed to get interface stats instance 52:54:00:2d:b2:35
error: internal error: Interface name not provided
The PCI device(usually SR-IOV virtual function device) is detached
while it's used in PCI passthrough mode. And we can not parse this
device from /proc/net/dev any more.
In this patch, libvirt check net device is VF of not firstly, then
query virNetDevVFInterfaceStats(new API).
virNetDevVFInterfaceStats parses VFs info of all PFs, compares MAC
address until the two MAC addresses match.
'#ip -s link show' can get the same result. Instead of parsing the
output result, implement this feature by libnl API.
Notice that this feature deponds on driver of PF.
Test on Mellanox ConnectX-4 Lx, it works well.
Also test on Intel Corporation 82599ES, it works, but only get 0.
(ip-link command get the same result).
Signed-off-by: zhenwei pi
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 ++
src/util/virnetdev.c | 137 +++
src/util/virnetdev.h | 5 ++
4 files changed, 146 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bdbe3431b8..bcc40b8d69 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2585,6 +2585,7 @@ virNetDevSetRcvMulti;
virNetDevSetupControl;
virNetDevSysfsFile;
virNetDevValidateConfig;
+virNetDevVFInterfaceStats;
# util/virnetdevbandwidth.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..f554010c40 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10196,6 +10196,9 @@ qemuDomainInterfaceStats(virDomainPtr dom,
if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
if (virNetDevOpenvswitchInterfaceStats(net->ifname, stats) < 0)
goto cleanup;
+} else if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+if (virNetDevVFInterfaceStats(>mac, stats) < 0)
+goto cleanup;
} else {
if (virNetDevTapInterfaceStats(net->ifname, stats,
!virDomainNetTypeSharesHostView(net)) <
0)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index e1a4cc2bef..377f25aae7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1489,6 +1489,7 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
.maxlen = sizeof(struct ifla_vf_mac) },
[IFLA_VF_VLAN] = { .type = NLA_UNSPEC,
.maxlen = sizeof(struct ifla_vf_vlan) },
+[IFLA_VF_STATS] = { .type = NLA_NESTED },
};
@@ -2265,6 +2266,132 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
return 0;
}
+static struct nla_policy ifla_vfstats_policy[IFLA_VF_STATS_MAX+1] = {
+[IFLA_VF_STATS_RX_PACKETS] = { .type = NLA_U64 },
+[IFLA_VF_STATS_TX_PACKETS] = { .type = NLA_U64 },
+[IFLA_VF_STATS_RX_BYTES]= { .type = NLA_U64 },
+[IFLA_VF_STATS_TX_BYTES]= { .type = NLA_U64 },
+[IFLA_VF_STATS_BROADCAST] = { .type = NLA_U64 },
+[IFLA_VF_STATS_MULTICAST] = { .type = NLA_U64 },
+};
+
+static int
+virNetDevParseVfStats(struct nlattr **tb, virMacAddrPtr mac,
+ virDomainInterfaceStatsPtr stats)
+{
+int ret = -1, len;
+struct ifla_vf_mac *vf_lladdr;
+struct nlattr *nla, *t[IFLA_VF_MAX+1];
+struct nlattr *stb[IFLA_VF_STATS_MAX+1];
+
+if (tb == NULL || mac == NULL || stats == NULL) {
+return -1;
+}
+
+if (!tb[IFLA_VFINFO_LIST])
+return -1;
+
+len = nla_len(tb[IFLA_VFINFO_LIST]);
+
+for (nla = nla_data(tb[IFLA_VFINFO_LIST]); nla_ok(nla, len);
+nla = nla_next(nla, )) {
+ret = nla_parse(t, IFLA_VF_MAX, nla_data(nla), nla_len(nla),
+ifla_vf_policy);
+if (ret < 0)
+return -1;
+
+if (t[IFLA_VF_MAC] == NULL) {
+continue;
+}
+
+vf_lladdr = nla_data(t[IFLA_VF_MAC]);
+if (virMacAddrCmpRaw(mac, vf_lladdr->mac)) {
+continue;
+}
+
+if (t[IFLA_VF_STATS]) {
+ret = nla_parse_nested(stb, IFLA_VF_STATS_MAX,
+t[IFLA_VF_STATS],
+ifla_vfstats_policy);
+if (ret < 0)
+return -1;
+
+stats->rx_bytes = nla_get_u64(stb[IFLA_VF_STATS_RX_BYTES]);
+stats->tx_bytes = nla_get_u64(stb[IFLA_VF_STATS_TX_BYTES]);
+stats->rx_packets = nla_get_u64(stb[IFLA_VF_STATS_RX_PACKETS]);
+stats->tx_packets = nla_get_u64(stb[IFLA_VF_STATS_TX_PACKETS]);
+}
+return 0;
+}
+
+return ret;
+}
+
+/**
+ * virNetDevVFInterfaceStats:
+ * @mac: MAC address of the VF interface
+ * @stats: returns stats of the VF interface
+ *
+ * Get the
[libvirt PATCH 4/4] tests: use g_new0 instead of VIR_ALLOC
Signed-off-by: Ján Tomko
---
tests/domaincapstest.c | 7 +++
tests/nodedevmdevctltest.c | 16 +---
tests/nwfilterxml2firewalltest.c | 3 +--
tests/qemublocktest.c| 9 +++--
tests/qemuhotplugtest.c | 3 +--
tests/qemumonitorjsontest.c | 12
tests/qemumonitortestutils.c | 22 ++
tests/qemusecuritymock.c | 3 +--
tests/securityselinuxhelper.c| 3 +--
tests/securityselinuxtest.c | 3 +--
tests/virhostdevtest.c | 3 +--
tests/virnetdaemontest.c | 3 +--
tests/virnetmessagetest.c| 9 -
tests/virnetserverclienttest.c | 3 +--
tests/virstringtest.c| 5 ++---
15 files changed, 35 insertions(+), 69 deletions(-)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index b3d36f55fc..9b0a0c5758 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -148,9 +148,9 @@ fillXenCaps(virDomainCapsPtr domCaps)
int ret = -1;
firmwares = g_new0(virFirmwarePtr, 2);
+firmwares[0] = g_new0(virFirmware, 1);
+firmwares[1] = g_new0(virFirmware, 1);
-if (VIR_ALLOC(firmwares[0]) < 0 || VIR_ALLOC(firmwares[1]) < 0)
-goto cleanup;
firmwares[0]->name = g_strdup("/usr/lib/xen/boot/hvmloader");
firmwares[1]->name = g_strdup("/usr/lib/xen/boot/ovmf.bin");
@@ -174,8 +174,7 @@ fillBhyveCaps(virDomainCapsPtr domCaps, unsigned int
*bhyve_caps)
virDomainCapsStringValuesPtr firmwares = NULL;
int ret = -1;
-if (VIR_ALLOC(firmwares) < 0)
-return -1;
+firmwares = g_new0(virDomainCapsStringValues, 1);
if (fillStringValues(firmwares, "/foo/bar", "/foo/baz", NULL) < 0)
goto cleanup;
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 57c1ad4f46..dab4b487b9 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -162,11 +162,8 @@ fakeRootDevice(void)
{
virNodeDeviceDefPtr def = NULL;
-if (VIR_ALLOC(def) != 0 || VIR_ALLOC(def->caps) != 0) {
-virNodeDeviceDefFree(def);
-return NULL;
-}
-
+def = g_new0(virNodeDeviceDef, 1);
+def->caps = g_new0(virNodeDevCapsDef, 1);
def->name = g_strdup("computer");
return def;
@@ -182,10 +179,8 @@ fakeParentDevice(void)
virNodeDeviceDefPtr def = NULL;
virNodeDevCapPCIDevPtr pci_dev;
-if (VIR_ALLOC(def) != 0 || VIR_ALLOC(def->caps) != 0) {
-virNodeDeviceDefFree(def);
-return NULL;
-}
+def = g_new0(virNodeDeviceDef, 1);
+def->caps = g_new0(virNodeDevCapsDef, 1);
def->name = g_strdup("pci__00_02_0");
def->parent = g_strdup("computer");
@@ -233,8 +228,7 @@ static int
nodedevTestDriverInit(void)
{
int ret = -1;
-if (VIR_ALLOC(driver) < 0)
-return -1;
+driver = g_new0(virNodeDeviceDriverState, 1);
driver->lockFD = -1;
if (virMutexInit(>lock) < 0 ||
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index dff2b62028..755732487a 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -209,8 +209,7 @@ virNWFilterRuleDefToRuleInst(virNWFilterDefPtr def,
virNWFilterRuleInstPtr ruleinst;
int ret = -1;
-if (VIR_ALLOC(ruleinst) < 0)
-goto cleanup;
+ruleinst = g_new0(virNWFilterRuleInst, 1);
ruleinst->chainSuffix = def->chainsuffix;
ruleinst->chainPriority = def->chainPriority;
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0685b703a1..2489b883ff 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -234,8 +234,7 @@ testQemuDiskXMLToJSONFakeSecrets(virStorageSourcePtr src)
srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
if (src->auth) {
-if (VIR_ALLOC(srcpriv->secinfo) < 0)
-return -1;
+srcpriv->secinfo = g_new0(qemuDomainSecretInfo, 1);
srcpriv->secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES;
srcpriv->secinfo->s.aes.username = g_strdup(src->auth->username);
@@ -245,8 +244,7 @@ testQemuDiskXMLToJSONFakeSecrets(virStorageSourcePtr src)
}
if (src->encryption) {
-if (VIR_ALLOC(srcpriv->encinfo) < 0)
-return -1;
+srcpriv->encinfo = g_new0(qemuDomainSecretInfo, 1);
srcpriv->encinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES;
srcpriv->encinfo->s.aes.alias = g_strdup_printf("%s-encalias",
@@ -493,8 +491,7 @@ testQemuImageCreateLoadDiskXML(const char *name,
return NULL;
}
-if (VIR_ALLOC(diskdef) < 0)
-return NULL;
+diskdef = g_new0(virDomainSnapshotDiskDef, 1);
if (virDomainSnapshotDiskDefParseXML(node, ctxt, diskdef,
VIR_DOMAIN_DEF_PARSE_STATUS,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index b023c49a69..2d12cacf28 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -413,8 +413,7 @@
[libvirt PATCH 2/4] tests: cpuTestLoadMultiXML: use g_new0 instead of VIR_ALLOC_N
Signed-off-by: Ján Tomko
---
tests/cputest.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/cputest.c b/tests/cputest.c
index 83d63bf495..383da94938 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -111,11 +111,13 @@ cpuTestLoadMultiXML(virArch arch,
goto cleanup;
n = virXPathNodeSet("/cpuTest/cpu", ctxt, );
-if (n <= 0 || (VIR_ALLOC_N(cpus, n) < 0)) {
+if (n <= 0) {
fprintf(stderr, "\nNo /cpuTest/cpu elements found in %s\n", xml);
goto cleanup;
}
+cpus = g_new0(virCPUDefPtr, n);
+
for (i = 0; i < n; i++) {
ctxt->node = nodes[i];
if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, [i]) < 0)
--
2.26.2
[libvirt PATCH 3/4] tests: use g_new0 instead of VIR_ALLOC_N
Signed-off-by: Ján Tomko
---
tests/commandtest.c | 7 +++
tests/domaincapstest.c | 3 +--
tests/fdstreamtest.c | 10 --
tests/qemuxml2argvtest.c | 3 +--
tests/securityselinuxlabeltest.c | 3 +--
tests/securityselinuxtest.c | 3 +--
tests/testutils.c| 8 ++--
tests/testutilsqemu.c| 6 ++
tests/vircgrouptest.c| 3 +--
tests/vircryptotest.c| 5 ++---
tests/virhostcputest.c | 3 +--
tests/virnetmessagetest.c| 6 ++
tests/virnetsockettest.c | 6 +-
tests/virnettlshelpers.c | 3 +--
tests/virstringtest.c| 3 +--
tests/xlconfigtest.c | 3 +--
tests/xmconfigtest.c | 3 +--
17 files changed, 26 insertions(+), 52 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index cbbcda4e5f..df86725f0e 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1057,10 +1057,9 @@ static int test27(const void *unused G_GNUC_UNUSED)
"%s%s%s" \
"END STDERR\n"
-if (VIR_ALLOC_N(buffer0, buflen) < 0 ||
-VIR_ALLOC_N(buffer1, buflen) < 0 ||
-VIR_ALLOC_N(buffer2, buflen) < 0)
-goto cleanup;
+buffer0 = g_new0(char, buflen);
+buffer1 = g_new0(char, buflen);
+buffer2 = g_new0(char, buflen);
memset(buffer0, 'H', buflen - 2);
buffer0[buflen - 2] = '\n';
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index d2ed820cf9..b3d36f55fc 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -147,8 +147,7 @@ fillXenCaps(virDomainCapsPtr domCaps)
virFirmwarePtr *firmwares;
int ret = -1;
-if (VIR_ALLOC_N(firmwares, 2) < 0)
-return ret;
+firmwares = g_new0(virFirmwarePtr, 2);
if (VIR_ALLOC(firmwares[0]) < 0 || VIR_ALLOC(firmwares[1]) < 0)
goto cleanup;
diff --git a/tests/fdstreamtest.c b/tests/fdstreamtest.c
index 83973137e7..7a2fe27477 100644
--- a/tests/fdstreamtest.c
+++ b/tests/fdstreamtest.c
@@ -54,9 +54,8 @@ static int testFDStreamReadCommon(const char *scratchdir,
bool blocking)
if (!(conn = virConnectOpen("test:///default")))
goto cleanup;
-if (VIR_ALLOC_N(pattern, PATTERN_LEN) < 0 ||
-VIR_ALLOC_N(buf, PATTERN_LEN) < 0)
-goto cleanup;
+pattern = g_new0(char, PATTERN_LEN);
+buf = g_new0(char, PATTERN_LEN);
for (i = 0; i < PATTERN_LEN; i++)
pattern[i] = i;
@@ -185,9 +184,8 @@ static int testFDStreamWriteCommon(const char *scratchdir,
bool blocking)
if (!(conn = virConnectOpen("test:///default")))
goto cleanup;
-if (VIR_ALLOC_N(pattern, PATTERN_LEN) < 0 ||
-VIR_ALLOC_N(buf, PATTERN_LEN) < 0)
-goto cleanup;
+pattern = g_new0(char, PATTERN_LEN);
+buf = g_new0(char, PATTERN_LEN);
for (i = 0; i < PATTERN_LEN; i++)
pattern[i] = i;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e93948e3fc..463e4c003d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -753,8 +753,7 @@ mymain(void)
driver.config->nbdTLSx509certdir =
g_strdup("/etc/pki/libvirt-nbd/dummy,path");
VIR_FREE(driver.config->hugetlbfs);
-if (VIR_ALLOC_N(driver.config->hugetlbfs, 2) < 0)
-return EXIT_FAILURE;
+driver.config->hugetlbfs = g_new0(virHugeTLBFS, 2);
driver.config->nhugetlbfs = 2;
driver.config->hugetlbfs[0].mnt_dir = g_strdup("/dev/hugepages2M");
driver.config->hugetlbfs[1].mnt_dir = g_strdup("/dev/hugepages1G");
diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 2989a668b7..168acc2bdf 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -114,8 +114,7 @@ testSELinuxLoadFileList(const char *testname,
if (!(fp = fopen(path, "r")))
goto cleanup;
-if (VIR_ALLOC_N(line, 1024) < 0)
-goto cleanup;
+line = g_new0(char, 1024);
while (!feof(fp)) {
char *file = NULL, *context = NULL, *tmp;
diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c
index ae4b08b9d8..fd746d1ca1 100644
--- a/tests/securityselinuxtest.c
+++ b/tests/securityselinuxtest.c
@@ -71,8 +71,7 @@ testBuildDomainDef(bool dynamic,
goto error;
def->virtType = VIR_DOMAIN_VIRT_KVM;
-if (VIR_ALLOC_N(def->seclabels, 1) < 0)
-goto error;
+def->seclabels = g_new0(char, 1);
if (VIR_ALLOC(secdef) < 0)
goto error;
diff --git a/tests/testutils.c b/tests/testutils.c
index 3f53f635fc..b747f65eea 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -211,10 +211,7 @@ virTestLoadFile(const char *file, char **buf)
tmplen = buflen = st.st_size + 1;
-if (VIR_ALLOC_N(*buf, buflen) < 0) {
-VIR_FORCE_FCLOSE(fp);
-return -1;
-}
+*buf = g_new0(char, buflen);
tmp = *buf;
(*buf)[0] = '\0';
@@ -977,8 +974,7 @@ virTestCapsBuildNUMATopology(int
[libvirt PATCH 1/4] tests: virNumaGetPages: use g_new0 instead of VIR_ALLOC_N
Signed-off-by: Ján Tomko
---
tests/virnumamock.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index e6282c7f33..40e18e646e 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -133,23 +133,13 @@ virNumaGetPages(int node,
size_t i = 0;
if (pages_size)
-*pages_size = NULL;
+*pages_size = g_new0(unsigned int, npages_def);
if (pages_avail)
-*pages_avail = NULL;
+*pages_avail = g_new0(unsigned long long, npages_def);
if (pages_free)
-*pages_free = NULL;
-
-*npages = 0;
-
-if ((pages_size && VIR_ALLOC_N(*pages_size, npages_def) < 0) ||
-(pages_avail && VIR_ALLOC_N(*pages_avail, npages_def) < 0) ||
-(pages_free && VIR_ALLOC_N(*pages_free, npages_def) < 0)) {
-VIR_FREE(*pages_size);
-VIR_FREE(*pages_avail);
-return -1;
-}
+*pages_free = g_new0(unsigned long long, npages_def);
*npages = npages_def;
if (pages_size)
--
2.26.2
[libvirt PATCH 0/4] tests: switch to g_new0 (glib chronicles)
With the exception of viralloctest. Ján Tomko (4): tests: virNumaGetPages: use g_new0 instead of VIR_ALLOC_N tests: cpuTestLoadMultiXML: use g_new0 instead of VIR_ALLOC_N tests: use g_new0 instead of VIR_ALLOC_N tests: use g_new0 instead of VIR_ALLOC tests/commandtest.c | 7 +++ tests/cputest.c | 4 +++- tests/domaincapstest.c | 10 -- tests/fdstreamtest.c | 10 -- tests/nodedevmdevctltest.c | 16 +--- tests/nwfilterxml2firewalltest.c | 3 +-- tests/qemublocktest.c| 9 +++-- tests/qemuhotplugtest.c | 3 +-- tests/qemumonitorjsontest.c | 12 tests/qemumonitortestutils.c | 22 ++ tests/qemusecuritymock.c | 3 +-- tests/qemuxml2argvtest.c | 3 +-- tests/securityselinuxhelper.c| 3 +-- tests/securityselinuxlabeltest.c | 3 +-- tests/securityselinuxtest.c | 6 ++ tests/testutils.c| 8 ++-- tests/testutilsqemu.c| 6 ++ tests/vircgrouptest.c| 3 +-- tests/vircryptotest.c| 5 ++--- tests/virhostcputest.c | 3 +-- tests/virhostdevtest.c | 3 +-- tests/virnetdaemontest.c | 3 +-- tests/virnetmessagetest.c| 15 ++- tests/virnetserverclienttest.c | 3 +-- tests/virnetsockettest.c | 6 +- tests/virnettlshelpers.c | 3 +-- tests/virnumamock.c | 16 +++- tests/virstringtest.c| 8 +++- tests/xlconfigtest.c | 3 +-- tests/xmconfigtest.c | 3 +-- 30 files changed, 67 insertions(+), 135 deletions(-) -- 2.26.2
[PATCH] apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
Like other distros, openSUSE Tumbleweed recently changed libexecdir from
/usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper
and pygrub.
Signed-off-by: Jim Fehlig
---
I considered including /usr/lib64, but I don't think any distros are
installing xen libexecdir targets to /usr/lib64. Happy to include it
if I'm wrong :-).
src/security/apparmor/usr.sbin.libvirtd.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/security/apparmor/usr.sbin.libvirtd.in
b/src/security/apparmor/usr.sbin.libvirtd.in
index f2030764cd..bf4563e1e8 100644
--- a/src/security/apparmor/usr.sbin.libvirtd.in
+++ b/src/security/apparmor/usr.sbin.libvirtd.in
@@ -86,8 +86,8 @@ profile libvirtd @sbindir@/libvirtd
flags=(attach_disconnected) {
/{usr/,}lib/udev/scsi_id PUx,
/usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
/usr/{lib,lib64}/xen/bin/* Ux,
- /usr/lib/xen-*/bin/libxl-save-helper PUx,
- /usr/lib/xen-*/bin/pygrub PUx,
+ /usr/{lib,libexec}/xen-*/bin/libxl-save-helper PUx,
+ /usr/{lib,libexec}/xen-*/bin/pygrub PUx,
/usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx,
/usr/{lib,lib64,lib/qemu,libexec}/virtiofsd PUx,
--
2.28.0
Re: [PATCH v2 2/2] block: deprecate the sheepdog block driver
22.09.2020 19:16, Daniel P. Berrangé wrote: This thread from a little over a year ago: http://lists.wpkg.org/pipermail/sheepdog/2019-March/thread.html states that sheepdog is no longer actively developed. The only mentioned users are some companies who are said to have it for legacy reasons with plans to replace it by Ceph. There is talk about cutting out existing features to turn it into a simple demo of how to write a distributed block service. There is no evidence of anyone working on that idea: https://github.com/sheepdog/sheepdog/commits/master No real commits to git since Jan 2018, and before then just some minor technical debt cleanup.. There is essentially no activity on the mailing list aside from patches to QEMU that get CC'd due to our MAINTAINERS entry. Fedora packages for sheepdog failed to build from upstream source because of the more strict linker that no longer merges duplicate global symbols. Fedora patches it to add the missing "extern" annotations and presumably other distros do to, but upstream source remains broken. There is only basic compile testing, no functional testing of the driver. Since there are no build pre-requisites the sheepdog driver is currently enabled unconditionally. This would result in configure issuing a deprecation warning by default for all users. Thus the configure default is changed to disable it, requiring users to pass --enable-sheepdog to build the driver. Signed-off-by: Daniel P. Berrangé Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH v2 1/2] block: drop moderated sheepdog mailing list from MAINTAINERS file
22.09.2020 19:16, Daniel P. Berrangé wrote: The sheepdog mailing list is setup to stop and queue messages from non-subscribers, pending moderator approval. Unfortunately it seems that the moderation queue is not actively dealt with. Even when messages are approved, the sender is never added to the whitelist, so every future mail from the same sender continues to get stopped for moderation. MAINTAINERS entries should be responsive and not unneccessarily block mails from QEMU contributors, so drop the sheepdog mailing list. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
[libvirt PATCH 5/5] Use (un)signed printf specifiers correctly
Various places reported by cppcheck's invalidPrintfArgType_sint
and invalidPrintfArgType_uint.
Signed-off-by: Ján Tomko
---
examples/c/domain/domtop.c | 2 +-
examples/c/domain/suspend.c | 2 +-
tests/qemusecuritymock.c| 2 +-
tests/virhashtest.c | 4 ++--
tests/virpcimock.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/examples/c/domain/domtop.c b/examples/c/domain/domtop.c
index 15611c586d..5228445b7c 100644
--- a/examples/c/domain/domtop.c
+++ b/examples/c/domain/domtop.c
@@ -115,7 +115,7 @@ parse_argv(int argc, char *argv[],
}
*milliseconds = val;
if (*milliseconds != val) {
-ERROR("Integer overflow: %ld", val);
+ERROR("Integer overflow: %lu", val);
exit(EXIT_FAILURE);
}
break;
diff --git a/examples/c/domain/suspend.c b/examples/c/domain/suspend.c
index 980c4584c7..3ff24f6861 100644
--- a/examples/c/domain/suspend.c
+++ b/examples/c/domain/suspend.c
@@ -105,7 +105,7 @@ parse_argv(int argc, char *argv[],
}
*seconds = val;
if (*seconds != val) {
-ERROR("Integer overflow: %ld", val);
+ERROR("Integer overflow: %lu", val);
return -1;
}
break;
diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c
index 6da8a91a9e..d544ae56cd 100644
--- a/tests/qemusecuritymock.c
+++ b/tests/qemusecuritymock.c
@@ -265,7 +265,7 @@ mock_chown(const char *path,
int ret = -1;
if (gid >> 16 || uid >> 16) {
-fprintf(stderr, "Attempt to set too high UID or GID: %lld %lld",
+fprintf(stderr, "Attempt to set too high UID or GID: %llu %llu",
(unsigned long long) uid, (unsigned long long) gid);
abort();
}
diff --git a/tests/virhashtest.c b/tests/virhashtest.c
index 4d05cbb0f8..af30791241 100644
--- a/tests/virhashtest.c
+++ b/tests/virhashtest.c
@@ -34,7 +34,7 @@ testHashInit(int size)
}
if (virHashTableSize(hash) != oldsize) {
-VIR_TEST_DEBUG("hash grown from %zd to %zd",
+VIR_TEST_DEBUG("hash grown from %zu to %zu",
(size_t)oldsize, (size_t)virHashTableSize(hash));
}
}
@@ -313,7 +313,7 @@ testHashRemoveSet(const void *data G_GNUC_UNUSED)
if (count != rcount) {
VIR_TEST_VERBOSE("\nvirHashRemoveSet didn't remove expected number of"
- " entries, %d != %u",
+ " entries, %d != %d",
rcount, count);
goto cleanup;
}
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 787172d24c..469aa01bb0 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -120,7 +120,7 @@ struct pciDeviceAddress {
unsigned int device;
unsigned int function;
};
-# define ADDR_STR_FMT "%04x:%02x:%02x.%d"
+# define ADDR_STR_FMT "%04x:%02x:%02x.%u"
struct pciDevice {
struct pciDeviceAddress addr;
--
2.26.2
[libvirt PATCH 3/5] rpc: socket: properly call virSetCloseExec
cppcheck reports:
style: Argument 'fd<0' to function virSetCloseExec is always 0 [knownArgument]
Signed-off-by: Ján Tomko
Fixes: 4b9919af4024a6fbc3d4ee996d8a4c27dbc44285
---
src/rpc/virnetsocket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index ebdeadc4a0..f79a638775 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1388,7 +1388,7 @@ int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec)
}
#ifndef F_DUPFD_CLOEXEC
if (cloexec &&
-virSetCloseExec(fd < 0)) {
+virSetCloseExec(fd) < 0) {
int saveerr = errno;
closesocket(fd);
errno = saveerr;
--
2.26.2
[libvirt PATCH 0/5] Various small fixes
Reported by Coverity (7 months ago) or cppcheck (today). Ján Tomko (5): util: event: check return value of virInitialize qemu: firmware: check virJSONValueObjectGet return value rpc: socket: properly call virSetCloseExec virsh: do not return bool in virshNetworkPortUUIDCompleter Use (un)signed printf specifiers correctly examples/c/domain/domtop.c | 2 +- examples/c/domain/suspend.c | 2 +- src/qemu/qemu_firmware.c| 9 - src/rpc/virnetsocket.c | 2 +- src/util/virevent.c | 3 ++- tests/qemusecuritymock.c| 2 +- tests/virhashtest.c | 4 ++-- tests/virpcimock.c | 2 +- tools/virsh-completer-network.c | 2 +- 9 files changed, 18 insertions(+), 10 deletions(-) -- 2.26.2
[libvirt PATCH 1/5] util: event: check return value of virInitialize
This function can possibly fail.
Signed-off-by: Ján Tomko
Fixes: 2e07a1e14635ad25c57b66c13488feff4c8d2b0c
---
src/util/virevent.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/util/virevent.c b/src/util/virevent.c
index 3477d63554..f6eb806faf 100644
--- a/src/util/virevent.c
+++ b/src/util/virevent.c
@@ -305,7 +305,8 @@ int virEventRegisterDefaultImpl(void)
{
VIR_DEBUG("registering default event implementation");
-virInitialize();
+if (virInitialize() < 0)
+return -1;
virResetLastError();
--
2.26.2
[libvirt PATCH 4/5] virsh: do not return bool in virshNetworkPortUUIDCompleter
portability: Returning an integer in a function with pointer return type is not portable. [CastIntegerToAddressAtReturn] Signed-off-by: Ján Tomko --- tools/virsh-completer-network.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c index a63d657477..73f7115ab2 100644 --- a/tools/virsh-completer-network.c +++ b/tools/virsh-completer-network.c @@ -103,7 +103,7 @@ virshNetworkPortUUIDCompleter(vshControl *ctl, return NULL; if (!(net = virshCommandOptNetwork(ctl, cmd, NULL))) -return false; +return NULL; if ((nports = virNetworkListAllPorts(net, , flags)) < 0) return NULL; -- 2.26.2
[libvirt PATCH 2/5] qemu: firmware: check virJSONValueObjectGet return value
If the mapping is not present, we should not try to
access its elements.
Signed-off-by: Ján Tomko
Fixes: 8b5b80f4c5f7342eedce0747469223387ab709ef
---
src/qemu/qemu_firmware.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index ffe2df20aa..480ce0b00d 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -434,10 +434,17 @@ qemuFirmwareMappingParse(const char *path,
virJSONValuePtr doc,
qemuFirmwarePtr fw)
{
-virJSONValuePtr mapping = virJSONValueObjectGet(doc, "mapping");
+virJSONValuePtr mapping;
const char *deviceStr;
int tmp;
+if (!(mapping = virJSONValueObjectGet(doc, "mapping"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing mapping in '%s'"),
+ path);
+return -1;
+}
+
if (!(deviceStr = virJSONValueObjectGetString(mapping, "device"))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing device type in '%s'"),
--
2.26.2
Re: [PATCH v2 0/2] block: deprecate the sheepdog driver
Patchew URL: https://patchew.org/QEMU/20200922161611.2049616-1-berra...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20200922161611.2049616-1-berra...@redhat.com Subject: [PATCH v2 0/2] block: deprecate the sheepdog driver === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Switched to a new branch 'test' 2c11635 block: deprecate the sheepdog block driver aa3d54c block: drop moderated sheepdog mailing list from MAINTAINERS file === OUTPUT BEGIN === 1/2 Checking commit aa3d54cfa28f (block: drop moderated sheepdog mailing list from MAINTAINERS file) 2/2 Checking commit 2c1163558579 (block: deprecate the sheepdog block driver) ERROR: do not initialise statics to 0 or NULL #54: FILE: block/sheepdog.c:247: +static bool warned = false; total: 1 errors, 0 warnings, 71 lines checked Patch 2/2 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200922161611.2049616-1-berra...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [RFC] Add basic driver for the Cloud-Hypervisor
On Tue, Sep 22, 2020 at 12:07 PM Ján Tomko wrote:
>
> On a Tuesday in 2020, Douglas, William wrote:
> >On Tue, Sep 22, 2020 at 1:11 AM Daniel P. Berrangé
> >wrote:
> >>
> >> On Mon, Sep 21, 2020 at 12:05:48PM -0700, Douglas, William wrote:
> >> > On Tue, Sep 15, 2020 at 5:53 AM Daniel P. Berrangé
> >> > wrote:
> >> > >
> >> > > On Thu, Aug 27, 2020 at 11:24:32AM -0700, William Douglas wrote:
> >> >
> >> >
> >> >
> >> > > > +virCHMonitorPtr
> >> > > > +virCHMonitorNew(virDomainObjPtr vm, const char *socketdir)
> >> > > > +{
> >> > > > +virCHMonitorPtr ret = NULL;
> >> > > > +virCHMonitorPtr mon = NULL;
> >> > > > +virCommandPtr cmd = NULL;
> >> > > > +int pings = 0;
> >> > > > +
> >> > > > +if (virCHMonitorInitialize() < 0)
> >> > > > +return NULL;
> >> > > > +
> >> > > > +if (!(mon = virObjectLockableNew(virCHMonitorClass)))
> >> > > > +return NULL;
> >> > > > +
> >> > > > +mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
> >> > > > vm->def->name);
> >> > > > +
> >> > > > +/* prepare to launch Cloud-Hypervisor socket */
> >> > > > +if (!(cmd = chMonitorBuildSocketCmd(vm, mon->socketpath)))
> >> > > > +goto cleanup;
> >> > > > +
> >> > > > +if (virFileMakePath(socketdir) < 0) {
> >> > > > +virReportSystemError(errno,
> >> > > > + _("Cannot create socket directory
> >> > > > '%s'"),
> >> > > > + socketdir);
> >> > > > +goto cleanup;
> >> > > > +}
> >> > > > +
> >> > > > +/* launch Cloud-Hypervisor socket */
> >> > > > +if (virCommandRunAsync(cmd, >pid) < 0)
> >> > > > +goto cleanup;
> >> > > > +
> >> > > > +/* get a curl handle */
> >> > > > +mon->handle = curl_easy_init();
> >> > > > +
> >> > > > +/* try to ping VMM socket 5 times to make sure it is ready */
> >> > > > +while (pings < 5) {
> >> > > > +if (virCHMonitorPingVMM(mon) == 0)
> >> > > > +break;
> >> > > > +if (pings == 5)
> >> > > > +goto cleanup;
> >> > > > +
> >> > > > +g_usleep(100 * 1000);
> >> > > > +}
> >> > >
> >> > > This is highly undesirable. Is there no way to launch the CH process
> >> > > such that the socket is guaranteed to be accepting requests by the
> >> > > time it has forked into the background ? Or is there a way to pass
> >> > > in a pre-opened FD for the monitor socket ?
> >> > >
> >> > > This kind of wait with timeout will cause startup failures due to
> >> > > timeout under load.
> >> >
> >> > Currently the cloud-hypervisor process doesn't fork into the
> >> > background so that was initially what I was thinking to add to
> >> > cloud-hypervsior. Would tracking the pid of the cloud-hypervsior
> >> > running in the background be required at that point (assuming the
> >> > socket path to control the daemon would be known and working given
> >> > virCommandRun returns successfully)?
> >>
> >> It doesn't especially matter whether cloud-hypervsior forks into
> >> the background itself, or whether libvirt forks it into the
> >> background when launching it.
> >>
> >> The important thing is simply to ensure that whichever startup
> >> approach is used can be implemented in a race-free manner without
> >> needing busy-waits or timeouts.
> >>
> >> If cloud-hypervisor forks into the background, then it would
> >> need to write a pidfile so libvirt can determine the pid that
> >> ended up running. The act of libvirt waiting for the pidfile
> >> to be written is potentially racy though, so that might not be
> >> be best.
> >>
> >> Based on what we learnt from QEMU, I think being able to pass
> >> in a pre-created UNIX listener socket is the best. That lets
> >> libvirt immedately issue a connect() start after forking the
> >> cloud-hypervisor process. If cloud-hypervisor succesfully
> >> starts up, then the client socket becomes live and can be used.
> >> if it fails to startup, then the client socket libvirt has
> >> will get EOF and thus we'll see the startup failure. This
> >> avoids any looping/sleeping/timeout.
> >
> >I think I'm misreading/missing something from the qemu setup but
> >looking at qemu_monitor.c's qemuMonitorOpenUnix function I see a retry
> >connect loop based on a timeout when trying to connect to a socket
>
> That loop is guarded by: if (retry)
>
> The important code path:
> qemuProcessLaunch
> `_ qemuProcessWaitForMonitor
>`_ qemuConnectMonitor
>
> qemuProcessWaitForMonitor sets retry=false if we're dealing with
> a recent enough QEMU that supports FD passing for its chardevs
>
> The logic that creates the socket on libvirt's side lives (sadly)
> in qemuBuildChrChardevStr.
>
Ah I was missing the possibility of retry being set to false (I saw
false would be set on a reconnect I think but didn't look much
further) and the entirety of the qemuBuildChrChardevStr being the key
part of the setup. Thank you very much!
Re: [RFC] Add basic driver for the Cloud-Hypervisor
On a Tuesday in 2020, Douglas, William wrote:
On Tue, Sep 22, 2020 at 1:11 AM Daniel P. Berrangé wrote:
On Mon, Sep 21, 2020 at 12:05:48PM -0700, Douglas, William wrote:
> On Tue, Sep 15, 2020 at 5:53 AM Daniel P. Berrangé
wrote:
> >
> > On Thu, Aug 27, 2020 at 11:24:32AM -0700, William Douglas wrote:
>
>
>
> > > +virCHMonitorPtr
> > > +virCHMonitorNew(virDomainObjPtr vm, const char *socketdir)
> > > +{
> > > +virCHMonitorPtr ret = NULL;
> > > +virCHMonitorPtr mon = NULL;
> > > +virCommandPtr cmd = NULL;
> > > +int pings = 0;
> > > +
> > > +if (virCHMonitorInitialize() < 0)
> > > +return NULL;
> > > +
> > > +if (!(mon = virObjectLockableNew(virCHMonitorClass)))
> > > +return NULL;
> > > +
> > > +mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
vm->def->name);
> > > +
> > > +/* prepare to launch Cloud-Hypervisor socket */
> > > +if (!(cmd = chMonitorBuildSocketCmd(vm, mon->socketpath)))
> > > +goto cleanup;
> > > +
> > > +if (virFileMakePath(socketdir) < 0) {
> > > +virReportSystemError(errno,
> > > + _("Cannot create socket directory '%s'"),
> > > + socketdir);
> > > +goto cleanup;
> > > +}
> > > +
> > > +/* launch Cloud-Hypervisor socket */
> > > +if (virCommandRunAsync(cmd, >pid) < 0)
> > > +goto cleanup;
> > > +
> > > +/* get a curl handle */
> > > +mon->handle = curl_easy_init();
> > > +
> > > +/* try to ping VMM socket 5 times to make sure it is ready */
> > > +while (pings < 5) {
> > > +if (virCHMonitorPingVMM(mon) == 0)
> > > +break;
> > > +if (pings == 5)
> > > +goto cleanup;
> > > +
> > > +g_usleep(100 * 1000);
> > > +}
> >
> > This is highly undesirable. Is there no way to launch the CH process
> > such that the socket is guaranteed to be accepting requests by the
> > time it has forked into the background ? Or is there a way to pass
> > in a pre-opened FD for the monitor socket ?
> >
> > This kind of wait with timeout will cause startup failures due to
> > timeout under load.
>
> Currently the cloud-hypervisor process doesn't fork into the
> background so that was initially what I was thinking to add to
> cloud-hypervsior. Would tracking the pid of the cloud-hypervsior
> running in the background be required at that point (assuming the
> socket path to control the daemon would be known and working given
> virCommandRun returns successfully)?
It doesn't especially matter whether cloud-hypervsior forks into
the background itself, or whether libvirt forks it into the
background when launching it.
The important thing is simply to ensure that whichever startup
approach is used can be implemented in a race-free manner without
needing busy-waits or timeouts.
If cloud-hypervisor forks into the background, then it would
need to write a pidfile so libvirt can determine the pid that
ended up running. The act of libvirt waiting for the pidfile
to be written is potentially racy though, so that might not be
be best.
Based on what we learnt from QEMU, I think being able to pass
in a pre-created UNIX listener socket is the best. That lets
libvirt immedately issue a connect() start after forking the
cloud-hypervisor process. If cloud-hypervisor succesfully
starts up, then the client socket becomes live and can be used.
if it fails to startup, then the client socket libvirt has
will get EOF and thus we'll see the startup failure. This
avoids any looping/sleeping/timeout.
I think I'm misreading/missing something from the qemu setup but
looking at qemu_monitor.c's qemuMonitorOpenUnix function I see a retry
connect loop based on a timeout when trying to connect to a socket
That loop is guarded by: if (retry)
The important code path:
qemuProcessLaunch
`_ qemuProcessWaitForMonitor
`_ qemuConnectMonitor
qemuProcessWaitForMonitor sets retry=false if we're dealing with
a recent enough QEMU that supports FD passing for its chardevs
The logic that creates the socket on libvirt's side lives (sadly)
in qemuBuildChrChardevStr.
Jano
(with a comment that leads me to believe it is possible to have the
monitor socket file not yet exist). I was expecting to see something
like forking qemu and passing the fd for the socket that was opened
that qemu should use (and that libvirt connects to) but I think I only
see that for the network socket fds that qemu is supposed to use. If
you could point me in the right direction here I'd appreciate it.
signature.asc
Description: PGP signature
Re: [RFC] Add basic driver for the Cloud-Hypervisor
On Tue, Sep 22, 2020 at 1:11 AM Daniel P. Berrangé wrote:
>
> On Mon, Sep 21, 2020 at 12:05:48PM -0700, Douglas, William wrote:
> > On Tue, Sep 15, 2020 at 5:53 AM Daniel P. Berrangé
> > wrote:
> > >
> > > On Thu, Aug 27, 2020 at 11:24:32AM -0700, William Douglas wrote:
> >
> >
> >
> > > > +virCHMonitorPtr
> > > > +virCHMonitorNew(virDomainObjPtr vm, const char *socketdir)
> > > > +{
> > > > +virCHMonitorPtr ret = NULL;
> > > > +virCHMonitorPtr mon = NULL;
> > > > +virCommandPtr cmd = NULL;
> > > > +int pings = 0;
> > > > +
> > > > +if (virCHMonitorInitialize() < 0)
> > > > +return NULL;
> > > > +
> > > > +if (!(mon = virObjectLockableNew(virCHMonitorClass)))
> > > > +return NULL;
> > > > +
> > > > +mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
> > > > vm->def->name);
> > > > +
> > > > +/* prepare to launch Cloud-Hypervisor socket */
> > > > +if (!(cmd = chMonitorBuildSocketCmd(vm, mon->socketpath)))
> > > > +goto cleanup;
> > > > +
> > > > +if (virFileMakePath(socketdir) < 0) {
> > > > +virReportSystemError(errno,
> > > > + _("Cannot create socket directory '%s'"),
> > > > + socketdir);
> > > > +goto cleanup;
> > > > +}
> > > > +
> > > > +/* launch Cloud-Hypervisor socket */
> > > > +if (virCommandRunAsync(cmd, >pid) < 0)
> > > > +goto cleanup;
> > > > +
> > > > +/* get a curl handle */
> > > > +mon->handle = curl_easy_init();
> > > > +
> > > > +/* try to ping VMM socket 5 times to make sure it is ready */
> > > > +while (pings < 5) {
> > > > +if (virCHMonitorPingVMM(mon) == 0)
> > > > +break;
> > > > +if (pings == 5)
> > > > +goto cleanup;
> > > > +
> > > > +g_usleep(100 * 1000);
> > > > +}
> > >
> > > This is highly undesirable. Is there no way to launch the CH process
> > > such that the socket is guaranteed to be accepting requests by the
> > > time it has forked into the background ? Or is there a way to pass
> > > in a pre-opened FD for the monitor socket ?
> > >
> > > This kind of wait with timeout will cause startup failures due to
> > > timeout under load.
> >
> > Currently the cloud-hypervisor process doesn't fork into the
> > background so that was initially what I was thinking to add to
> > cloud-hypervsior. Would tracking the pid of the cloud-hypervsior
> > running in the background be required at that point (assuming the
> > socket path to control the daemon would be known and working given
> > virCommandRun returns successfully)?
>
> It doesn't especially matter whether cloud-hypervsior forks into
> the background itself, or whether libvirt forks it into the
> background when launching it.
>
> The important thing is simply to ensure that whichever startup
> approach is used can be implemented in a race-free manner without
> needing busy-waits or timeouts.
>
> If cloud-hypervisor forks into the background, then it would
> need to write a pidfile so libvirt can determine the pid that
> ended up running. The act of libvirt waiting for the pidfile
> to be written is potentially racy though, so that might not be
> be best.
>
> Based on what we learnt from QEMU, I think being able to pass
> in a pre-created UNIX listener socket is the best. That lets
> libvirt immedately issue a connect() start after forking the
> cloud-hypervisor process. If cloud-hypervisor succesfully
> starts up, then the client socket becomes live and can be used.
> if it fails to startup, then the client socket libvirt has
> will get EOF and thus we'll see the startup failure. This
> avoids any looping/sleeping/timeout.
I think I'm misreading/missing something from the qemu setup but
looking at qemu_monitor.c's qemuMonitorOpenUnix function I see a retry
connect loop based on a timeout when trying to connect to a socket
(with a comment that leads me to believe it is possible to have the
monitor socket file not yet exist). I was expecting to see something
like forking qemu and passing the fd for the socket that was opened
that qemu should use (and that libvirt connects to) but I think I only
see that for the network socket fds that qemu is supposed to use. If
you could point me in the right direction here I'd appreciate it.
Re: [PATCH v2 0/2] block: deprecate the sheepdog driver
On Tue, Sep 22, 2020 at 1:43 PM Daniel P. Berrangé wrote: > > On Tue, Sep 22, 2020 at 01:09:06PM -0400, Neal Gompa wrote: > > On Tue, Sep 22, 2020 at 12:16 PM Daniel P. Berrangé > > wrote: > > > > > > 2 years back I proposed dropping the sheepdog mailing list from the > > > MAINTAINERS file, but somehow the patch never got picked up: > > > > > > https://lists.gnu.org/archive/html/qemu-block/2018-03/msg01048.html > > > > > > So here I am with the same patch again. > > > > > > This time I go further and deprecate the sheepdog driver entirely. > > > See the rationale in the second patch commit message. > > > > > > Daniel P. Berrangé (2): > > > block: drop moderated sheepdog mailing list from MAINTAINERS file > > > block: deprecate the sheepdog block driver > > > > > > MAINTAINERS| 1 - > > > block/sheepdog.c | 15 +++ > > > configure | 5 +++-- > > > docs/system/deprecated.rst | 9 + > > > 4 files changed, 27 insertions(+), 3 deletions(-) > > > > > > -- > > > 2.26.2 > > > > > > > > > > I don't know of anyone shipping this other than Fedora, and I > > certainly don't use it there. > > > > Upstream looks like it's unmaintained now, with no commits in over two > > years: https://github.com/sheepdog/sheepdog/commits/master > > > > Can we also get a corresponding change to eliminate this from libvirt? > > This is only deprecation in QEMU, the feature still exists and is > intended to be as fully functional as in previous releases. > > Assuming QEMU actually deletes the feature at end of the deprecation > cycle, then libvirt would look at removing its own support. > Does that stop us from deprecating it in libvirt though? -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [libvirt PATCH 1/9] Jailhouse driver: first commit with skeleton code
On Thu, Sep 17, 2020 at 04:23:51PM +0100, Daniel P. Berrangé wrote:
> From: Prakhar Bansal
>
> ---
> include/libvirt/virterror.h | 2 +-
> libvirt.spec.in | 7 +
> m4/virt-driver-jailhouse.m4 | 42 +
> meson.build | 4 +
> meson_options.txt| 1 +
> src/conf/domain_conf.c | 1 +
> src/conf/domain_conf.h | 1 +
> src/jailhouse/Makefile.inc.am| 21 +++
> src/jailhouse/jailhouse_driver.c | 219 +++
> src/jailhouse/jailhouse_driver.h | 23 +++
> src/jailhouse/libvirtd_jailhouse.aug | 43 ++
> src/jailhouse/meson.build| 48 ++
> src/libvirt.c| 10 ++
> src/meson.build | 1 +
> src/qemu/qemu_command.c | 1 +
> src/util/virerror.c | 1 +
> 16 files changed, 424 insertions(+), 1 deletion(-)
> create mode 100644 m4/virt-driver-jailhouse.m4
> create mode 100644 src/jailhouse/Makefile.inc.am
> create mode 100644 src/jailhouse/jailhouse_driver.c
> create mode 100644 src/jailhouse/jailhouse_driver.h
> create mode 100644 src/jailhouse/libvirtd_jailhouse.aug
> create mode 100644 src/jailhouse/meson.build
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 0f1c32283d..97f2ac16d8 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -136,7 +136,7 @@ typedef enum {
>
> VIR_FROM_TPM = 70, /* Error from TPM */
> VIR_FROM_BPF = 71, /* Error from BPF code */
> -
> +VIR_FROM_JAILHOUSE = 72,/* Error from Jailhouse driver */
> # ifdef VIR_ENUM_SENTINELS
> VIR_ERR_DOMAIN_LAST
> # endif
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 62b401bd08..ca65063e79 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -17,6 +17,7 @@
> %define with_lxc 0%{!?_without_lxc:1}
> %define with_libxl 0%{!?_without_libxl:1}
> %define with_vbox 0%{!?_without_vbox:1}
> +%define with_jailhouse 0%{!?_without_jailhouse:1}
The RPM spec targets Fedora and RHEL distros and AFAIK, neither of
these ship jailhouse. So we should fix it to have jailhouse disabled
in the RPM builds.
ie just set -Ddriver_jailhouse=disabled in %meson
> diff --git a/src/jailhouse/jailhouse_driver.c
> b/src/jailhouse/jailhouse_driver.c
> new file mode 100644
> index 00..0175ba771b
> --- /dev/null
> +++ b/src/jailhouse/jailhouse_driver.c
> @@ -0,0 +1,219 @@
> +/*
> + * jailhouse_driver.c: Implementation of driver for Jailhouse hypervisor
> + *
> + * Copyright (C) 2020 Prakhar Bansal
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + *
Re: [PATCH v2 0/3] improve pSeries NVDIMM support
On 9/22/20 11:56 AM, Andrea Bolognani wrote: On Tue, 2020-09-22 at 09:29 -0300, Daniel Henrique Barboza wrote: Hi, This is a follow up of [1] after Andrea pushed the revert patch as standalone. Changes from v1: - patch 2 (former 3): moved the auto-align code from virDomainMemoryDefParseXML() to virDomainMemoryDefPostParse() - patch 3 (former 4): updated NEWS.rst based on Andrea's changes already upstream [1] https://www.redhat.com/archives/libvir-list/2020-September/msg00864.html Daniel Henrique Barboza (3): conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefPostParse() NEWS.rst: update NVDIMM changes entry Looks good! Reviewed-by: Andrea Bolognani Can you please confirm your GitLab account is 'danielhb'? I would like to add you to the libvirt organization so that you can start pushing your own patches instead of relying on someone else doing that for you. Yes, 'danielhb' is my Gitlab username as well: https://gitlab.com/danielhb DHB
Re: [PATCH v2 0/2] block: deprecate the sheepdog driver
On Tue, Sep 22, 2020 at 01:09:06PM -0400, Neal Gompa wrote: > On Tue, Sep 22, 2020 at 12:16 PM Daniel P. Berrangé > wrote: > > > > 2 years back I proposed dropping the sheepdog mailing list from the > > MAINTAINERS file, but somehow the patch never got picked up: > > > > https://lists.gnu.org/archive/html/qemu-block/2018-03/msg01048.html > > > > So here I am with the same patch again. > > > > This time I go further and deprecate the sheepdog driver entirely. > > See the rationale in the second patch commit message. > > > > Daniel P. Berrangé (2): > > block: drop moderated sheepdog mailing list from MAINTAINERS file > > block: deprecate the sheepdog block driver > > > > MAINTAINERS| 1 - > > block/sheepdog.c | 15 +++ > > configure | 5 +++-- > > docs/system/deprecated.rst | 9 + > > 4 files changed, 27 insertions(+), 3 deletions(-) > > > > -- > > 2.26.2 > > > > > > I don't know of anyone shipping this other than Fedora, and I > certainly don't use it there. > > Upstream looks like it's unmaintained now, with no commits in over two > years: https://github.com/sheepdog/sheepdog/commits/master > > Can we also get a corresponding change to eliminate this from libvirt? This is only deprecation in QEMU, the feature still exists and is intended to be as fully functional as in previous releases. Assuming QEMU actually deletes the feature at end of the deprecation cycle, then libvirt would look at removing its own support. 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 v2 2/2] block: deprecate the sheepdog block driver
On 22/09/2020 18.16, Daniel P. Berrangé wrote: > This thread from a little over a year ago: > > http://lists.wpkg.org/pipermail/sheepdog/2019-March/thread.html > > states that sheepdog is no longer actively developed. The only mentioned > users are some companies who are said to have it for legacy reasons with > plans to replace it by Ceph. There is talk about cutting out existing > features to turn it into a simple demo of how to write a distributed > block service. There is no evidence of anyone working on that idea: > > https://github.com/sheepdog/sheepdog/commits/master > > No real commits to git since Jan 2018, and before then just some minor > technical debt cleanup.. > > There is essentially no activity on the mailing list aside from > patches to QEMU that get CC'd due to our MAINTAINERS entry. > > Fedora packages for sheepdog failed to build from upstream source > because of the more strict linker that no longer merges duplicate > global symbols. Fedora patches it to add the missing "extern" > annotations and presumably other distros do to, but upstream source > remains broken. > > There is only basic compile testing, no functional testing of the > driver. > > Since there are no build pre-requisites the sheepdog driver is currently > enabled unconditionally. This would result in configure issuing a > deprecation warning by default for all users. Thus the configure default > is changed to disable it, requiring users to pass --enable-sheepdog to > build the driver. > > Signed-off-by: Daniel P. Berrangé > --- > block/sheepdog.c | 15 +++ > configure | 5 +++-- > docs/system/deprecated.rst | 9 + > 3 files changed, 27 insertions(+), 2 deletions(-) Reviewed-by: Thomas Huth
Re: [PATCH v2 1/2] block: drop moderated sheepdog mailing list from MAINTAINERS file
On 22/09/2020 18.16, Daniel P. Berrangé wrote: > The sheepdog mailing list is setup to stop and queue messages from > non-subscribers, pending moderator approval. Unfortunately it seems > that the moderation queue is not actively dealt with. Even when messages > are approved, the sender is never added to the whitelist, so every > future mail from the same sender continues to get stopped for moderation. > > MAINTAINERS entries should be responsive and not unneccessarily block > mails from QEMU contributors, so drop the sheepdog mailing list. > > Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Daniel P. Berrangé > --- > MAINTAINERS | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3d17cad19a..8e8a4fb0a8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2852,7 +2852,6 @@ F: block/rbd.c > Sheepdog > M: Liu Yuan > L: qemu-bl...@nongnu.org > -L: sheep...@lists.wpkg.org > S: Odd Fixes > F: block/sheepdog.c Reviewed-by: Thomas Huth
Re: [PATCH v2 0/2] block: deprecate the sheepdog driver
On Tue, Sep 22, 2020 at 12:16 PM Daniel P. Berrangé wrote: > > 2 years back I proposed dropping the sheepdog mailing list from the > MAINTAINERS file, but somehow the patch never got picked up: > > https://lists.gnu.org/archive/html/qemu-block/2018-03/msg01048.html > > So here I am with the same patch again. > > This time I go further and deprecate the sheepdog driver entirely. > See the rationale in the second patch commit message. > > Daniel P. Berrangé (2): > block: drop moderated sheepdog mailing list from MAINTAINERS file > block: deprecate the sheepdog block driver > > MAINTAINERS| 1 - > block/sheepdog.c | 15 +++ > configure | 5 +++-- > docs/system/deprecated.rst | 9 + > 4 files changed, 27 insertions(+), 3 deletions(-) > > -- > 2.26.2 > > I don't know of anyone shipping this other than Fedora, and I certainly don't use it there. Upstream looks like it's unmaintained now, with no commits in over two years: https://github.com/sheepdog/sheepdog/commits/master Can we also get a corresponding change to eliminate this from libvirt? Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
[PATCH v2 1/2] block: drop moderated sheepdog mailing list from MAINTAINERS file
The sheepdog mailing list is setup to stop and queue messages from non-subscribers, pending moderator approval. Unfortunately it seems that the moderation queue is not actively dealt with. Even when messages are approved, the sender is never added to the whitelist, so every future mail from the same sender continues to get stopped for moderation. MAINTAINERS entries should be responsive and not unneccessarily block mails from QEMU contributors, so drop the sheepdog mailing list. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Daniel P. Berrangé --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 3d17cad19a..8e8a4fb0a8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2852,7 +2852,6 @@ F: block/rbd.c Sheepdog M: Liu Yuan L: qemu-bl...@nongnu.org -L: sheep...@lists.wpkg.org S: Odd Fixes F: block/sheepdog.c -- 2.26.2
[PATCH v2 2/2] block: deprecate the sheepdog block driver
This thread from a little over a year ago:
http://lists.wpkg.org/pipermail/sheepdog/2019-March/thread.html
states that sheepdog is no longer actively developed. The only mentioned
users are some companies who are said to have it for legacy reasons with
plans to replace it by Ceph. There is talk about cutting out existing
features to turn it into a simple demo of how to write a distributed
block service. There is no evidence of anyone working on that idea:
https://github.com/sheepdog/sheepdog/commits/master
No real commits to git since Jan 2018, and before then just some minor
technical debt cleanup..
There is essentially no activity on the mailing list aside from
patches to QEMU that get CC'd due to our MAINTAINERS entry.
Fedora packages for sheepdog failed to build from upstream source
because of the more strict linker that no longer merges duplicate
global symbols. Fedora patches it to add the missing "extern"
annotations and presumably other distros do to, but upstream source
remains broken.
There is only basic compile testing, no functional testing of the
driver.
Since there are no build pre-requisites the sheepdog driver is currently
enabled unconditionally. This would result in configure issuing a
deprecation warning by default for all users. Thus the configure default
is changed to disable it, requiring users to pass --enable-sheepdog to
build the driver.
Signed-off-by: Daniel P. Berrangé
---
block/sheepdog.c | 15 +++
configure | 5 +++--
docs/system/deprecated.rst | 9 +
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index cbbebc1aaf..7f68bd6a1a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -242,6 +242,17 @@ typedef struct SheepdogInode {
*/
#define FNV1A_64_INIT ((uint64_t)0xcbf29ce484222325ULL)
+static void deprecation_warning(void)
+{
+static bool warned = false;
+
+if (!warned) {
+warn_report("the sheepdog block driver is deprecated and will be "
+"removed in a future release");
+warned = true;
+}
+}
+
/*
* 64 bit Fowler/Noll/Vo FNV-1a hash code
*/
@@ -1548,6 +1559,8 @@ static int sd_open(BlockDriverState *bs, QDict *options,
int flags,
char *buf = NULL;
QemuOpts *opts;
+deprecation_warning();
+
s->bs = bs;
s->aio_context = bdrv_get_aio_context(bs);
@@ -2007,6 +2020,8 @@ static int sd_co_create(BlockdevCreateOptions *options,
Error **errp)
assert(options->driver == BLOCKDEV_DRIVER_SHEEPDOG);
+deprecation_warning();
+
s = g_new0(BDRVSheepdogState, 1);
/* Steal SocketAddress from QAPI, set NULL to prevent double free */
diff --git a/configure b/configure
index 7564479008..c6af83f2e6 100755
--- a/configure
+++ b/configure
@@ -533,7 +533,7 @@ vdi="yes"
vvfat="yes"
qed="yes"
parallels="yes"
-sheepdog="yes"
+sheepdog="no"
libxml2=""
debug_mutex="no"
libpmem=""
@@ -1941,7 +1941,7 @@ disabled with --disable-FEATURE, default is enabled if
available:
vvfat vvfat image format support
qed qed image format support
parallels parallels image format support
- sheepdogsheepdog block driver support
+ sheepdogsheepdog block driver support (deprecated)
crypto-afalgLinux AF_ALG crypto backend driver
capstonecapstone disassembler support
debug-mutex mutex debugging support
@@ -7350,6 +7350,7 @@ if test "$parallels" = "yes" ; then
echo "CONFIG_PARALLELS=y" >> $config_host_mak
fi
if test "$sheepdog" = "yes" ; then
+ add_to deprecated_features "sheepdog"
echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
fi
if test "$pty_h" = "yes" ; then
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 0cb8b01424..49b9f4b02e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -405,6 +405,15 @@ The above, converted to the current supported format::
json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}
+``sheepdog`` driver (since 5.2.0)
+^
+
+The ``sheepdog`` block device driver is deprecated. The corresponding upstream
+server project is no longer actively maintained. Users are recommended to
switch
+to an alternative distributed block device driver such as RBD. The
+``qemu-img convert`` command can be used to liberate existing data by moving
+it out of sheepdog volumes into an alternative storage backend.
+
linux-user mode CPUs
--
2.26.2
[PATCH v2 0/2] block: deprecate the sheepdog driver
2 years back I proposed dropping the sheepdog mailing list from the MAINTAINERS file, but somehow the patch never got picked up: https://lists.gnu.org/archive/html/qemu-block/2018-03/msg01048.html So here I am with the same patch again. This time I go further and deprecate the sheepdog driver entirely. See the rationale in the second patch commit message. Daniel P. Berrangé (2): block: drop moderated sheepdog mailing list from MAINTAINERS file block: deprecate the sheepdog block driver MAINTAINERS| 1 - block/sheepdog.c | 15 +++ configure | 5 +++-- docs/system/deprecated.rst | 9 + 4 files changed, 27 insertions(+), 3 deletions(-) -- 2.26.2
Re: [PATCH] libvirt: ensure defresult is used in virConnectAuthCallbackDefault
On Mon, Sep 21, 2020 at 10:01:46PM -0400, Matt Coleman wrote: > A previous change to this function's password handling broke the use of > default values for credential types other than VIR_CRED_PASSPHRASE and > VIR_CRED_NOECHOPROMPT. > > Signed-off-by: Matt Coleman > --- > src/libvirt.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé and will push shortly 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 v2 0/3] improve pSeries NVDIMM support
On Tue, 2020-09-22 at 09:29 -0300, Daniel Henrique Barboza wrote: > Hi, > > This is a follow up of [1] after Andrea pushed the revert > patch as standalone. > > Changes from v1: > - patch 2 (former 3): moved the auto-align code from > virDomainMemoryDefParseXML() to virDomainMemoryDefPostParse() > - patch 3 (former 4): updated NEWS.rst based on Andrea's > changes already upstream > > > [1] https://www.redhat.com/archives/libvir-list/2020-September/msg00864.html > > Daniel Henrique Barboza (3): > conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c > domain_conf.c: auto-align pSeries NVDIMM in > virDomainMemoryDefPostParse() > NEWS.rst: update NVDIMM changes entry Looks good! Reviewed-by: Andrea Bolognani Can you please confirm your GitLab account is 'danielhb'? I would like to add you to the libvirt organization so that you can start pushing your own patches instead of relying on someone else doing that for you. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v2 1/4] bhyve: support parsing fbuf PCI device
On a Tuesday in 2020, Roman Bogorodskiy wrote:
+static int
+bhyveParsePCIFbuf(virDomainDefPtr def,
+ virDomainXMLOptionPtr xmlopt,
+ unsigned caps G_GNUC_UNUSED,
+ unsigned bus,
+ unsigned slot,
+ unsigned function,
+ const char *config)
+{
+/* -s slot,fbuf,wait,vga=on|io|off,rfb=:port,w=width,h=height */
+
+virDomainVideoDefPtr video = NULL;
+virDomainGraphicsDefPtr graphics = NULL;
+char **params = NULL;
+char *param = NULL, *separator = NULL;
+size_t nparams = 0;
+unsigned int i = 0;
Interesting, some of the CI jobs complain that this should be "size_t"
(which I'll fix before merging), and some don't (including my local
environment). Wondering if that's caused by different sed versions or
something else.
Looking at .gitlab-ci.yml:
We have a 'codestyle' job which specifically runs syntax-check:
meson test -C build --suite syntax-check --no-rebuild ||
And it's also run by the centos-7 job (which is special due to an old
git version):
ninja -C build test
The other jobs execute:
ninja -C build dist
Which apparently does not include syntax-check.
A different sed version might be the culprit - we use it to build the
list of different checks in build-aux/meson.build:
rc = run_command(
'sed', '-n',
's/^\\(sc_[a-zA-Z0-9_-]*\\):.*/\\1/p',
meson.current_source_dir() / 'syntax-check.mk',
check: true,
)
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH] tests: bhyve: Fix the broken build after recent 'isa' LPC patches
On Tue, Sep 22, 2020 at 04:07:40PM +0200, Erik Skultety wrote: > .args and .ldargs counterparts of the > bhyvexml2argv-addr-non-isa-controller-on-slot-1 test were missing. > > fixes: 16a2882350a85afdb0574a03117b36daac53750d > > Signed-off-by: Erik Skultety > --- > > Pushed under the build breaker rule. Nevermind, Roman was faster.
[libvirt PATCH] tests: bhyve: Fix the broken build after recent 'isa' LPC patches
.args and .ldargs counterparts of the bhyvexml2argv-addr-non-isa-controller-on-slot-1 test were missing. fixes: 16a2882350a85afdb0574a03117b36daac53750d Signed-off-by: Erik Skultety --- Pushed under the build breaker rule. ...yvexml2argv-addr-non-isa-controller-on-slot-1.args | 11 +++ ...exml2argv-addr-non-isa-controller-on-slot-1.ldargs | 1 + 2 files changed, 12 insertions(+) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args new file mode 100644 index 00..cbbf768d71 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args @@ -0,0 +1,11 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,lpc \ +-s 3:0,ahci,hd:/tmp/freebsd.img \ +-s 1:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs new file mode 100644 index 00..421376db9e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs @@ -0,0 +1 @@ +dummy -- 2.26.2
Re: [libvirt PATCH 1/1] qemu: substitute missing model name for host-passthrough
On Tue, Sep 22, 2020 at 15:29:28 +0200, Tim Wiederhake wrote:
The stuff (before/after) you put into the cover letter should actually
be here. Or a better explanation if you don't like that.
But the cover letter blurb will get lost in time and this commit will
have no explanation/justification.
> Signed-off-by: Tim Wiederhake
> ---
> src/qemu/qemu_driver.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ae715c01d7..7f5cfc1a7f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
> if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, ) < 0)
> goto cleanup;
>
> +if (!cpu->model) {
> +if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
> +cpu->model = g_strdup("host");
> +} else {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
VIR_ERR_INTERNAL_ERROR doesn't seem to be appropriate since it's in user
provided input.
> + _("cpu parameter is missing a model name"));
> +goto cleanup;
> +}
> +}
> ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir,
> cfg->user, cfg->group,
> hvCPU, cpu, failIncompatible);
> --
> 2.26.2
>
Re: [PATCH 2/2] Add unit test for timer validation
Subject is missing that this is for timers on non-x86. On Tue, Sep 22, 2020 at 11:57:40 +, Sebastian Mitterle wrote: > Add minimal coverage for non-x86_64 timer validation > from commit 2f5d8ffebe5d3d00e16a051ed62ce8a703f18e7c > > Signed-off-by: Sebastian Mitterle > --- > .../non-x86_64-timer-error.err | 1 + > .../non-x86_64-timer-error.xml | 18 ++ > tests/qemuxml2argvtest.c | 2 ++ > 3 files changed, 21 insertions(+) > create mode 100644 tests/qemuxml2argvdata/non-x86_64-timer-error.err > create mode 100644 tests/qemuxml2argvdata/non-x86_64-timer-error.xml Reviewed-by: Peter Krempa
[libvirt PATCH 1/1] qemu: substitute missing model name for host-passthrough
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_driver.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..7f5cfc1a7f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, ) < 0)
goto cleanup;
+if (!cpu->model) {
+if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+cpu->model = g_strdup("host");
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cpu parameter is missing a model name"));
+goto cleanup;
+}
+}
ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir,
cfg->user, cfg->group,
hvCPU, cpu, failIncompatible);
--
2.26.2
[libvirt PATCH 0/1] qemu: substitute missing model name for host-passthrough
See also https://www.redhat.com/archives/libvir-list/2020-June/msg01232.html Before: $ uname -m s390x $ cat passthrough-cpu.xml $ virsh hypervisor-cpu-compare passthrough-cpu.xml error: Failed to compare hypervisor CPU with passthrough-cpu.xml error: internal error: unable to execute QEMU command 'query-cpu-model-comp arison': Invalid parameter type for 'modelb.name', expected: string After: $ virsh hypervisor-cpu-compare passthrough-cpu.xml CPU described in passthrough-cpu.xml is identical to the CPU provided by hy pervisor on the host Tim Wiederhake (1): qemu: substitute missing model name for host-passthrough src/qemu/qemu_driver.c | 9 + 1 file changed, 9 insertions(+) -- 2.26.2
Re: [PATCH 1/2] Add error message check in qemuxml2argv tests
On Tue, Sep 22, 2020 at 11:57:39 +, Sebastian Mitterle wrote:
> When an error is expected, the error message will be checked.
> This is expressed by creating an additional ".err" file containing
> the expected error message.
>
> It is added in order to make sure the expected errors
> are not masked by other errors during test execution while
> leveraging the existing framework.
>
> In order to keep it simple, an input file cannot be reused
> anymore to cover several expected error cases configured
> in the test code. An input file can still be reused by creating
> a test case specific symlink.
>
> For consistency, the mock needs to report an error now, too,
> as every failure must have an error; otherwise a test case will
> fail.
>
> Require LC_ALL=C explicitly to make sure error messages are not
> localized for testing.
>
> Lastly, remove trailing blank in error message for domain_addr.c
> virDomainCCWAddressAssign, uncovered by this change.
>
> Signed-off-by: Sebastian Mitterle
> Suggested-by: Peter Krempa
> ---
[...]
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 1bfa164a47..6e77a72f7c 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -1401,7 +1401,7 @@ virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
>
> if (virHashLookup(addrs->defined, addr)) {
> virReportError(VIR_ERR_XML_ERROR,
> - _("The CCW devno '%s' is in use already "),
> + _("The CCW devno '%s' is in use already"),
> addr);
> return -1;
> }
This should have been done in a separate commit.
> diff --git a/tests/qemuxml2argvdata/440fx-ide-address-conflict.err
> b/tests/qemuxml2argvdata/440fx-ide-address-conflict.err
> new file mode 100644
> index 00..36dfc6cd08
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/440fx-ide-address-conflict.err
> @@ -0,0 +1 @@
> +XML error: Attempted double use of PCI Address :00:01.1
[...]
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index e5841bc8e3..17be4bedfc 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
> @@ -23,6 +23,7 @@
> #include "vircommand.h"
> #include "vircrypto.h"
> #include "virmock.h"
> +#include "virlog.h"
> #include "virnetdev.h"
> #include "virnetdevip.h"
> #include "virnetdevtap.h"
> @@ -91,6 +92,8 @@ virNumaNodesetIsAvailable(virBitmapPtr nodeset)
> if (virNumaNodeIsAvailable(bit))
> continue;
>
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + "Mock: no numa node set is available at bit %li.",
> bit);
> return false;
This also seems a good candidate for splitting out.
> }
[...]
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index e93948e3fc..882a6837b0 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
[...]
> @@ -615,7 +616,13 @@ testCompareXMLToArgv(const void *data)
> if (!(vm->def = virDomainDefParseFile(info->infile,
>driver.xmlopt,
>NULL, parseFlags))) {
> -if (flags & FLAG_EXPECT_PARSE_ERROR)
> +err = virGetLastError();
> +if (!err) {
> +VIR_TEST_DEBUG("no error was reported for expected parse error");
Technically at this point the failure is not yet expected, but the error
should have been reported anyways.
IMO just removing the 'expected' word will be okay, but theoretically
this could be sold as another improvement to the testsuited (checking
whether an error is actually reported when failure is returned).
> +goto cleanup;
> +}
> +if (flags & FLAG_EXPECT_PARSE_ERROR &&
> +virTestCompareToFile(err->message, info->errfile) >= 0)
> goto ok;
> goto cleanup;
> }
> @@ -651,7 +658,13 @@ testCompareXMLToArgv(const void *data)
>
> if (!(cmd = testCompareXMLToArgvCreateArgs(, vm, migrateURI, info,
> flags, false))) {
> -if (flags & FLAG_EXPECT_FAILURE)
> +err = virGetLastError();
> +if (!err) {
> +VIR_TEST_DEBUG("no error was reported for expected failure");
Same here.
> +goto cleanup;
> +}
> +if (flags & FLAG_EXPECT_FAILURE &&
> +virTestCompareToFile(err->message, info->errfile) >= 0)
> goto ok;
> goto cleanup;
> }
I'm going through all the reported errors to see whether they make sense
in terms of the context of the test. Problems there will not be a
problem of this series and we can fix them later.
If nobody objects once I get through the test data I'll split out and
push a version with the stuff above addressed.
Reviewed-by: Peter Krempa
Re: [PATCH v2 1/4] bhyve: support parsing fbuf PCI device
Roman Bogorodskiy wrote:
> From: Fabian Freyer
>
> Add a new helper function, bhyveParsePCIFbuf, to parse the bhyve-argv
> parameters for a frame-buffer device to and
> definitions.
>
> For now, only the listen address, port, and vga mode are detected.
> Unsupported parameters are silently skipped.
>
> This involves upgrading the private API to expose the
> virDomainGraphicsDefNew helper function, which is used by
> bhyveParsePCIFbuf.
>
> Signed-off-by: Fabian Freyer
> Signed-off-by: Roman Bogorodskiy
> ---
> src/bhyve/bhyve_parse_command.c | 91 ++-
> src/libvirt_private.syms | 1 +
> .../bhyveargv2xml-vnc-listen.args | 10 ++
> .../bhyveargv2xml-vnc-listen.xml | 22 +
> .../bhyveargv2xml-vnc-vga-io.args | 10 ++
> .../bhyveargv2xml-vnc-vga-io.xml | 22 +
> .../bhyveargv2xml-vnc-vga-off.args| 10 ++
> .../bhyveargv2xml-vnc-vga-off.xml | 23 +
> .../bhyveargv2xml-vnc-vga-on.args | 10 ++
> .../bhyveargv2xml-vnc-vga-on.xml | 23 +
> .../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++
> tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 +
> tests/bhyveargv2xmltest.c | 5 +
> 13 files changed, 258 insertions(+), 1 deletion(-)
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml
>
> diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
> index 50a5e88408..388c565317 100644
> --- a/src/bhyve/bhyve_parse_command.c
> +++ b/src/bhyve/bhyve_parse_command.c
> @@ -4,7 +4,7 @@
> * Copyright (C) 2006-2016 Red Hat, Inc.
> * Copyright (C) 2006 Daniel P. Berrange
> * Copyright (c) 2011 NetApp, Inc.
> - * Copyright (C) 2016 Fabian Freyer
> + * Copyright (C) 2020 Fabian Freyer
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -553,6 +553,93 @@ bhyveParsePCINet(virDomainDefPtr def,
> return -1;
> }
>
> +static int
> +bhyveParsePCIFbuf(virDomainDefPtr def,
> + virDomainXMLOptionPtr xmlopt,
> + unsigned caps G_GNUC_UNUSED,
> + unsigned bus,
> + unsigned slot,
> + unsigned function,
> + const char *config)
> +{
> +/* -s slot,fbuf,wait,vga=on|io|off,rfb=:port,w=width,h=height */
> +
> +virDomainVideoDefPtr video = NULL;
> +virDomainGraphicsDefPtr graphics = NULL;
> +char **params = NULL;
> +char *param = NULL, *separator = NULL;
> +size_t nparams = 0;
> +unsigned int i = 0;
Interesting, some of the CI jobs complain that this should be "size_t"
(which I'll fix before merging), and some don't (including my local
environment). Wondering if that's caused by different sed versions or
something else.
> +
> +if (!(video = virDomainVideoDefNew(xmlopt)))
> +goto cleanup;
> +
> +if (!(graphics = virDomainGraphicsDefNew(xmlopt)))
> +goto cleanup;
> +
> +graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC;
> +video->info.addr.pci.bus = bus;
> +video->info.addr.pci.slot = slot;
> +video->info.addr.pci.function = function;
Roman Bogorodskiy
signature.asc
Description: PGP signature
[libvirt PATCH] tests: Don't advertise VIR_TEST_EXPENSIVE to users
Right now, the logic that takes care of deciding whether expensive
tests should be run or not is not working correctly: more
specifically, it's not possible to use something like
$ VIR_TEST_EXPENSIVE=1 ninja test
to override the default choice, because in meson.build we always
pass an explicit value that overrides whatever is present in the
environment.
We could implement logic to make this work properly, but that
would require some refactoring of our test infrastructure and is
arguably of little value given that running
$ meson build -Dexpensive_tests=enabled
is very fast, so let's just stop telling users about the variable
instead and call it a day.
Signed-off-by: Andrea Bolognani
---
docs/advanced-tests.rst | 8 +---
meson_options.txt | 2 +-
tests/test-lib.sh | 2 +-
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/docs/advanced-tests.rst b/docs/advanced-tests.rst
index 772fe1dd16..f17d8b0031 100644
--- a/docs/advanced-tests.rst
+++ b/docs/advanced-tests.rst
@@ -26,13 +26,7 @@ Some tests are skipped by default in a development
environment,
based on the time they take in comparison to the likelihood
that those tests will turn up problems during incremental
builds. These tests default to being run when building from a
-tarball or with the configure option -Dexpensive_tests=enabled;
-you can also force a one-time toggle of these tests by setting
-VIR_TEST_EXPENSIVE to 0 or 1 at make time, as in:
-
-::
-
- $ VIR_TEST_EXPENSIVE=1 ninja test
+tarball or with the configure option -Dexpensive_tests=enabled.
If you encounter any failing tests, the VIR_TEST_DEBUG
environment variable may provide extra information to debug the
diff --git a/meson_options.txt b/meson_options.txt
index f92c80553c..74de064384 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -3,7 +3,7 @@ option('packager', type: 'string', value: '', description:
'Extra packager name'
option('packager_version', type: 'string', value: '', description: 'Extra
packager version')
option('system', type: 'boolean', value: false, description: 'Set install
paths to system ones')
option('runstatedir', type: 'string', value: '', description: 'State directory
for temporary sockets, pid files, etc')
-option('expensive_tests', type: 'feature', value: 'auto', description: 'set
the default for enabling expensive tests (long timeouts), use
VIR_TEST_EXPENSIVE to override')
+option('expensive_tests', type: 'feature', value: 'auto', description: 'set
the default for enabling expensive tests (long timeouts)')
option('test_coverage', type: 'boolean', value: false, description: 'turn on
code coverage instrumentation')
option('git_werror', type: 'feature', value: 'auto', description: 'use -Werror
if building from GIT')
option('rpath', type: 'feature', value: 'auto', description: 'whether to
include rpath information in installed binaries and libraries')
diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index cc3924c07a..67065a9362 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -200,7 +200,7 @@ test_expensive()
if test "$VIR_TEST_EXPENSIVE" != 1; then
skip_test_ '
This test is very expensive, so it is disabled by default.
-To run it anyway, rerun: VIR_TEST_EXPENSIVE=1 ninja test
+To change the default, configure with: meson -Dexpensive_tests=enabled
'
fi
}
--
2.26.2
[libvirt PATCH] meson: Include value of expensive_tests in summary
It's useful information to have available at a glance.
Signed-off-by: Andrea Bolognani
---
meson.build | 1 +
1 file changed, 1 insertion(+)
diff --git a/meson.build b/meson.build
index 24535a403c..4d42468a51 100644
--- a/meson.build
+++ b/meson.build
@@ -2444,6 +2444,7 @@ win_summary = {
summary(win_summary, section: 'Windows', bool_yn: true)
test_summary = {
+ 'Expensive': use_expensive_tests,
'Coverage': coverage_flags.length() > 0,
}
summary(test_summary, section: 'Test suite', bool_yn: true)
--
2.26.2
Re: [PATCH v2 4/4] bhyve: add VNC password support
On Tue, Sep 22, 2020 at 04:28:51PM +0400, Roman Bogorodskiy wrote:
> From: Fabian Freyer
>
> Support setting a password for the VNC framebuffer using the passwd
> attribute on the element, if the driver has the
> BHYVE_CAP_VNC_PASSWORD capability.
>
> Note that virsh domxml-from-native does not output the password in the
> generated XML, as VIR_DOMAIN_DEF_FORMAT_SECURE is not set when
> formatting the domain definition.
>
> Signed-off-by: Fabian Freyer
> Signed-off-by: Roman Bogorodskiy
> ---
> NEWS.rst | 7 +++
> src/bhyve/bhyve_command.c | 33 +-
> src/bhyve/bhyve_parse_command.c | 5 +++
> .../bhyveargv2xml-vnc-password.args | 10 +
> .../bhyveargv2xml-vnc-password.xml| 22 ++
> tests/bhyveargv2xmltest.c | 3 +-
> .../bhyvexml2argv-vnc-password-comma.xml | 26 +++
> .../bhyvexml2argv-vnc-password.args | 12 +
> .../bhyvexml2argv-vnc-password.ldargs | 1 +
> .../bhyvexml2argv-vnc-password.xml| 26 +++
> tests/bhyvexml2argvtest.c | 8 +++-
> .../bhyvexml2xmlout-vnc-password.xml | 44 +++
> tests/bhyvexml2xmltest.c | 1 +
> 13 files changed, 185 insertions(+), 13 deletions(-)
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.args
> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.xml
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password-comma.xml
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.ldargs
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.xml
> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-password.xml
>
> diff --git a/NEWS.rst b/NEWS.rst
> index bb48f5bd43..c949cb941b 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -25,6 +25,13 @@ v6.8.0 (unreleased)
> Libvirt can now set the framebuffer's "w" and "h" parameters
> using the ``resolution`` element.
>
> + * bhyve: Support VNC password authentication
> +
> +Libvirt can now probe whether the bhyve binary supports
> +VNC password authentication. In case it does, a VNC password
> +can now be passed using the ``passwd`` attribute on
> +the element.
> +
> * **Improvements**
>
>* qemu: Allow migration over UNIX sockets
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index 176a339d5a..1b48438168 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -424,17 +424,6 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
> return -1;
> }
>
> -if (graphics->data.vnc.auth.passwd) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("vnc password auth not supported"));
> -return -1;
> -} else {
> - /* Bhyve doesn't support VNC Auth yet, so print a warning about
> - * unauthenticated VNC sessions */
> - VIR_WARN("%s", _("Security warning: currently VNC auth is not"
> - " supported."));
> -}
> -
> if (glisten->address) {
> escapeAddr = strchr(glisten->address, ':') != NULL;
> if (escapeAddr)
> @@ -468,6 +457,28 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
> return -1;
> }
>
> +if (graphics->data.vnc.auth.passwd) {
> +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_VNC_PASSWORD)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("VNC Passwort authentication not supported "
s/Passwort/Password/
> + "by bhyve"));
> +return -1;
> +}
> +
> +if (strchr(graphics->data.vnc.auth.passwd, ',')) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Password may not contain ',' character"));
> +return -1;
> +}
> +
> +virBufferAsprintf(, ",password=%s",
> graphics->data.vnc.auth.passwd);
> +} else {
> +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_VNC_PASSWORD))
> +VIR_WARN("%s", _("Security warning: VNC auth is not
> supported."));
> +else
> +VIR_WARN("%s", _("Security warning: VNC is used without
> authentication."));
> +}
> +
> if (video->res)
> virBufferAsprintf(, ",w=%d,h=%d", video->res->x, video->res->y);
>
With typo fixed:
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 v2 3/4] bhyve: probe for VNC password capability
On Tue, Sep 22, 2020 at 04:28:50PM +0400, Roman Bogorodskiy wrote: > From: Fabian Freyer > > Introduces the BHYVE_CAP_VNC_PASSWORD capability, which is probed by > parsing the error message from the bhyve command. When it is not > supported, bhyve -s 0,fbuf,password= will return an error message. > > Signed-off-by: Fabian Freyer > Signed-off-by: Roman Bogorodskiy > --- > src/bhyve/bhyve_capabilities.c | 16 +++- > src/bhyve/bhyve_capabilities.h | 1 + > 2 files changed, 16 insertions(+), 1 deletion(-) 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 v2 2/4] bhyve: add support for setting fbuf resolution
On Tue, Sep 22, 2020 at 04:28:49PM +0400, Roman Bogorodskiy wrote: > From: Fabian Freyer > > The resolution of the VNC framebuffer can now be set via the resolution > definition introduced in 5.9.0. > > Also, add "gop" to the list of model types the > sub-element is valid for. > > Signed-off-by: Fabian Freyer > Signed-off-by: Roman Bogorodskiy > --- > NEWS.rst | 5 +++ > docs/formatdomain.rst | 3 +- > src/bhyve/bhyve_command.c | 3 ++ > src/bhyve/bhyve_parse_command.c | 20 > .../bhyveargv2xml-vnc-resolution.args | 10 ++ > .../bhyveargv2xml-vnc-resolution.xml | 24 ++ > tests/bhyveargv2xmltest.c | 1 + > .../bhyvexml2argv-vnc-resolution.args | 10 ++ > .../bhyvexml2argv-vnc-resolution.ldargs | 1 + > .../bhyvexml2argv-vnc-resolution.xml | 20 > tests/bhyvexml2argvtest.c | 1 + > .../bhyvexml2xmlout-vnc-resolution.xml| 31 +++ > tests/bhyvexml2xmltest.c | 1 + > 13 files changed, 129 insertions(+), 1 deletion(-) > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.ldargs > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-resolution.xml 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 v2 1/4] bhyve: support parsing fbuf PCI device
On Tue, Sep 22, 2020 at 04:28:48PM +0400, Roman Bogorodskiy wrote: > From: Fabian Freyer > > Add a new helper function, bhyveParsePCIFbuf, to parse the bhyve-argv > parameters for a frame-buffer device to and > definitions. > > For now, only the listen address, port, and vga mode are detected. > Unsupported parameters are silently skipped. > > This involves upgrading the private API to expose the > virDomainGraphicsDefNew helper function, which is used by > bhyveParsePCIFbuf. > > Signed-off-by: Fabian Freyer > Signed-off-by: Roman Bogorodskiy > --- > src/bhyve/bhyve_parse_command.c | 91 ++- > src/libvirt_private.syms | 1 + > .../bhyveargv2xml-vnc-listen.args | 10 ++ > .../bhyveargv2xml-vnc-listen.xml | 22 + > .../bhyveargv2xml-vnc-vga-io.args | 10 ++ > .../bhyveargv2xml-vnc-vga-io.xml | 22 + > .../bhyveargv2xml-vnc-vga-off.args| 10 ++ > .../bhyveargv2xml-vnc-vga-off.xml | 23 + > .../bhyveargv2xml-vnc-vga-on.args | 10 ++ > .../bhyveargv2xml-vnc-vga-on.xml | 23 + > .../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++ > tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 + > tests/bhyveargv2xmltest.c | 5 + > 13 files changed, 258 insertions(+), 1 deletion(-) > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml 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 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).
[PATCH v2 3/3] NEWS.rst: update NVDIMM changes entry
Signed-off-by: Daniel Henrique Barboza --- NEWS.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS.rst b/NEWS.rst index 685c5e2225..d75c9a7c9e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -40,7 +40,8 @@ v6.8.0 (unreleased) The auto-alignment logic was removed in v6.7.0 in favor of requiring the size provided by the user to be already aligned; however, this had the unintended consequence of breaking some existing guests. v6.8.0 restores -the previous behavior. +the previous behavior with an improvement: it also reflects the auto-aligned +value in the domain XML. * **Bug fixes** -- 2.26.2
[PATCH v2 1/3] conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c
We'll use the auto-alignment function during parse time, in
domain_conf.c. Let's move the function to that file, renaming
it to virDomainNVDimmAlignSizePseries(). This will also make it
clearer that, although QEMU is the only driver that currently
supports it, pSeries NVDIMM restrictions aren't tied to QEMU.
Signed-off-by: Daniel Henrique Barboza
---
src/conf/domain_conf.c | 36 ++
src/conf/domain_conf.h | 3 +++
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain.c | 42 ++--
4 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9289c147fe..ea6a097161 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16866,6 +16866,42 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
return NULL;
}
+int
+virDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem)
+{
+/* For NVDIMMs in ppc64 in we want to align down the guest
+ * visible space, instead of align up, to avoid writing
+ * beyond the end of file by adding a potential 256MiB
+ * to the user specified size.
+ *
+ * The label-size is mandatory for ppc64 as well, meaning that
+ * the guest visible space will be target_size-label_size.
+ *
+ * Finally, target_size must include label_size.
+ *
+ * The above can be summed up as follows:
+ *
+ * target_size = AlignDown(target_size - label_size) + label_size
+ */
+unsigned long long ppc64AlignSize = 256 * 1024;
+unsigned long long guestArea = mem->size - mem->labelsize;
+
+/* Align down guest_area. 256MiB is the minimum size. Error
+ * out if target_size is smaller than 256MiB + label_size,
+ * since aligning it up will cause QEMU errors. */
+if (mem->size < (ppc64AlignSize + mem->labelsize)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("minimum target size for the NVDIMM "
+ "must be 256MB plus the label size"));
+return -1;
+}
+
+guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize;
+mem->size = guestArea + mem->labelsize;
+
+return 0;
+}
+
static virDomainMemoryDefPtr
virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr memdevNode,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4724206828..8f1662aae0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3904,6 +3904,9 @@ bool
virDomainBlockIoTuneInfoEqual(const virDomainBlockIoTuneInfo *a,
const virDomainBlockIoTuneInfo *b);
+int
+virDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem);
+
bool
virHostdevIsSCSIDevice(const virDomainHostdevDef *hostdev)
ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bdbe3431b8..6bca87eede 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -543,6 +543,7 @@ virDomainNetTypeToString;
virDomainNetUpdate;
virDomainNostateReasonTypeFromString;
virDomainNostateReasonTypeToString;
+virDomainNVDimmAlignSizePseries;
virDomainObjAssignDef;
virDomainObjBroadcast;
virDomainObjCheckActive;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9e0d3a15b2..9fa6fac99a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8067,44 +8067,6 @@ qemuDomainGetMemoryModuleSizeAlignment(const
virDomainDef *def,
}
-static int
-qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def,
- virDomainMemoryDefPtr mem)
-{
-/* For NVDIMMs in ppc64 in we want to align down the guest
- * visible space, instead of align up, to avoid writing
- * beyond the end of file by adding a potential 256MiB
- * to the user specified size.
- *
- * The label-size is mandatory for ppc64 as well, meaning that
- * the guest visible space will be target_size-label_size.
- *
- * Finally, target_size must include label_size.
- *
- * The above can be summed up as follows:
- *
- * target_size = AlignDown(target_size - label_size) + label_size
- */
-unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def);
-unsigned long long guestArea = mem->size - mem->labelsize;
-
-/* Align down guest_area. 256MiB is the minimum size. Error
- * out if target_size is smaller than 256MiB + label_size,
- * since aligning it up will cause QEMU errors. */
-if (mem->size < (ppc64AlignSize + mem->labelsize)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("minimum target size for the NVDIMM "
- "must be 256MB plus the label size"));
-return -1;
-}
-
-guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize;
-mem->size = guestArea + mem->labelsize;
-
-return 0;
-}
-
-
int
qemuDomainAlignMemorySizes(virDomainDefPtr def)
{
@@ -8153,7 +8115,7 @@
[PATCH v2 0/3] improve pSeries NVDIMM support
Hi, This is a follow up of [1] after Andrea pushed the revert patch as standalone. Changes from v1: - patch 2 (former 3): moved the auto-align code from virDomainMemoryDefParseXML() to virDomainMemoryDefPostParse() - patch 3 (former 4): updated NEWS.rst based on Andrea's changes already upstream [1] https://www.redhat.com/archives/libvir-list/2020-September/msg00864.html Daniel Henrique Barboza (3): conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefPostParse() NEWS.rst: update NVDIMM changes entry NEWS.rst | 3 +- src/conf/domain_conf.c| 59 ++- src/conf/domain_conf.h| 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 42 + .../memory-hotplug-nvdimm-ppc64.xml | 2 +- 6 files changed, 67 insertions(+), 43 deletions(-) -- 2.26.2
[PATCH v2 2/3] domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefPostParse()
The alignment for the pSeries NVDIMM does not depend on runtime
constraints. This means that it can be done in device parse
time, instead of runtime, allowing the domain XML to reflect
what the auto-alignment would do when the domain starts.
This brings consistency between the NVDIMM size reported by the
domain XML and what the guest sees, without impacting existing
guests that are using an unaligned size - they'll work as usual,
but the domain XML will be updated with the actual size of the
NVDIMM.
Signed-off-by: Daniel Henrique Barboza
---
Note: the 'hypervisor agnostic' comment mentioned below is true for
all ppc64 DIMMs, not just NVDIMMs. In theory it is possible to
move a lot of pSeries memory handling code from QEMU to this
new function. Given that the only practical gain is extra nerd
points (ppc64 is supported only by QEMU), for now let's settle just
for NVDIMMs.
src/conf/domain_conf.c| 23 ++-
.../memory-hotplug-nvdimm-ppc64.xml | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ea6a097161..22c6ba3b0d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5396,6 +5396,24 @@ virDomainVsockDefPostParse(virDomainVsockDefPtr vsock)
}
+static int
+virDomainMemoryDefPostParse(virDomainMemoryDefPtr mem,
+const virDomainDef *def)
+{
+/* Although only the QEMU driver implements PPC64 support, this
+ * code is related to the platform specification (PAPR), i.e. it
+ * is hypervisor agnostic, and any future PPC64 hypervisor driver
+ * will have the same restriction.
+ */
+if (ARCH_IS_PPC64(def->os.arch) &&
+mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
+virDomainNVDimmAlignSizePseries(mem) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
const virDomainDef *def,
@@ -5437,6 +5455,10 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr
dev,
ret = virDomainVsockDefPostParse(dev->data.vsock);
break;
+case VIR_DOMAIN_DEVICE_MEMORY:
+ret = virDomainMemoryDefPostParse(dev->data.memory, def);
+break;
+
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -5451,7 +5473,6 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr
dev,
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
-case VIR_DOMAIN_DEVICE_MEMORY:
case VIR_DOMAIN_DEVICE_IOMMU:
case VIR_DOMAIN_DEVICE_AUDIO:
ret = 0;
diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
index ae5a17d3c8..ecb1b83b4a 100644
--- a/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
+++ b/tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
@@ -38,7 +38,7 @@
/tmp/nvdimm
-55
+524416
0
128
--
2.26.2
[PATCH v2 2/4] bhyve: add support for setting fbuf resolution
From: Fabian Freyer
The resolution of the VNC framebuffer can now be set via the resolution
definition introduced in 5.9.0.
Also, add "gop" to the list of model types the
sub-element is valid for.
Signed-off-by: Fabian Freyer
Signed-off-by: Roman Bogorodskiy
---
NEWS.rst | 5 +++
docs/formatdomain.rst | 3 +-
src/bhyve/bhyve_command.c | 3 ++
src/bhyve/bhyve_parse_command.c | 20
.../bhyveargv2xml-vnc-resolution.args | 10 ++
.../bhyveargv2xml-vnc-resolution.xml | 24 ++
tests/bhyveargv2xmltest.c | 1 +
.../bhyvexml2argv-vnc-resolution.args | 10 ++
.../bhyvexml2argv-vnc-resolution.ldargs | 1 +
.../bhyvexml2argv-vnc-resolution.xml | 20
tests/bhyvexml2argvtest.c | 1 +
.../bhyvexml2xmlout-vnc-resolution.xml| 31 +++
tests/bhyvexml2xmltest.c | 1 +
13 files changed, 129 insertions(+), 1 deletion(-)
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.xml
create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-resolution.xml
diff --git a/NEWS.rst b/NEWS.rst
index 685c5e2225..bb48f5bd43 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -20,6 +20,11 @@ v6.8.0 (unreleased)
attribute of the device's element can be used to disable the
filtering and allow all guest writes to the configuration space.
+ * bhyve: Support setting the framebuffer resolution
+
+Libvirt can now set the framebuffer's "w" and "h" parameters
+using the ``resolution`` element.
+
* **Improvements**
* qemu: Allow migration over UNIX sockets
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index d5930a4ac1..888db5ea29 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -5822,7 +5822,8 @@ A video device.
:since:`Since 5.9.0` , the ``model`` element may also have an optional
``resolution`` sub-element. The ``resolution`` element has attributes ``x``
and ``y`` to set the minimum resolution for the video device. This
- sub-element is valid for model types "vga", "qxl", "bochs", and "virtio".
+ sub-element is valid for model types "vga", "qxl", "bochs", "gop",
+ and "virtio".
``acceleration``
Configure if video acceleration should be enabled.
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 4df5baabdf..176a339d5a 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -468,6 +468,9 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
return -1;
}
+if (video->res)
+virBufferAsprintf(, ",w=%d,h=%d", video->res->x, video->res->y);
+
if (video->driver)
virBufferAsprintf(, ",vga=%s",
virDomainVideoVGAConfTypeToString(video->driver->vgaconf));
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 388c565317..c6abdfacf3 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -621,6 +621,26 @@ bhyveParsePCIFbuf(virDomainDefPtr def,
if (virStrToLong_i(param, NULL, 10, >data.vnc.port))
goto error;
}
+
+if (STRPREFIX(param, "w=")) {
+param += strlen("w=");
+
+if (video->res == NULL)
+video->res = g_new0(virDomainVideoResolutionDef, 1);
+
+if (virStrToLong_uip(param, NULL, 10, >res->x))
+goto error;
+}
+
+if (STRPREFIX(param, "h=")) {
+param += strlen("h=");
+
+if (video->res == NULL)
+video->res = g_new0(virDomainVideoResolutionDef, 1);
+
+if (virStrToLong_uip(param, NULL, 10, >res->y))
+goto error;
+}
}
cleanup:
diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args
b/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args
new file mode 100644
index 00..e5e2c0f2e8
--- /dev/null
+++ b/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args
@@ -0,0 +1,10 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-l bootrom,/path/to/test.fd \
+-s 2:0,fbuf,tcp=127.0.0.1:5904,w=1920,h=1080 \
+-s 1,lpc bhyve
diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml
b/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml
new file mode 100644
index 00..f8fa0ed1ce
--- /dev/null
+++ b/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml
@@ -0,0 +1,24 @@
+
+ bhyve
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+
[PATCH v2 4/4] bhyve: add VNC password support
From: Fabian Freyer
Support setting a password for the VNC framebuffer using the passwd
attribute on the element, if the driver has the
BHYVE_CAP_VNC_PASSWORD capability.
Note that virsh domxml-from-native does not output the password in the
generated XML, as VIR_DOMAIN_DEF_FORMAT_SECURE is not set when
formatting the domain definition.
Signed-off-by: Fabian Freyer
Signed-off-by: Roman Bogorodskiy
---
NEWS.rst | 7 +++
src/bhyve/bhyve_command.c | 33 +-
src/bhyve/bhyve_parse_command.c | 5 +++
.../bhyveargv2xml-vnc-password.args | 10 +
.../bhyveargv2xml-vnc-password.xml| 22 ++
tests/bhyveargv2xmltest.c | 3 +-
.../bhyvexml2argv-vnc-password-comma.xml | 26 +++
.../bhyvexml2argv-vnc-password.args | 12 +
.../bhyvexml2argv-vnc-password.ldargs | 1 +
.../bhyvexml2argv-vnc-password.xml| 26 +++
tests/bhyvexml2argvtest.c | 8 +++-
.../bhyvexml2xmlout-vnc-password.xml | 44 +++
tests/bhyvexml2xmltest.c | 1 +
13 files changed, 185 insertions(+), 13 deletions(-)
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password-comma.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.xml
create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-password.xml
diff --git a/NEWS.rst b/NEWS.rst
index bb48f5bd43..c949cb941b 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -25,6 +25,13 @@ v6.8.0 (unreleased)
Libvirt can now set the framebuffer's "w" and "h" parameters
using the ``resolution`` element.
+ * bhyve: Support VNC password authentication
+
+Libvirt can now probe whether the bhyve binary supports
+VNC password authentication. In case it does, a VNC password
+can now be passed using the ``passwd`` attribute on
+the element.
+
* **Improvements**
* qemu: Allow migration over UNIX sockets
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 176a339d5a..1b48438168 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -424,17 +424,6 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
return -1;
}
-if (graphics->data.vnc.auth.passwd) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("vnc password auth not supported"));
-return -1;
-} else {
- /* Bhyve doesn't support VNC Auth yet, so print a warning about
- * unauthenticated VNC sessions */
- VIR_WARN("%s", _("Security warning: currently VNC auth is not"
- " supported."));
-}
-
if (glisten->address) {
escapeAddr = strchr(glisten->address, ':') != NULL;
if (escapeAddr)
@@ -468,6 +457,28 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def,
return -1;
}
+if (graphics->data.vnc.auth.passwd) {
+if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_VNC_PASSWORD)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("VNC Passwort authentication not supported "
+ "by bhyve"));
+return -1;
+}
+
+if (strchr(graphics->data.vnc.auth.passwd, ',')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Password may not contain ',' character"));
+return -1;
+}
+
+virBufferAsprintf(, ",password=%s",
graphics->data.vnc.auth.passwd);
+} else {
+if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_VNC_PASSWORD))
+VIR_WARN("%s", _("Security warning: VNC auth is not supported."));
+else
+VIR_WARN("%s", _("Security warning: VNC is used without
authentication."));
+}
+
if (video->res)
virBufferAsprintf(, ",w=%d,h=%d", video->res->x, video->res->y);
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index c6abdfacf3..05cb8eb7d6 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -641,6 +641,11 @@ bhyveParsePCIFbuf(virDomainDefPtr def,
if (virStrToLong_uip(param, NULL, 10, >res->y))
goto error;
}
+
+if (STRPREFIX(param, "password=")) {
+param += strlen("password=");
+graphics->data.vnc.auth.passwd = g_strdup(param);
+}
}
cleanup:
diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.args
[PATCH v2 0/4] bhyve: framebuffer resolution and VNC password
Changes from v2: No functional changes, only rebase. Fabian Freyer (4): bhyve: support parsing fbuf PCI device bhyve: add support for setting fbuf resolution bhyve: probe for VNC password capability bhyve: add VNC password support NEWS.rst | 12 ++ docs/formatdomain.rst | 3 +- src/bhyve/bhyve_capabilities.c| 16 ++- src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 36 -- src/bhyve/bhyve_parse_command.c | 116 +- src/libvirt_private.syms | 1 + .../bhyveargv2xml-vnc-listen.args | 10 ++ .../bhyveargv2xml-vnc-listen.xml | 22 .../bhyveargv2xml-vnc-password.args | 10 ++ .../bhyveargv2xml-vnc-password.xml| 22 .../bhyveargv2xml-vnc-resolution.args | 10 ++ .../bhyveargv2xml-vnc-resolution.xml | 24 .../bhyveargv2xml-vnc-vga-io.args | 10 ++ .../bhyveargv2xml-vnc-vga-io.xml | 22 .../bhyveargv2xml-vnc-vga-off.args| 10 ++ .../bhyveargv2xml-vnc-vga-off.xml | 23 .../bhyveargv2xml-vnc-vga-on.args | 10 ++ .../bhyveargv2xml-vnc-vga-on.xml | 23 .../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++ tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 tests/bhyveargv2xmltest.c | 9 +- .../bhyvexml2argv-vnc-password-comma.xml | 26 .../bhyvexml2argv-vnc-password.args | 12 ++ .../bhyvexml2argv-vnc-password.ldargs | 1 + .../bhyvexml2argv-vnc-password.xml| 26 .../bhyvexml2argv-vnc-resolution.args | 10 ++ .../bhyvexml2argv-vnc-resolution.ldargs | 1 + .../bhyvexml2argv-vnc-resolution.xml | 20 +++ tests/bhyvexml2argvtest.c | 9 +- .../bhyvexml2xmlout-vnc-password.xml | 44 +++ .../bhyvexml2xmlout-vnc-resolution.xml| 31 + tests/bhyvexml2xmltest.c | 2 + 33 files changed, 588 insertions(+), 16 deletions(-) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password-comma.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-password.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-resolution.xml -- 2.27.0
[PATCH v2 1/4] bhyve: support parsing fbuf PCI device
From: Fabian Freyer
Add a new helper function, bhyveParsePCIFbuf, to parse the bhyve-argv
parameters for a frame-buffer device to and
definitions.
For now, only the listen address, port, and vga mode are detected.
Unsupported parameters are silently skipped.
This involves upgrading the private API to expose the
virDomainGraphicsDefNew helper function, which is used by
bhyveParsePCIFbuf.
Signed-off-by: Fabian Freyer
Signed-off-by: Roman Bogorodskiy
---
src/bhyve/bhyve_parse_command.c | 91 ++-
src/libvirt_private.syms | 1 +
.../bhyveargv2xml-vnc-listen.args | 10 ++
.../bhyveargv2xml-vnc-listen.xml | 22 +
.../bhyveargv2xml-vnc-vga-io.args | 10 ++
.../bhyveargv2xml-vnc-vga-io.xml | 22 +
.../bhyveargv2xml-vnc-vga-off.args| 10 ++
.../bhyveargv2xml-vnc-vga-off.xml | 23 +
.../bhyveargv2xml-vnc-vga-on.args | 10 ++
.../bhyveargv2xml-vnc-vga-on.xml | 23 +
.../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++
tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 +
tests/bhyveargv2xmltest.c | 5 +
13 files changed, 258 insertions(+), 1 deletion(-)
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args
create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 50a5e88408..388c565317 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -4,7 +4,7 @@
* Copyright (C) 2006-2016 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (c) 2011 NetApp, Inc.
- * Copyright (C) 2016 Fabian Freyer
+ * Copyright (C) 2020 Fabian Freyer
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -553,6 +553,93 @@ bhyveParsePCINet(virDomainDefPtr def,
return -1;
}
+static int
+bhyveParsePCIFbuf(virDomainDefPtr def,
+ virDomainXMLOptionPtr xmlopt,
+ unsigned caps G_GNUC_UNUSED,
+ unsigned bus,
+ unsigned slot,
+ unsigned function,
+ const char *config)
+{
+/* -s slot,fbuf,wait,vga=on|io|off,rfb=:port,w=width,h=height */
+
+virDomainVideoDefPtr video = NULL;
+virDomainGraphicsDefPtr graphics = NULL;
+char **params = NULL;
+char *param = NULL, *separator = NULL;
+size_t nparams = 0;
+unsigned int i = 0;
+
+if (!(video = virDomainVideoDefNew(xmlopt)))
+goto cleanup;
+
+if (!(graphics = virDomainGraphicsDefNew(xmlopt)))
+goto cleanup;
+
+graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC;
+video->info.addr.pci.bus = bus;
+video->info.addr.pci.slot = slot;
+video->info.addr.pci.function = function;
+
+if (!config)
+goto error;
+
+if (!(params = virStringSplitCount(config, ",", 0, )))
+goto error;
+
+for (i = 0; i < nparams; i++) {
+param = params[i];
+if (!video->driver && VIR_ALLOC(video->driver) < 0)
+goto error;
+
+if (STREQ(param, "vga=on"))
+video->driver->vgaconf = VIR_DOMAIN_VIDEO_VGACONF_ON;
+
+if (STREQ(param, "vga=io"))
+video->driver->vgaconf = VIR_DOMAIN_VIDEO_VGACONF_IO;
+
+if (STREQ(param, "vga=off"))
+video->driver->vgaconf = VIR_DOMAIN_VIDEO_VGACONF_OFF;
+
+if (STRPREFIX(param, "rfb=") || STRPREFIX(param, "tcp=")) {
+/* fortunately, this is the same length as "tcp=" */
+param += strlen("rfb=");
+
+if (!(separator = strchr(param, ':')))
+goto error;
+
+*separator = '\0';
+
+if (separator != param)
+virDomainGraphicsListenAppendAddress(graphics, param);
+else
+/* Default to 127.0.0.1, just like bhyve does */
+virDomainGraphicsListenAppendAddress(graphics, "127.0.0.1");
+
+param = ++separator;
+if (virStrToLong_i(param, NULL, 10, >data.vnc.port))
+goto error;
+}
+}
+
+ cleanup:
+if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0)
+goto error;
+
+if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0)
+
[PATCH v2 3/4] bhyve: probe for VNC password capability
From: Fabian Freyer
Introduces the BHYVE_CAP_VNC_PASSWORD capability, which is probed by
parsing the error message from the bhyve command. When it is not
supported, bhyve -s 0,fbuf,password= will return an error message.
Signed-off-by: Fabian Freyer
Signed-off-by: Roman Bogorodskiy
---
src/bhyve/bhyve_capabilities.c | 16 +++-
src/bhyve/bhyve_capabilities.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
index 36f3985335..523a31e287 100644
--- a/src/bhyve/bhyve_capabilities.c
+++ b/src/bhyve/bhyve_capabilities.c
@@ -3,7 +3,7 @@
*
* Copyright (C) 2014 Roman Bogorodskiy
* Copyright (C) 2014 Semihalf
- * Copyright (C) 2016 Fabian Freyer
+ * Copyright (C) 2020 Fabian Freyer
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -334,6 +334,17 @@ bhyveProbeCapsSoundHda(unsigned int *caps, char *binary)
}
+static int
+bhyveProbeCapsVNCPassword(unsigned int *caps, char *binary)
+{
+return bhyveProbeCapsDeviceHelper(caps, binary,
+ "-s",
+ "0,fbuf,password=",
+ "Invalid fbuf emulation \"password\"",
+ BHYVE_CAP_VNC_PASSWORD);
+}
+
+
int
virBhyveProbeCaps(unsigned int *caps)
{
@@ -365,6 +376,9 @@ virBhyveProbeCaps(unsigned int *caps)
if ((ret = bhyveProbeCapsSoundHda(caps, binary)))
goto out;
+if ((ret = bhyveProbeCapsVNCPassword(caps, binary)))
+goto out;
+
out:
VIR_FREE(binary);
return ret;
diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h
index 1ac9ff4283..b2a16b0189 100644
--- a/src/bhyve/bhyve_capabilities.h
+++ b/src/bhyve/bhyve_capabilities.h
@@ -50,6 +50,7 @@ typedef enum {
BHYVE_CAP_XHCI = 1 << 5,
BHYVE_CAP_CPUTOPOLOGY = 1 << 6,
BHYVE_CAP_SOUND_HDA = 1 << 7,
+BHYVE_CAP_VNC_PASSWORD = 1 << 8,
} virBhyveCapsFlags;
int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps);
--
2.27.0
[PATCH] bhyve: add missing test files
Fixes: 4277e61e22b7532dc476c44a356081053da470f8 Signed-off-by: Roman Bogorodskiy --- Pushed as build breaker & trivial. ...yvexml2argv-addr-non-isa-controller-on-slot-1.args | 11 +++ ...exml2argv-addr-non-isa-controller-on-slot-1.ldargs | 1 + 2 files changed, 12 insertions(+) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args new file mode 100644 index 00..cbbf768d71 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args @@ -0,0 +1,11 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,lpc \ +-s 3:0,ahci,hd:/tmp/freebsd.img \ +-s 1:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs new file mode 100644 index 00..421376db9e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs @@ -0,0 +1 @@ +dummy -- 2.27.0
[PATCH 2/2] Add unit test for timer validation
Add minimal coverage for non-x86_64 timer validation
from commit 2f5d8ffebe5d3d00e16a051ed62ce8a703f18e7c
Signed-off-by: Sebastian Mitterle
---
.../non-x86_64-timer-error.err | 1 +
.../non-x86_64-timer-error.xml | 18 ++
tests/qemuxml2argvtest.c | 2 ++
3 files changed, 21 insertions(+)
create mode 100644 tests/qemuxml2argvdata/non-x86_64-timer-error.err
create mode 100644 tests/qemuxml2argvdata/non-x86_64-timer-error.xml
diff --git a/tests/qemuxml2argvdata/non-x86_64-timer-error.err
b/tests/qemuxml2argvdata/non-x86_64-timer-error.err
new file mode 100644
index 00..f46673eb62
--- /dev/null
+++ b/tests/qemuxml2argvdata/non-x86_64-timer-error.err
@@ -0,0 +1 @@
+unsupported configuration: Configuring the 'tsc' timer is not supported for
virtType=kvm arch=s390x machine=s390-ccw guests
diff --git a/tests/qemuxml2argvdata/non-x86_64-timer-error.xml
b/tests/qemuxml2argvdata/non-x86_64-timer-error.xml
new file mode 100644
index 00..11559bb52b
--- /dev/null
+++ b/tests/qemuxml2argvdata/non-x86_64-timer-error.xml
@@ -0,0 +1,18 @@
+
+ test
+ 9aa4b45c-b9dd-45ef-91fe-862b27b4231f
+ 262144
+ 262144
+
+hvm
+
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-s390x
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 882a6837b0..b8311351a7 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1142,6 +1142,8 @@ mymain(void)
QEMU_CAPS_DEVICE_ZPCI,
QEMU_CAPS_CCW,
QEMU_CAPS_VIRTIO_S390);
+DO_TEST_PARSE_ERROR("non-x86_64-timer-error",
+QEMU_CAPS_VIRTIO_S390);
DO_TEST("disk-order", QEMU_CAPS_VIRTIO_BLK_SCSI);
DO_TEST("disk-virtio-queues",
QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES);
--
2.26.2
[PATCH 1/2] Add error message check in qemuxml2argv tests
When an error is expected, the error message will be checked. This is expressed by creating an additional ".err" file containing the expected error message. It is added in order to make sure the expected errors are not masked by other errors during test execution while leveraging the existing framework. In order to keep it simple, an input file cannot be reused anymore to cover several expected error cases configured in the test code. An input file can still be reused by creating a test case specific symlink. For consistency, the mock needs to report an error now, too, as every failure must have an error; otherwise a test case will fail. Require LC_ALL=C explicitly to make sure error messages are not localized for testing. Lastly, remove trailing blank in error message for domain_addr.c virDomainCCWAddressAssign, uncovered by this change. Signed-off-by: Sebastian Mitterle Suggested-by: Peter Krempa --- src/conf/domain_addr.c| 2 +- .../440fx-ide-address-conflict.err| 1 + tests/qemuxml2argvdata/440fx-wrong-root.err | 1 + .../qemuxml2argvdata/aarch64-acpi-nouefi.err | 1 + ...4-features-sve-disabled.aarch64-latest.err | 1 + .../aarch64-features-sve.aarch64-4.0.0.err| 1 + .../aarch64-features-wrong.aarch64-latest.err | 1 + tests/qemuxml2argvdata/aarch64-gic-host.err | 1 + .../qemuxml2argvdata/aarch64-gic-invalid.err | 1 + .../qemuxml2argvdata/aarch64-gic-not-arm.err | 1 + .../qemuxml2argvdata/aarch64-gic-not-virt.err | 1 + tests/qemuxml2argvdata/aarch64-gic-v3.err | 1 + .../qemuxml2argvdata/aarch64-kvm-32-on-64.err | 1 + tests/qemuxml2argvdata/boot-dev+order.err | 1 + .../boot-menu-enable-with-timeout-invalid.err | 1 + .../boot-menu-enable-with-timeout.err | 1 + .../chardev-reconnect-generated-path.err | 1 + .../chardev-reconnect-invalid-timeout.err | 1 + .../qemuxml2argvdata/cpu-cache-emulate-l2.err | 1 + .../cpu-cache-passthrough-l3.err | 1 + .../cpu-cache-passthrough3.err| 1 + .../cpu-host-model-nofallback.err | 1 + .../cpu-hotplug-granularity.err | 1 + tests/qemuxml2argvdata/cpu-nofallback.err | 1 + tests/qemuxml2argvdata/cpu-numa-disjoint.err | 1 + .../qemuxml2argvdata/cpu-numa-memshared-1.err | 1 + .../qemuxml2argvdata/cpu-numa-memshared-1.xml | 1 + tests/qemuxml2argvdata/cpu-numa-memshared.err | 1 + tests/qemuxml2argvdata/cpu-numa3.err | 1 + .../cpu-qemu-host-passthrough.err | 1 + tests/qemuxml2argvdata/cpu-s390-features.err | 1 + .../cputune-iothreadsched-toomuch.err | 1 + .../cputune-vcpusched-overlap.err | 1 + .../default-video-type-x86_64-caps-test-0.err | 1 + .../disk-address-conflict.err | 1 + ...hing-partition-nosupport.x86_64-latest.err | 1 + .../disk-device-lun-type-invalid.err | 1 + .../disk-fdc-incompatible-address.err | 1 + .../qemuxml2argvdata/disk-floppy-pseries.err | 1 + tests/qemuxml2argvdata/disk-fmt-cow.err | 1 + tests/qemuxml2argvdata/disk-fmt-dir.err | 1 + tests/qemuxml2argvdata/disk-fmt-iso.err | 1 + .../disk-hostdev-scsi-address-conflict.err| 1 + .../disk-ide-incompatible-address.err | 1 + ...-network-iscsi-auth-secrettype-invalid.err | 1 + ...sk-network-iscsi-auth-wrong-secrettype.err | 1 + .../disk-network-rbd-no-colon.err | 1 + .../disk-network-source-auth-both.err | 1 + tests/qemuxml2argvdata/disk-same-targets.err | 1 + .../disk-sata-incompatible-address.err| 1 + .../disk-scsi-disk-vpd-build-error.err| 1 + .../disk-scsi-incompatible-address.err| 1 + tests/qemuxml2argvdata/disk-shared-qcow.err | 1 + tests/qemuxml2argvdata/disk-usb-nosupport.err | 1 + tests/qemuxml2argvdata/disk-usb-pci.err | 1 + ...raphics-sdl-egl-headless.x86_64-latest.err | 1 + ...ice-invalid-egl-headless.x86_64-latest.err | 1 + .../hostdev-mdev-display-missing-graphics.err | 1 + .../hostdev-mdev-invalid-target-address.err | 1 + .../hostdev-mdev-src-address-invalid.err | 1 + ...vhost-scsi-pci-boot-fail.x86_64-latest.err | 1 + ...ys-mdev-vfio-ap-boot-fail.s390x-latest.err | 1 + ...subsys-mdev-vfio-ccw-duplicate-address.err | 1 + ...v-subsys-mdev-vfio-ccw-invalid-address.err | 1 + .../hostdev-subsys-mdev-vfio-ccw.err | 1 + .../hostdev-vfio-zpci-autogenerate-fids.err | 1 + .../hostdev-vfio-zpci-duplicate.err | 1 + ...ostdev-vfio-zpci-invalid-uid-valid-fid.err | 1 + .../hostdev-vfio-zpci-set-zero.err| 1 + .../hostdev-vfio-zpci-uid-set-zero.err| 1 + .../hostdev-vfio-zpci-wrong-arch.err | 1 + tests/qemuxml2argvdata/hostdev-vfio-zpci.err | 1 + .../hostdevs-drive-address-conflict.err | 1 + .../hugepages-default-1G-nodeset-2M.err | 1 + .../hugepages-memaccess-invalid.err | 1 +
[PATCH 0/2] Add unit test for timer validation
In v1, the minimal test case is moved to the existing test suite qemuxml2argv. Support for error message checking is added. The error message expectations for existing test cases have not been validated and reflect the current state. Localization suppression was tested manually by running the test suite executable with a different locale that should have made one of the tests fail if it had been localized. Requiring the an expected failure to have an error was tested manually by using the original qemuxml2argvmock that didn't create an error message. Sebastian Mitterle (2): Add error message check in qemuxml2argv tests Add unit test for timer validation src/conf/domain_addr.c| 2 +- .../440fx-ide-address-conflict.err| 1 + tests/qemuxml2argvdata/440fx-wrong-root.err | 1 + .../qemuxml2argvdata/aarch64-acpi-nouefi.err | 1 + ...4-features-sve-disabled.aarch64-latest.err | 1 + .../aarch64-features-sve.aarch64-4.0.0.err| 1 + .../aarch64-features-wrong.aarch64-latest.err | 1 + tests/qemuxml2argvdata/aarch64-gic-host.err | 1 + .../qemuxml2argvdata/aarch64-gic-invalid.err | 1 + .../qemuxml2argvdata/aarch64-gic-not-arm.err | 1 + .../qemuxml2argvdata/aarch64-gic-not-virt.err | 1 + tests/qemuxml2argvdata/aarch64-gic-v3.err | 1 + .../qemuxml2argvdata/aarch64-kvm-32-on-64.err | 1 + tests/qemuxml2argvdata/boot-dev+order.err | 1 + .../boot-menu-enable-with-timeout-invalid.err | 1 + .../boot-menu-enable-with-timeout.err | 1 + .../chardev-reconnect-generated-path.err | 1 + .../chardev-reconnect-invalid-timeout.err | 1 + .../qemuxml2argvdata/cpu-cache-emulate-l2.err | 1 + .../cpu-cache-passthrough-l3.err | 1 + .../cpu-cache-passthrough3.err| 1 + .../cpu-host-model-nofallback.err | 1 + .../cpu-hotplug-granularity.err | 1 + tests/qemuxml2argvdata/cpu-nofallback.err | 1 + tests/qemuxml2argvdata/cpu-numa-disjoint.err | 1 + .../qemuxml2argvdata/cpu-numa-memshared-1.err | 1 + .../qemuxml2argvdata/cpu-numa-memshared-1.xml | 1 + tests/qemuxml2argvdata/cpu-numa-memshared.err | 1 + tests/qemuxml2argvdata/cpu-numa3.err | 1 + .../cpu-qemu-host-passthrough.err | 1 + tests/qemuxml2argvdata/cpu-s390-features.err | 1 + .../cputune-iothreadsched-toomuch.err | 1 + .../cputune-vcpusched-overlap.err | 1 + .../default-video-type-x86_64-caps-test-0.err | 1 + .../disk-address-conflict.err | 1 + ...hing-partition-nosupport.x86_64-latest.err | 1 + .../disk-device-lun-type-invalid.err | 1 + .../disk-fdc-incompatible-address.err | 1 + .../qemuxml2argvdata/disk-floppy-pseries.err | 1 + tests/qemuxml2argvdata/disk-fmt-cow.err | 1 + tests/qemuxml2argvdata/disk-fmt-dir.err | 1 + tests/qemuxml2argvdata/disk-fmt-iso.err | 1 + .../disk-hostdev-scsi-address-conflict.err| 1 + .../disk-ide-incompatible-address.err | 1 + ...-network-iscsi-auth-secrettype-invalid.err | 1 + ...sk-network-iscsi-auth-wrong-secrettype.err | 1 + .../disk-network-rbd-no-colon.err | 1 + .../disk-network-source-auth-both.err | 1 + tests/qemuxml2argvdata/disk-same-targets.err | 1 + .../disk-sata-incompatible-address.err| 1 + .../disk-scsi-disk-vpd-build-error.err| 1 + .../disk-scsi-incompatible-address.err| 1 + tests/qemuxml2argvdata/disk-shared-qcow.err | 1 + tests/qemuxml2argvdata/disk-usb-nosupport.err | 1 + tests/qemuxml2argvdata/disk-usb-pci.err | 1 + ...raphics-sdl-egl-headless.x86_64-latest.err | 1 + ...ice-invalid-egl-headless.x86_64-latest.err | 1 + .../hostdev-mdev-display-missing-graphics.err | 1 + .../hostdev-mdev-invalid-target-address.err | 1 + .../hostdev-mdev-src-address-invalid.err | 1 + ...vhost-scsi-pci-boot-fail.x86_64-latest.err | 1 + ...ys-mdev-vfio-ap-boot-fail.s390x-latest.err | 1 + ...subsys-mdev-vfio-ccw-duplicate-address.err | 1 + ...v-subsys-mdev-vfio-ccw-invalid-address.err | 1 + .../hostdev-subsys-mdev-vfio-ccw.err | 1 + .../hostdev-vfio-zpci-autogenerate-fids.err | 1 + .../hostdev-vfio-zpci-duplicate.err | 1 + ...ostdev-vfio-zpci-invalid-uid-valid-fid.err | 1 + .../hostdev-vfio-zpci-set-zero.err| 1 + .../hostdev-vfio-zpci-uid-set-zero.err| 1 + .../hostdev-vfio-zpci-wrong-arch.err | 1 + tests/qemuxml2argvdata/hostdev-vfio-zpci.err | 1 + .../hostdevs-drive-address-conflict.err | 1 + .../hugepages-default-1G-nodeset-2M.err | 1 + .../hugepages-memaccess-invalid.err | 1 + .../qemuxml2argvdata/hugepages-memaccess3.err | 1 + .../hugepages-nodeset-nonexist.err| 1 + .../hugepages-numa-nodeset-nonexist.err | 1 + ...ntel-iommu-wrong-machine.x86_64-latest.err | 1 + tests/qemuxml2argvdata/iothreads-nocap.err| 1 +
Re: [PATCH v6 3/3] bhyve: soften requirements for slot 1
Daniel P. Berrangé wrote: > On Sun, Sep 20, 2020 at 07:21:15PM +0400, Roman Bogorodskiy wrote: > > Currently, slot 1 is only allowed to be used by the LPC device. > > Relax this requirement and allow to use slot 1 if it was explicitly > > specified by the user for any other device type. In this case the LPC > > device will have the next available address. > > > > If slot 1 was not used by the user, it'll be reserved for the LPC > > device, even if it is not configured to make address assignment > > consistent in case the LPC device becomes necessary (e.g. the user > > adds a console or a video device which require LPC). > > > > Signed-off-by: Roman Bogorodskiy > > --- > > po/POTFILES.in| 1 - > > src/bhyve/bhyve_device.c | 50 +-- > > ...argv-addr-non-isa-controller-on-slot-1.xml | 1 + > > tests/bhyvexml2argvtest.c | 2 +- > > 4 files changed, 25 insertions(+), 29 deletions(-) > > We're missing the bhyvexml2argv-addr-non-isa-controller-on-slot-1.args > file from the commit, so this breaks CI. I presume you forgot to commit > it from your local checkout. Indeed, I'll add that shortly. Sorry for the breakage. > > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH v6 3/3] bhyve: soften requirements for slot 1
On Sun, Sep 20, 2020 at 07:21:15PM +0400, Roman Bogorodskiy wrote: > Currently, slot 1 is only allowed to be used by the LPC device. > Relax this requirement and allow to use slot 1 if it was explicitly > specified by the user for any other device type. In this case the LPC > device will have the next available address. > > If slot 1 was not used by the user, it'll be reserved for the LPC > device, even if it is not configured to make address assignment > consistent in case the LPC device becomes necessary (e.g. the user > adds a console or a video device which require LPC). > > Signed-off-by: Roman Bogorodskiy > --- > po/POTFILES.in| 1 - > src/bhyve/bhyve_device.c | 50 +-- > ...argv-addr-non-isa-controller-on-slot-1.xml | 1 + > tests/bhyvexml2argvtest.c | 2 +- > 4 files changed, 25 insertions(+), 29 deletions(-) We're missing the bhyvexml2argv-addr-non-isa-controller-on-slot-1.args file from the commit, so this breaks CI. I presume you forgot to commit it from your local checkout. 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: [libvirt PATCH] news: Document changes to NVDIMM handling on ppc64
On 9/22/20 7:26 AM, Andrea Bolognani wrote: Signed-off-by: Andrea Bolognani --- Reviewed-by: Daniel Henrique Barboza NEWS.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index a2f7ecaf1d..685c5e2225 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -35,6 +35,13 @@ v6.8.0 (unreleased) change we also remove dependency on libdbus and possibly fix all the DBus related libvirtd crashes seen over the time. + * Re-introduce NVDIMM auto-alignment for pSeries Guests + +The auto-alignment logic was removed in v6.7.0 in favor of requiring the +size provided by the user to be already aligned; however, this had the +unintended consequence of breaking some existing guests. v6.8.0 restores +the previous behavior. + * **Bug fixes** * **Removed features**
Re: [PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes
On 9/22/20 7:31 AM, Andrea Bolognani wrote: On Thu, 2020-09-17 at 17:34 -0300, Daniel Henrique Barboza wrote: On 9/17/20 7:48 AM, Andrea Bolognani wrote: A few tweaks to the commit message, though: * keep the order of commits consistent to the one they were merged in, which more specifically means putting 2d93cbdea9d1 at the very top of the list; * lose the commas after the various commit hashes - they break convenient double-click selection in most terminals; * indent the commit subject by two spaces for better readability. Ok! The commit is also missing your S-o-b, and as you know that's a blocker for merging. Ouch. reverting + squashing took its toll Since we're approaching the freeze period and don't want the revert to miss the deadline, resulting in another libvirt release implementing the problematic behavior, I have gone ahead and pushed this single patch after applying all the tweaks I mentioned above and that you had confirmed you were okay with anyway. Roger that. I'll resend the series without this revert patch then (hopefully soon). Thanks, DHB I've also posted an update to the release notes covering the revert: https://www.redhat.com/archives/libvir-list/2020-September/msg01105.html This is the exact same wording I had suggested in [1], which you were also okay with, minus the part about reflecting the updated value in the domain XML, since that's obviously not implemented yet. [1] https://www.redhat.com/archives/libvir-list/2020-September/msg01020.html
Re: [PATCH v1 1/4] qemu: revert latest pSeries NVDIMM design changes
On Thu, 2020-09-17 at 17:34 -0300, Daniel Henrique Barboza wrote: > On 9/17/20 7:48 AM, Andrea Bolognani wrote: > > A few tweaks to the commit message, though: > > > >* keep the order of commits consistent to the one they were merged > > in, which more specifically means putting 2d93cbdea9d1 at the > > very top of the list; > > > >* lose the commas after the various commit hashes - they break > > convenient double-click selection in most terminals; > > > >* indent the commit subject by two spaces for better readability. > > Ok! > > > The commit is also missing your S-o-b, and as you know that's a > > blocker for merging. > > Ouch. reverting + squashing took its toll Since we're approaching the freeze period and don't want the revert to miss the deadline, resulting in another libvirt release implementing the problematic behavior, I have gone ahead and pushed this single patch after applying all the tweaks I mentioned above and that you had confirmed you were okay with anyway. I've also posted an update to the release notes covering the revert: https://www.redhat.com/archives/libvir-list/2020-September/msg01105.html This is the exact same wording I had suggested in [1], which you were also okay with, minus the part about reflecting the updated value in the domain XML, since that's obviously not implemented yet. [1] https://www.redhat.com/archives/libvir-list/2020-September/msg01020.html -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] news: Document changes to NVDIMM handling on ppc64
Signed-off-by: Andrea Bolognani --- NEWS.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index a2f7ecaf1d..685c5e2225 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -35,6 +35,13 @@ v6.8.0 (unreleased) change we also remove dependency on libdbus and possibly fix all the DBus related libvirtd crashes seen over the time. + * Re-introduce NVDIMM auto-alignment for pSeries Guests + +The auto-alignment logic was removed in v6.7.0 in favor of requiring the +size provided by the user to be already aligned; however, this had the +unintended consequence of breaking some existing guests. v6.8.0 restores +the previous behavior. + * **Bug fixes** * **Removed features** -- 2.26.2
Re: [External] Re: [PATCH] util: support PCI passthrough net device stats collection
On 9/22/20 9:48 AM, Laine Stump wrote:
(Please don't Cc individual developers in patch submissions unless
they've specifically asked you to do so)
On 9/21/20 8:25 AM, zhenwei pi wrote:
Collect PCI passthrough net device stats from kernel by netlink
API.
Currently, libvirt can not get PCI passthrough net device stats,
run command:
#virsh domifstat instance --interface=52:54:00:2d:b2:35
error: Failed to get interface stats instance 52:54:00:2d:b2:35
error: internal error: Interface name not provided
The PCI device(usually SR-IOV virtual function device) is detached
while it's used in PCI passthrough mode. And we can not parse this
device from /proc/net/dev any more.
In this patch, libvirt check net device is VF of not firstly, then
query virNetDevVFInterfaceStats(new API).
virNetDevVFInterfaceStats parses VFs info of all PFs, compares MAC
address until the two MAC addresses match.
'#ip -s link show' can get the same result. Instead of parsing the
output result, implement this feature by libnl API.
Notice that this feature deponds on driver of PF.
Test on Mellanox ConnectX-4 Lx, it works well.
Also test on Intel Corporation 82599ES, it works, but only get 0.
(ip-link command get the same result).
Signed-off-by: zhenwei pi
---
meson.build | 4 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 +
src/util/virnetdev.c | 158
+++
src/util/virnetdev.h | 5 ++
5 files changed, 171 insertions(+)
diff --git a/meson.build b/meson.build
index 24535a403c..e17da9e2b9 100644
--- a/meson.build
+++ b/meson.build
@@ -1392,6 +1392,10 @@ if not get_option('virtualport').disabled()
endif
endif
+if cc.has_header_symbol('linux/if_link.h', 'IFLA_VF_STATS')
+ conf.set('WITH_VF_STATS', 1)
+endif
+
(a bit of digression here...)
I understand why you put this chunk in, but I'm fairly certain that it
isn't needed.
In the case of the few other things from if_link.h that have their own
"WITH_BLAH" compile-time flag (e.g. WITH_VIRTUALPORT, which checks for
existence of IFLA_PORT_MAX in if_link.h), they were added in the before
before time, when the feature in question had been recently added to the
kernel, and so there were some supported distro versions that had the
new feature, and some that didn't yet. In order to compile properly on
all supported platforms (though lacking these new-at-the-time features
on some) we had to gate them on the presence of some sentinel in
if_link.h. Those WITH_BLAH flags were then forgotten about, even though
the list of supported platform/versions has changed over the years, and
they will now work on *any* supported Linux platform.
Now that this patch has pointed them out, I think it's time for some
house cleaning. IFLA_PORT_MAX, for example, has been in the upstream
kernel since 2.6.35, and was even backported to the RHEL*6* 2.6.32
kernel. Even router firmware these days has a newer kernel than that, so
it's a safe bet that any system with __linux_ and WITH_LIBNL can enable
all of that code, i.e. we can just get rid of the extra
WITH_VIRTUALPORT. I just added this to my personal todo list; let's see
how long it takes until I actually accomplish it (betting starts now!)
Back to the topic of *this* patch: I looked up IFLA_VF_STATS for the
upstream kernel, and it first appeared in kernel 4.2. RHEL7 (which is
the oldest RHEL now supported by libvirt) is based on kernel 3.10, but
has *a lot* of backports, and so when I checked I found that it also
supports IFLA_VF_STATS. I don't have the details handy for the oldest
version of other Linuxes we support, but I tried eliminating the extra
check for IFLA_VF_STATS, and the full CI suite on gitlab builds without
error (there is an unrelated error in ubuntu-18.04 in libxl, and
different errors in the freebsd, mingw, and macos builds that *are*
caused by different aspects of this patch, but IFLA_VF_STATS does exist
on all Linux platforms included in the libvirt upstream CI testing).
So support for this feature can be gated on "defined(__linux__) &&
WITH_LIBNL (as should just about anything else using netlink)
if host_machine.system() == 'windows'
ole32_dep = cc.find_library('ole32')
oleaut32_dep = cc.find_library('oleaut32')
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bdbe3431b8..bcc40b8d69 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2585,6 +2585,7 @@ virNetDevSetRcvMulti;
virNetDevSetupControl;
virNetDevSysfsFile;
virNetDevValidateConfig;
+virNetDevVFInterfaceStats;
# util/virnetdevbandwidth.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..f554010c40 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10196,6 +10196,9 @@ qemuDomainInterfaceStats(virDomainPtr dom,
if (virDomainNetGetActualType(net) ==
VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
if
Re: [RFC DOCUMENT 00/12] kubevirt-and-kvm: Add documents
Hi Andrea,
On 9/16/20 6:44 PM, Andrea Bolognani wrote:
> Hello there!
>
> Several weeks ago, a group of Red Hatters working on the
> virtualization stack (primarily QEMU and libvirt) started a
> conversation with developers from the KubeVirt project with the goal
> of better understanding and documenting the interactions between the
> two.
>
> Specifically, we were interested in integration pain points, with the
> underlying ideas being that only once those issues are understood it
> becomes possible to look for solutions, and that better communication
> would naturally lead to improvements on both sides.
>
> This series of documents was born out of that conversation. We're
> sharing them with the QEMU and libvirt communities in the hope that
> they can be a valuable resource for understanding how the projects
> they're working on are consumed by higher-level tools, and what
> challenges are encountered in the process.
>
> Note that, while the documents describe a number of potential
> directions for things like development of new components, that's all
> just brainstorming that naturally occurred as we were learning new
> things: the actual design process should, and will, happen on the
> upstream lists.
>
> Right now the documents live in their own little git repository[1],
> but the expectation is that eventually they will find a suitable
> long-term home. The most likely candidate right now is the main
> KubeVirt repository, but if you have other locations in mind please
> do speak up!
>
> I'm also aware of the fact that this delivery mechanism is fairly
> unconventional, but I thought it would be the best way to spark a
> discussion around these topics with the QEMU and libvirt developers.
>
> Last but not least, please keep in mind that the documents are a work
> in progress, and polish has been applied to them unevenly: while the
> information presented is, to the best of our knowledge, all accurate,
> some parts are in a rougher state than others. Improvements will
> hopefully come over time - and if you feel like helping out in making
> that happen, it would certainly be appreciated!
>
> Looking forward to your feedback :)
>
>
> [1] https://gitlab.com/abologna/kubevirt-and-kvm
Thanks a lot for this documentation, I could learn new things,
use cases out of my interest area. Useful as a developer to
better understand how are used the areas I'm coding. This
shorten a bit that gap between developers and users.
What would be more valuable than a developer review/feedback is
having feedback from users and technical writers.
Suggestion: also share it on qemu-disc...@nongnu.org which is
less technical (maybe simply repost the cover and link to the
Wiki).
--
What is not obvious in this cover (and the documents pasted on
the list) is there are schema pictures on the Wiki pages which
are not viewable and appreciable via an email post.
--
I had zero knowledge on Kubernetes. I have been confused by their
use in the introduction...
>From Index:
"The intended audience is people who are familiar with the traditional
virtualization stack (QEMU plus libvirt), and in order to make it
more approachable to them comparisons, are included and little to no
knowledge of KubeVirt or Kubernetes is assumed."
Then in Architecture's {Goals and Components} there is an assumption
Kubernetes is known. Entering in Components, Kubernetes is briefly
but enough explained.
Then KubeVirt is very well explained.
--
Sometimes the "Other topics" category is confusing, it seems out
of the scope of the "better understanding and documenting the
interactions between KubeVirt and KVM" and looks like left over
notes. I.e.:
"Another possibility is to leverage the device-mapper from Linux to
provide features such as snapshots and even like Incremental Backup.
For example, dm-era seems to provide the basic primitives for
bitmap tracking.
This could be part of scenario number 1, or cascaded with other PVs
somewhere else.
Is this already being used? For example, cybozu-go/topolvm is a
CSI LVM Plugin for k8s."
"vhost-user-blk in other CSI backends
Would it make sense for other CSI backends to implement support for
vhost-user-blk?"
"The audience is people who are familiar with the traditional
virtualization stack (QEMU plus libvirt)". Feeling part of the
audience, I have no clue how to answer these questions...
I'd prefer you tell me :)
Maybe renaming the "Other topics" section would help.
"Unanswered questions", "Other possibilities to investigate",...
--
Very good contribution in documentation,
Thanks!
Phil.
Re: [PATCH] udevProcessCSS: fix segfault
On Tue, Sep 22, 2020 at 10:45:09AM +0200, Boris Fiuczynski wrote:
> On 9/22/20 8:26 AM, Erik Skultety wrote:
> > On Mon, Sep 21, 2020 at 07:06:32PM +0200, Marc Hartmayer wrote:
> > > Don't process subchannel devices where `def->driver` is not set. This
> > > fixes the following segfault:
> > >
> > > Thread 21 "nodedev-init" received signal SIGSEGV, Segmentation fault.
> > > [Switching to Thread 0x3ffb08fc910 (LWP 64303)]
> > > (gdb) bt
> > > #0 0x03fffd1272b4 in __strcmp_vx () at /lib64/libc.so.6
> > > #1 0x03ffc260c3a8 in udevProcessCSS (device=0x3ff9018d130,
> > > def=0x3ff90194a90)
> > > #2 0x03ffc260cb78 in udevGetDeviceDetails (device=0x3ff9018d130,
> > > def=0x3ff90194a90)
> > > #3 0x03ffc260d126 in udevAddOneDevice (device=0x3ff9018d130)
> > > #4 0x03ffc260d414 in udevProcessDeviceListEntry
> > > (udev=0x3ffa810d800, list_entry=0x3ff90001990)
> > > #5 0x03ffc260d638 in udevEnumerateDevices (udev=0x3ffa810d800)
> > > #6 0x03ffc260e08e in nodeStateInitializeEnumerate
> > > (opaque=0x3ffa810d800)
> > > #7 0x03fffdaa14b6 in virThreadHelper (data=0x3ffa810df00)
> > > #8 0x03fffc309ed6 in start_thread ()
> > > #9 0x03fffd185e66 in thread_start ()
> > > (gdb) p *def
> > > $2 = {
> > >name = 0x0,
> > >sysfs_path = 0x3ff90198e80 "/sys/devices/css0/0.0.ff40",
> >
> > Okay, this patch fixes the segfault. However, if ^this generated it because
> > the
> > driver name is not set, how do we even get to the resulting device tree as
> > outlined in 05e6cdafa6e0?
> >
> > +- css_0_0_003a
> > |
> > +- ccw_0_0_1a2b
> > |
> > +- scsi_host0
> >
> > What kind of CSS device is the one causing the error? If we skip this CSS
> > device, we don't generate a name for it and don't put it in the list, so I'm
> > quite puzzled on what I missed in the IBM document and thus in the review
> > process.
> >
> > FWIW:
> > Reviewed-by: Erik Skultety
> >
>
> Erik,
> for whatever reason Marcs system does not have the subchannel device driver
> "chsc_subchannel" loaded. Therefore the subchannel is not bound to any
> driver and by the way this would be a filtered device since we want to show
> the users only io_subchannel and vfio_ccw subchannels.
> The tree above shows a subchannel bound to the io_subchannel device driver.
> Maybe it helps you to see a full example of how it looks on my system:
Oh, it would have been a different driver anyway - impossible to spot just from
the address :).
Yeah, the tree dump makes it much clearer, thanks, I pushed the patch.
Erik
Re: [PATCH] udevProcessCSS: fix segfault
On 9/22/20 8:26 AM, Erik Skultety wrote:
On Mon, Sep 21, 2020 at 07:06:32PM +0200, Marc Hartmayer wrote:
Don't process subchannel devices where `def->driver` is not set. This
fixes the following segfault:
Thread 21 "nodedev-init" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x3ffb08fc910 (LWP 64303)]
(gdb) bt
#0 0x03fffd1272b4 in __strcmp_vx () at /lib64/libc.so.6
#1 0x03ffc260c3a8 in udevProcessCSS (device=0x3ff9018d130,
def=0x3ff90194a90)
#2 0x03ffc260cb78 in udevGetDeviceDetails (device=0x3ff9018d130,
def=0x3ff90194a90)
#3 0x03ffc260d126 in udevAddOneDevice (device=0x3ff9018d130)
#4 0x03ffc260d414 in udevProcessDeviceListEntry (udev=0x3ffa810d800,
list_entry=0x3ff90001990)
#5 0x03ffc260d638 in udevEnumerateDevices (udev=0x3ffa810d800)
#6 0x03ffc260e08e in nodeStateInitializeEnumerate (opaque=0x3ffa810d800)
#7 0x03fffdaa14b6 in virThreadHelper (data=0x3ffa810df00)
#8 0x03fffc309ed6 in start_thread ()
#9 0x03fffd185e66 in thread_start ()
(gdb) p *def
$2 = {
name = 0x0,
sysfs_path = 0x3ff90198e80 "/sys/devices/css0/0.0.ff40",
Okay, this patch fixes the segfault. However, if ^this generated it because the
driver name is not set, how do we even get to the resulting device tree as
outlined in 05e6cdafa6e0?
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
What kind of CSS device is the one causing the error? If we skip this CSS
device, we don't generate a name for it and don't put it in the list, so I'm
quite puzzled on what I missed in the IBM document and thus in the review
process.
FWIW:
Reviewed-by: Erik Skultety
Erik,
for whatever reason Marcs system does not have the subchannel device
driver "chsc_subchannel" loaded. Therefore the subchannel is not bound
to any driver and by the way this would be a filtered device since we
want to show the users only io_subchannel and vfio_ccw subchannels.
The tree above shows a subchannel bound to the io_subchannel device driver.
Maybe it helps you to see a full example of how it looks on my system:
# virsh nodedev-list --tree
computer
|
+- css_0_0_000e
| |
| +- ccw_0_0_f500
|
+- css_0_0_000f
| |
| +- ccw_0_0_f501
|
+- css_0_0_0010
| |
| +- ccw_0_0_f502
|
+- css_0_0_0011
| |
| +- ccw_0_0_bd00
|
+- css_0_0_0012
| |
| +- ccw_0_0_bd01
|
+- css_0_0_0013
| |
| +- ccw_0_0_bd02
|
+- css_0_0_002f
| |
| +- ccw_0_0_19c0
| |
| +- scsi_host3
| |
| +- scsi_target3_0_1
| |
| +- scsi_3_0_1_1078935649
| |
| +- block_sda_36005076307ffc5e3614f
| +- scsi_generic_sg0
|
+- css_0_0_0038
| |
| +- ccw_0_0_1900
| |
| +- scsi_host1
| |
| +- scsi_target1_0_0
| | |
| | +- scsi_1_0_0_1075986530
| | | |
| | | +- block_sdd_36005076307ffc5e36222
| | | +- scsi_generic_sg3
| | |
| | +- scsi_1_0_0_1078935649
| | |
| | +- block_sdc_36005076307ffc5e3614f
| | +- scsi_generic_sg2
| |
| +- scsi_target1_0_2
| |
| +- scsi_1_0_2_1075986530
| | |
| | +- block_sdf_36005076307ffc5e36222
| | +- scsi_generic_sg5
| |
| +- scsi_1_0_2_1078935649
| |
| +- block_sde_36005076307ffc5e3614f
| +- scsi_generic_sg4
|
+- css_0_0_003a
| |
| +- ccw_0_0_1940
| |
| +- scsi_host0
| |
| +- scsi_target0_0_1
| | |
| | +- scsi_0_0_1_1075986530
| | | |
| | | +- block_sdi_36005076307ffc5e36222
| | | +- scsi_generic_sg8
| | |
| | +- scsi_0_0_1_1078935649
| | |
| | +- block_sdh_36005076307ffc5e3614f
| | +- scsi_generic_sg7
| |
| +- scsi_target0_0_3
| |
| +- scsi_0_0_3_1075986530
| | |
| | +- block_sdg_36005076307ffc5e36222
| | +- scsi_generic_sg6
| |
| +- scsi_0_0_3_1078935649
| |
| +- block_sdb_36005076307ffc5e3614f
| +- scsi_generic_sg1
|
+- css_0_0_003c
| |
| +- ccw_0_0_1980
| |
| +- scsi_host2
|
+- css_0_0_006b
| |
| +- ccw_0_0_1000
| |
| +- block_dasdg_IBM_75000DHVL1_0001_00
|
+- css_0_0_006c
| |
Re: [RFC] Add basic driver for the Cloud-Hypervisor
On Mon, Sep 21, 2020 at 12:05:48PM -0700, Douglas, William wrote:
> On Tue, Sep 15, 2020 at 5:53 AM Daniel P. Berrangé
> wrote:
> >
> > On Thu, Aug 27, 2020 at 11:24:32AM -0700, William Douglas wrote:
>
>
>
> > > +virCHMonitorPtr
> > > +virCHMonitorNew(virDomainObjPtr vm, const char *socketdir)
> > > +{
> > > +virCHMonitorPtr ret = NULL;
> > > +virCHMonitorPtr mon = NULL;
> > > +virCommandPtr cmd = NULL;
> > > +int pings = 0;
> > > +
> > > +if (virCHMonitorInitialize() < 0)
> > > +return NULL;
> > > +
> > > +if (!(mon = virObjectLockableNew(virCHMonitorClass)))
> > > +return NULL;
> > > +
> > > +mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
> > > vm->def->name);
> > > +
> > > +/* prepare to launch Cloud-Hypervisor socket */
> > > +if (!(cmd = chMonitorBuildSocketCmd(vm, mon->socketpath)))
> > > +goto cleanup;
> > > +
> > > +if (virFileMakePath(socketdir) < 0) {
> > > +virReportSystemError(errno,
> > > + _("Cannot create socket directory '%s'"),
> > > + socketdir);
> > > +goto cleanup;
> > > +}
> > > +
> > > +/* launch Cloud-Hypervisor socket */
> > > +if (virCommandRunAsync(cmd, >pid) < 0)
> > > +goto cleanup;
> > > +
> > > +/* get a curl handle */
> > > +mon->handle = curl_easy_init();
> > > +
> > > +/* try to ping VMM socket 5 times to make sure it is ready */
> > > +while (pings < 5) {
> > > +if (virCHMonitorPingVMM(mon) == 0)
> > > +break;
> > > +if (pings == 5)
> > > +goto cleanup;
> > > +
> > > +g_usleep(100 * 1000);
> > > +}
> >
> > This is highly undesirable. Is there no way to launch the CH process
> > such that the socket is guaranteed to be accepting requests by the
> > time it has forked into the background ? Or is there a way to pass
> > in a pre-opened FD for the monitor socket ?
> >
> > This kind of wait with timeout will cause startup failures due to
> > timeout under load.
>
> Currently the cloud-hypervisor process doesn't fork into the
> background so that was initially what I was thinking to add to
> cloud-hypervsior. Would tracking the pid of the cloud-hypervsior
> running in the background be required at that point (assuming the
> socket path to control the daemon would be known and working given
> virCommandRun returns successfully)?
It doesn't especially matter whether cloud-hypervsior forks into
the background itself, or whether libvirt forks it into the
background when launching it.
The important thing is simply to ensure that whichever startup
approach is used can be implemented in a race-free manner without
needing busy-waits or timeouts.
If cloud-hypervisor forks into the background, then it would
need to write a pidfile so libvirt can determine the pid that
ended up running. The act of libvirt waiting for the pidfile
to be written is potentially racy though, so that might not be
be best.
Based on what we learnt from QEMU, I think being able to pass
in a pre-created UNIX listener socket is the best. That lets
libvirt immedately issue a connect() start after forking the
cloud-hypervisor process. If cloud-hypervisor succesfully
starts up, then the client socket becomes live and can be used.
if it fails to startup, then the client socket libvirt has
will get EOF and thus we'll see the startup failure. This
avoids any looping/sleeping/timeout.
> > What is the security model for the processes ? The libvirt driver
> > here is running as root and spawning CH as root. We don't really
> > want the VM process running as root though. It really needs to
> > be unprivileged from a DAC POV. Obviously that's quite a bit more
> > work for you todo, but it should be opssible to share alot of the
> > security mgmt infrastructure that we already have for QEMU and
> > LXC drivers. It can manage DAC permissions and apply SELinux or
> > AppArmor MAC labelling/profiles.
>
> Currently cloud-hypervisor runs with cap_net_admin permitted and
> effective on it. The implementation will be updated to run under the
> user's session using a non-root driver (with its own daemon as you had
> mentioned earlier).
>
> >
> > The other big question is around device addressing. If using PCI,
> > then PCI address assignment logic is critical to ensure consistent
> > guest ABI.
> >
>
> I'll be adding the device passthrough for the actual driver submission
> but if you would like to see an updated RFC with that added I could do
> so.
No need to do that in an initial patch. It is just something I wanted
to raise as an item needed to make it sustainable for live migration
type uses cases in particular.
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] libvirt: ensure defresult is used in virConnectAuthCallbackDefault
Some additional info (feel free to add it to the commit message): Starting in version 6.1.0, virsh does not use the default username while connecting: $ ./run tools/virsh -c hyperv://example-hyperv-server?transport=http capabilities Enter username for example-hyperv-server [administrator]: Enter 's password for example-hyperv-server: error: failed to connect to the hypervisor error: internal error: Transport error during enumeration: User, password or similar was not accepted (26) $ ./run tools/virsh -c hyperv://example-hyperv-server?transport=http capabilities Enter username for example-hyperv-server [administrator]: administrator Enter administrator's password for example-hyperv-server: … I’ve reproduced this on several versions of Fedora and Ubuntu 20.04. You can reproduce it locally (although with a slightly different error) by replacing example-hyperv-server with 127.0.0.1. -- Matt
Re: [PATCH] udevProcessCSS: fix segfault
On Mon, Sep 21, 2020 at 07:06:32PM +0200, Marc Hartmayer wrote:
> Don't process subchannel devices where `def->driver` is not set. This
> fixes the following segfault:
>
> Thread 21 "nodedev-init" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x3ffb08fc910 (LWP 64303)]
> (gdb) bt
> #0 0x03fffd1272b4 in __strcmp_vx () at /lib64/libc.so.6
> #1 0x03ffc260c3a8 in udevProcessCSS (device=0x3ff9018d130,
> def=0x3ff90194a90)
> #2 0x03ffc260cb78 in udevGetDeviceDetails (device=0x3ff9018d130,
> def=0x3ff90194a90)
> #3 0x03ffc260d126 in udevAddOneDevice (device=0x3ff9018d130)
> #4 0x03ffc260d414 in udevProcessDeviceListEntry (udev=0x3ffa810d800,
> list_entry=0x3ff90001990)
> #5 0x03ffc260d638 in udevEnumerateDevices (udev=0x3ffa810d800)
> #6 0x03ffc260e08e in nodeStateInitializeEnumerate (opaque=0x3ffa810d800)
> #7 0x03fffdaa14b6 in virThreadHelper (data=0x3ffa810df00)
> #8 0x03fffc309ed6 in start_thread ()
> #9 0x03fffd185e66 in thread_start ()
> (gdb) p *def
> $2 = {
> name = 0x0,
> sysfs_path = 0x3ff90198e80 "/sys/devices/css0/0.0.ff40",
Okay, this patch fixes the segfault. However, if ^this generated it because the
driver name is not set, how do we even get to the resulting device tree as
outlined in 05e6cdafa6e0?
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
What kind of CSS device is the one causing the error? If we skip this CSS
device, we don't generate a name for it and don't put it in the list, so I'm
quite puzzled on what I missed in the IBM document and thus in the review
process.
FWIW:
Reviewed-by: Erik Skultety
[PATCH] libvirt: ensure defresult is used in virConnectAuthCallbackDefault
A previous change to this function's password handling broke the use of default values for credential types other than VIR_CRED_PASSPHRASE and VIR_CRED_NOECHOPROMPT. Signed-off-by: Matt Coleman --- src/libvirt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 0748eb2352..63c8bdea9f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -146,7 +146,9 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred, len = strlen(buf); if (len != 0 && buf[len-1] == '\n') buf[len-1] = '\0'; -bufptr = g_strdup(buf); + +if (strlen(buf) > 0) +bufptr = g_strdup(buf); break; case VIR_CRED_PASSPHRASE: -- 2.27.0
