[libvirt] [PATCH] util: mdev: support persistent devices with mdevctl
Boris Fiuczynski
fiuczy at linux.ibm.com
Wed Aug 14 14:14:32 UTC 2019
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 <jjongsma at 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?
> - As I said above, the approach is pretty naive. I could have checked whether
> the device was defined in mdevctl's registry and given a different error
> message ("try registering this device with mdevctl") in that scenario. But
> I chose to keep it simple.
>
> src/util/virmdev.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> index 3d5488cdae..7a2f0adedc 100644
> --- a/src/util/virmdev.c
> +++ b/src/util/virmdev.c
> @@ -25,6 +25,7 @@
> #include "virfile.h"
> #include "virstring.h"
> #include "viralloc.h"
> +#include "vircommand.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -148,9 +149,24 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
> return NULL;
>
> if (!virFileExists(sysfspath)) {
> - virReportError(VIR_ERR_DEVICE_MISSING,
> - _("mediated device '%s' not found"), uuidstr);
> - return NULL;
> + bool activated = false;
> + /* if the mdev device path doesn't exist, we may still be able to start
> + * the device using mdevctl
> + */
> + char *mdevctl = virFindFileInPath("mdevctl");
> + if (mdevctl) {
> + const char *runargs[] = { mdevctl, "start", "-u", uuidstr, NULL };
> + if (virRun(runargs, NULL) == 0) {
> + /* check to see if it the device exists now */
> + activated = virFileExists(sysfspath);
> + }
> + }
> +
> + if (!activated) {
> + virReportError(VIR_ERR_DEVICE_MISSING,
> + _("mediated device '%s' not found"), uuidstr);
> + return NULL;
> + }
> }
>
> if (VIR_ALLOC(dev) < 0)
>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
More information about the libvir-list
mailing list