[libvirt] [PATCH] util: allow ignoring SIOCSIFHWADDR when errno is EPERM
by Laine Stump
From: Laine Stump <laine(a)redhat.com>
Commit f4ef3a71 made a variation of virNetDevSetMAC that would return
without logging an error message if errno was set to
EADDRNOTAVAIL. This errno is set by some SRIOV VF drivers (in
particular igbvf) when they fail to set the device's MAC address due
to the PF driver refusing the request. This is useful if we want to
try a different method of setting the VF MAC address before giving up
(Commit 86556e16 actually does this, setting the desired MAC address
to the "admin MAC in the PF, then detaching and reattaching the VF
netdev driver to force a reinit of the MAC address).
During testing of Bug 1442040 t was discovered that the ixgbe driver
returns EPERM in this situation, so this patch changes the exception
case for silent+non-terminal failure to account for this difference.
Completes resolution to: https://bugzilla.redhat.com/1415609 (RHEL 7.4)
https://bugzilla.redhat.com/1442040 (RHEL 7.3.z)
---
src/util/virnetdev.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 170e348..9aa9c9f 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -226,7 +226,8 @@ int virNetDevExists(const char *ifname)
* virNetDevSetMACInternal:
* @ifname: interface name to set MTU for
* @macaddr: MAC address
- * @quiet: true if a failure to set MAC address with errno == EADDRNOTAVAIL
+ * @quiet: true if a failure to set MAC address with
+ * errno == EADDRNOTAVAIL || errno == EPERM
* should be silent (still returns error, but without log)
*
* This function sets the @macaddr for a given interface @ifname.
@@ -258,7 +259,8 @@ virNetDevSetMACInternal(const char *ifname,
if (ioctl(fd, SIOCSIFHWADDR, &ifr) < 0) {
- if (quiet && errno == EADDRNOTAVAIL)
+ if (quiet &&
+ (errno == EADDRNOTAVAIL || errno == EPERM))
goto cleanup;
virReportSystemError(errno,
@@ -305,7 +307,8 @@ virNetDevSetMACInternal(const char *ifname,
ifr.ifr_addr.sa_len = VIR_MAC_BUFLEN;
if (ioctl(s, SIOCSIFLLADDR, &ifr) < 0) {
- if (quiet && errno == EADDRNOTAVAIL)
+ if (quiet &&
+ (errno == EADDRNOTAVAIL || errno == EPERM))
goto cleanup;
virReportSystemError(errno,
@@ -2229,11 +2232,12 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
int retries = 100;
/* if pfDevOrig == NULL, this isn't a VF, so we've failed */
- if (!pfDevOrig || errno != EADDRNOTAVAIL)
+ if (!pfDevOrig ||
+ (errno != EADDRNOTAVAIL && errno != EPERM))
goto cleanup;
/* Otherwise this is a VF, and virNetDevSetMAC failed with
- * EADDRNOTAVAIL, which could be due to the
+ * EADDRNOTAVAIL/EPERM, which could be due to the
* "administratively set" flag being set in the PF for
* this VF. When this happens, we can attempt to use an
* alternate method to set the VF MAC: first set it into
--
2.9.3
7 years, 7 months
[libvirt] [PATCH v3 1/2] daemon: Fix domain name leak in error path
by Wang King
Domain name duplicated in make_nonnull_domain, but not freed when virTypedParamsSerialize
return negative.
---
daemon/remote.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 1610fea..5d726f4 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1067,7 +1067,7 @@ remoteRelayDomainEventTunable(virConnectPtr conn,
(virTypedParameterRemotePtr *) &data.params.params_val,
&data.params.params_len,
VIR_TYPED_PARAM_STRING_OKAY) < 0)
- return -1;
+ goto error;
remoteDispatchObjectEventSend(callback->client, remoteProgram,
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE,
@@ -1075,6 +1075,10 @@ remoteRelayDomainEventTunable(virConnectPtr conn,
&data);
return 0;
+ error:
+ VIR_FREE(data.dom.name);
+ return -1;
+
}
@@ -1207,13 +1211,16 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn,
(virTypedParameterRemotePtr *) &data.params.params_val,
&data.params.params_len,
VIR_TYPED_PARAM_STRING_OKAY) < 0)
- return -1;
+ goto error;
remoteDispatchObjectEventSend(callback->client, remoteProgram,
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED,
(xdrproc_t)xdr_remote_domain_event_callback_job_completed_msg,
&data);
return 0;
+ error:
+ VIR_FREE(data.dom.name);
+ return -1;
}
--
2.8.3
7 years, 7 months
[libvirt] [PATCH] util: allow ignoring SIOCSIFHWADDR when errno is EPERM
by Laine Stump
From: Laine Stump <laine(a)redhat.com>
Commit f4ef3a71 made a variation of virNetDevSetMAC that would return
without logging an error message if errno was set to
EADDRNOTAVAIL. This errno is set by some SRIOV VF drivers (in
particular igbvf) when they fail to set the device's MAC address due
to the PF driver refusing the request. This is useful if we want to
try a different method of setting the VF MAC address before giving up
(Commit 86556e16 actually does this, setting the desired MAC address
to the "admin MAC in the PF, then detaching and reattaching the VF
netdev driver to force a reinit of the MAC address).
During testing of Bug 1442040 t was discovered that the ixgbe driver
returns EPERM in this situation, so this patch changes the exception
case for silent+non-terminal failure to account for this difference.
Completes resolution to: https://bugzilla.redhat.com/1415609 (RHEL 7.4)
https://bugzilla.redhat.com/1442040 (RHEL 7.3.z)
---
NB: I could also change it to be silent on *any* error, but this could
lead to situations where the actual cause of a failure to set the MAC
address is obscured to the point of being unable to figure out the
problem (because the error would instead show up when the kernel is
trying to initialize the MAC address as it reattached the VF net
driver, for example). Limiting the errno's that warrant the silent
treatment allows us to give more useful error reporting in these other
cases.
src/util/virnetdev.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 170e348..9aa9c9f 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -226,7 +226,8 @@ int virNetDevExists(const char *ifname)
* virNetDevSetMACInternal:
* @ifname: interface name to set MTU for
* @macaddr: MAC address
- * @quiet: true if a failure to set MAC address with errno == EADDRNOTAVAIL
+ * @quiet: true if a failure to set MAC address with
+ * errno == EADDRNOTAVAIL || errno == EPERM
* should be silent (still returns error, but without log)
*
* This function sets the @macaddr for a given interface @ifname.
@@ -258,7 +259,8 @@ virNetDevSetMACInternal(const char *ifname,
if (ioctl(fd, SIOCSIFHWADDR, &ifr) < 0) {
- if (quiet && errno == EADDRNOTAVAIL)
+ if (quiet &&
+ (errno == EADDRNOTAVAIL || errno == EPERM))
goto cleanup;
virReportSystemError(errno,
@@ -305,7 +307,8 @@ virNetDevSetMACInternal(const char *ifname,
ifr.ifr_addr.sa_len = VIR_MAC_BUFLEN;
if (ioctl(s, SIOCSIFLLADDR, &ifr) < 0) {
- if (quiet && errno == EADDRNOTAVAIL)
+ if (quiet &&
+ (errno == EADDRNOTAVAIL || errno == EPERM))
goto cleanup;
virReportSystemError(errno,
@@ -2229,11 +2232,12 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
int retries = 100;
/* if pfDevOrig == NULL, this isn't a VF, so we've failed */
- if (!pfDevOrig || errno != EADDRNOTAVAIL)
+ if (!pfDevOrig ||
+ (errno != EADDRNOTAVAIL && errno != EPERM))
goto cleanup;
/* Otherwise this is a VF, and virNetDevSetMAC failed with
- * EADDRNOTAVAIL, which could be due to the
+ * EADDRNOTAVAIL/EPERM, which could be due to the
* "administratively set" flag being set in the PF for
* this VF. When this happens, we can attempt to use an
* alternate method to set the VF MAC: first set it into
--
2.9.3
7 years, 7 months
[libvirt] [PATCH v2 0/3] libxl: nestedhvm support
by Wim Ten Have
From: Wim ten Have <wim.ten.have(a)oracle.com>
This patch enhances host-passthrough capability to advertise the
required vendor CPU virtualization feature which will be used to
enable 'nestedhvm' in the libxl driver.
Wim ten Have (3):
libxl: set nestedhvm for mode host-passthrough
xenconfig: add conversions for xen-xl
xlconfigtest: add tests for 'nestedhvm' support
src/libxl/libxl_conf.c | 47 +++++++++++++--
src/libxl/libxl_conf.h | 2 +-
src/libxl/libxl_domain.c | 2 +-
src/xenconfig/xen_xl.c | 67 ++++++++++++++++++++++
.../test-fullvirt-nestedhvm-disabled.cfg | 26 +++++++++
.../test-fullvirt-nestedhvm-disabled.xml | 62 ++++++++++++++++++++
.../test-fullvirt-nestedhvm-undefined.cfg | 25 ++++++++
.../test-fullvirt-nestedhvm-undefined.xml | 58 +++++++++++++++++++
tests/xlconfigdata/test-fullvirt-nestedhvm.cfg | 26 +++++++++
tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 59 +++++++++++++++++++
tests/xlconfigtest.c | 3 +
11 files changed, 370 insertions(+), 7 deletions(-)
create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml
create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.xml
create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.xml
--
2.9.3
7 years, 7 months
[libvirt] [RFC] Add support for QEMU's l3-cache CPU property
by Jiri Denemark
Hi,
Apparently, reporting a level 3 cache on a virtual CPU can dramatically
increase performance in some use cases [1]. The interesting part is that
l3-cache=on does not provide the real CPU cache data, it's just making
it up. Anyway, we should be able to enable this via libvirt. And since
there is another property which enables real CPU cache data to be passed
to a guest, I suggest the following /domain/cpu/cache element equivalent
to l3-cache=on:
<cache level='3' mode='emulate'/>
If we need to add support for passing the real CPU cache data, we can do
that with
<cache level='3' mode='passthrough'/>
or we can even make the level attribute optional and support
<cache mode='passthrough'/>
Missing cache element means default behaviour of the hypervisor and we
can eventually add <cache mode='disable'/> to turn off passing any CPU
cache info to the guest.
But I think we should now focus only on <cache level='3' mode='emulate'/>
and leaving the rest for the future when we actually need it.
This is how a more complete example would look like:
<cpu mode='custom' match='exact'>
<model>Broadwell</model>
<cache level='3' mode='emulate'/>
</cpu>
And libvirt would translate it into -cpu Broadwell,l3-cache=on.
Do you have any thoughts about the XML schema or naming?
Thanks,
Jirka
[1] https://patchwork.kernel.org/patch/9308401/
7 years, 7 months
[libvirt] [PATCH] virsh: report errors in virshInit()
by Daniel P. Berrange
There are several functions in virshInit which can fail, especially
when running win32 builds under WINE. Currently virsh just exits
without reporting what error happened.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
tools/virsh.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 31e23bd..90f8125 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -358,16 +358,22 @@ virshInit(vshControl *ctl)
/* set up the library error handler */
virSetErrorFunc(NULL, vshErrorHandler);
- if (virEventRegisterDefaultImpl() < 0)
+ if (virEventRegisterDefaultImpl() < 0) {
+ vshReportError(ctl);
return false;
+ }
- if (virThreadCreate(&ctl->eventLoop, true, vshEventLoop, ctl) < 0)
+ if (virThreadCreate(&ctl->eventLoop, true, vshEventLoop, ctl) < 0) {
+ vshReportError(ctl);
return false;
+ }
ctl->eventLoopStarted = true;
if ((ctl->eventTimerId = virEventAddTimeout(-1, vshEventTimeout, ctl,
- NULL)) < 0)
+ NULL)) < 0) {
+ vshReportError(ctl);
return false;
+ }
if (ctl->connname) {
/* Connecting to a named connection must succeed, but we delay
--
2.9.3
7 years, 7 months
[libvirt] [PATCH] Increase default task limit for libvirtd
by Jim Fehlig
libvirtd can spawn threads/tasks when creating new domains for
some hypervisors such as Xen's libxl driver, quickly reaching
the cgroups pids controller default TasksMax setting of 512. When
the limit is reached, attempting to create additional domains
results in an error from the cgroups pids controller, e.g.
kernel: [71282.213347] cgroup: fork rejected by pids controller in
/system.slice/libvirtd.service
Depending on domain type and configuration, anywhere from 4-7
threads/tasks may be created by libxl when starting a domain.
In order to support 4096 domains, similar to commit 27cd763500,
increase the TasksMax setting in libvirtd.service to
4096 * 8 = 32768 tasks.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
daemon/libvirtd.service.in | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 899abdf37..fbaf02f3b 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -29,6 +29,11 @@ Restart=on-failure
# If changing this, also consider virtlogd.service & virtlockd.service
# limits which are also related to number of guests
LimitNOFILE=8192
+# The cgroups pids controller can limit the number of tasks started by
+# the daemon, which can limit the number of domains for some hypervisors.
+# A conservative default of 8 tasks per guest results in a TasksMax of
+# 32k to support 4096 guests.
+TasksMax=32768
[Install]
WantedBy=multi-user.target
--
2.11.0
7 years, 7 months
[libvirt] increase daemon task limit
by Jim Fehlig
I recently received a report of a libvirt+Xen installation reaching the pids
cgroup controller TasksMax limit
kernel: [71282.213347] cgroup: fork rejected by pids controller in
/system.slice/libvirtd.service
The default setting of TaskMax is 512 on this system
# cat /sys/fs/cgroup/pids/system.slice/libvirtd.service/pids.max
512
Depending on domain type and configuration, I've noticed between 5-7 tasks (IO
threads, qemu process, etc.) are created when starting a domain. It doesn't take
too many domains before the 512 limit is reached.
LimitNOFILE was recently changed by commit 27cd763500 to support 4096 domains.
Following similar logic, would it be ok to increase TasksMax to 32768? That
would accommodate 4096 domains with 8 tasks each. TasksMax also supports the
special value of "infinity", but that seems a bit aggressive to me.
Regards,
Jim
7 years, 7 months
[libvirt] [PATCH] Fix error reporting when poll returns POLLHUP/POLLERR
by Daniel P. Berrange
In the RPC client event loop code, if poll() returns only a POLLHUP
or POLLERR status, then we end up reporting a bogus error message:
error: failed to connect to the hypervisor
error: An error occurred, but the cause is unknown
We do actually report an error, but we virNetClientMarkClose method
has already captured the error status before we report it, so the
real error gets thrown away. The key fix is to report the error
before calling virNetClientMarkClose(). In changing this, we also
split out reporting of POLLHUP vs POLLERR to make any future bugs
easier to diagnose.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/rpc/virnetclient.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 5174614..c959747 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1743,10 +1743,16 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
if (error)
goto error;
- if (fds[0].revents & (POLLHUP | POLLERR)) {
+ if (fds[0].revents & POLLHUP) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("received hangup event on socket"));
virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_EOF);
+ goto error;
+ }
+ if (fds[0].revents & POLLERR) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("received hangup / error event on socket"));
+ _("received error event on socket"));
+ virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR);
goto error;
}
}
--
2.9.3
7 years, 7 months
[libvirt] [PATCH v2] daemon: Fix domain name leak in error path
by Wang King
Domain name duplicated in make_nonnull_domain, but not freed when virTypedParamsSerialize
return negative.
Remove useless error label incidentally.
---
daemon/remote.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 1610fea..d8a55c7 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -648,7 +648,7 @@ remoteRelayDomainEventBlockJob(virConnectPtr conn,
/* build return data */
memset(&data, 0, sizeof(data));
if (VIR_STRDUP(data.path, path) < 0)
- goto error;
+ return -1;
data.type = type;
data.status = status;
make_nonnull_domain(&data.dom, dom);
@@ -667,9 +667,6 @@ remoteRelayDomainEventBlockJob(virConnectPtr conn,
}
return 0;
- error:
- VIR_FREE(data.path);
- return -1;
}
@@ -1025,7 +1022,7 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn,
memset(&data, 0, sizeof(data));
data.callbackID = callback->callbackID;
if (VIR_STRDUP(data.dst, dst) < 0)
- goto error;
+ return -1;
data.type = type;
data.status = status;
make_nonnull_domain(&data.dom, dom);
@@ -1035,9 +1032,6 @@ remoteRelayDomainEventBlockJob2(virConnectPtr conn,
(xdrproc_t)xdr_remote_domain_event_block_job_2_msg, &data);
return 0;
- error:
- VIR_FREE(data.dst);
- return -1;
}
@@ -1067,7 +1061,7 @@ remoteRelayDomainEventTunable(virConnectPtr conn,
(virTypedParameterRemotePtr *) &data.params.params_val,
&data.params.params_len,
VIR_TYPED_PARAM_STRING_OKAY) < 0)
- return -1;
+ goto error;
remoteDispatchObjectEventSend(callback->client, remoteProgram,
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE,
@@ -1075,6 +1069,10 @@ remoteRelayDomainEventTunable(virConnectPtr conn,
&data);
return 0;
+ error:
+ VIR_FREE(data.dom.name);
+ return -1;
+
}
@@ -1207,13 +1205,16 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn,
(virTypedParameterRemotePtr *) &data.params.params_val,
&data.params.params_len,
VIR_TYPED_PARAM_STRING_OKAY) < 0)
- return -1;
+ goto error;
remoteDispatchObjectEventSend(callback->client, remoteProgram,
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED,
(xdrproc_t)xdr_remote_domain_event_callback_job_completed_msg,
&data);
return 0;
+ error:
+ VIR_FREE(data.dom.name);
+ return -1;
}
--
2.8.3
7 years, 7 months