On 8/19/19 4:48 PM, Jonathon Jongsma wrote:
On Wed, 2019-08-14 at 16:14 +0200, Boris Fiuczynski wrote:
On 8/14/19 12:02 AM, Jonathon Jongsma wrote:
When a host is rebooted, mediated devices disappear from
sysfs. mdevctl
(https://github.com/mdevctl/mdevctl) is a utility for managing and
persisting these devices. It maintains a registry of mediated
devices
and can start and stop them by UUID.
when libvirt attempts to create a new mediated device object, we
currently
fail if the path does not exist in sysfs. This patch tries a little
bit
harder by using mdevctl to attempt to activate the device. The
approach
is fairly naive -- it just calls 'mdevctl start -u $UUID' without
checking whether this UUID is registered with mdevctl or not.
See https://bugzilla.redhat.com/show_bug.cgi?id=1699274
Signed-off-by: Jonathon Jongsma <jjong...@redhat.com>
---
NOTES:
- an argument could be made that we should simply do nothing here.
mdevctl does
have support for automatically activating the mediated device
when the parent
device becomes available (via udev). So if the administrator set
up the mdev
to start automatically, this patch should not even be necessary.
That said, I
think this patch could still be useful.
I would actually like to use this argument since this patch
unconditionally starts/creates a persistently defined mdev without
ever
stopping/destroying it and also not looking if it is defined as to
be
automatically started or manually started. If the mdev is specified
in
mdevctl to be started automatically I would rate this as mdevctl is
in
control of this mdevs lifecycle and libvirt should not interfere
with
it. It might be that I am over-interpreting auto and manual as start
options in mdevctl but it feels to me that libvirt and mdevctl
should
not run into a management clash of host resources.
In addition what about a user specifiable tag in the domain xmls
mdev
definition which controls the management behavior of an mdev with
mdevctl or another tool?
Thanks for the feedback. I welcome additional opinions on this. If
there's a concensus that the right approach is to do nothing, I can
drop this patch. Or alternately, we could simply point users toward
mdevctl in the error message. For example, something like:
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 3d5488cdae..70d990eace 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -149,7 +149,7 @@ virMediatedDeviceNew(const char *uuidstr,
virMediatedDeviceModelType model)
if (!virFileExists(sysfspath)) {
virReportError(VIR_ERR_DEVICE_MISSING,
- _("mediated device '%s' not found"), uuidstr);
+ _("mediated device '%s' not found. Persistent
devices can be managed with 'mdevctl'."), uuidstr);
return NULL;
}
Unless there is a request for us to start mdevs, I'd rather see this
error message than us trying to do anything because libvirt's already
trying to manage more than enough and it's biting us back.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
