Re: [libvirt] mdevctl: A shoestring mediated device management and persistence utility

On Mon, Jun 17, 2019 at 08:54:38AM -0600, Alex Williamson wrote: > On Mon, 17 Jun 2019 15:00:00 +0100 > Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > > On Thu, May 23, 2019 at 05:20:01PM -0600, Alex Williamson wrote: > > > Hi, > > > > > > Currently mediated device management, much like SR-IOV VF management, > > > is largely left as an exercise for the user. This is an attempt to > > > provide something and see where it goes. I doubt we'll solve > > > everyone's needs on the first pass, but maybe we'll solve enough and > > > provide helpers for the rest. Without further ado, I'll point to what > > > I have so far: > > > > > > https://github.com/awilliam/mdevctl > > > > > > This is inspired by driverctl, which is also a bash utility. mdevctl > > > uses udev and systemd to record and recreate mdev devices for > > > persistence and provides a command line utility for querying, listing, > > > starting, stopping, adding, and removing mdev devices. Currently, for > > > better or worse, it considers anything created to be persistent. I can > > > imagine a global configuration option that might disable this and > > > perhaps an autostart flag per mdev device, such that mdevctl might > > > simply "know" about some mdevs but not attempt to create them > > > automatically. Clearly command line usage help, man pages, and > > > packaging are lacking as well, release early, release often, plus this > > > is a discussion starter to see if perhaps this is sufficient to meet > > > some needs. > > > > I think from libvirt's POV, we would *not* want devices to be made > > unconditionally persistent. We usually wish to expose a choice to > > applications whether to have resources be transient or persistent. > > > > So from that POV, a global config option to turn off persistence > > is not workable either. We would want control per-device, with > > autostart control per device too. > > The code has progressed somewhat in the past 3+ weeks, we still persist > all devices, but the start-up mode can be selected per device or with a > global default mode. Devices configured with 'auto' start-up > automatically while 'manual' devices are simply known and available to > be started. I imagine we could add a 'transient' mode where we purge > the information about the device when it is removed or the next time > the parent device is added. Having a pesistent config written out & then purged later is still problematic. If the host crashes, nothing will purge the config file, so it will become a persistent device. Also when listing devices we want to be able to report whether it is persistent or transient. The obvious way todo that is to simply look if a config file exists or not.

> > I would simply get rid of the udev rule that magically persists > > stuff. Any person/tool using sysfs right now expects devices to > > be transient. If they want to have persistence they can stop using > > sysfs & use higher level tools directly. > > I think it's an interesting feature, but it's easy enough to control > via a global option in sysconfig with the default off if it's seen as > overstepping. A global option is really not desirable, as it means that the behaviour of the system that libvirt sees can silently change at any time. IMHO this udev hook is intermixing the two layers in the stack - keep the low level sysfs layer completely separate from the higher level mgmt concepts provided by this mdevctrl.

> > > Originally I thought about making a utility to manage both mdev and > > > SR-IOV VFs all in one, but it seemed more natural to start here > > > (besides, I couldn't think of a good name for the combined utility). > > > If this seems useful, maybe I'll start on a vfctl for SR-IOV and we'll > > > see whether they have enough synergy to become one. > > > > [snip] > > > > > I'm also curious how or if libvirt or openstack might use this. If > > > nothing else, it makes libvirt hook scripts easier to write, especially > > > if we add an option not to autostart mdevs, or if users don't mind > > > persistent mdevs, maybe there's nothing more to do. > > > > We currently have an API for creating host devices in libvirt which > > we use for NPIV devices only, which is where we'd like to put mdev > > creation support. This API is for creating transient devices > > though, so we don't want anything created this way to magically > > become persistent. > > > > For persistence we'd create a new API in libvirt allowing you to > > define & undefine the persistent config for a devices, and another > > set of APIs to create/delete from the persistent config. > > > > As a general rule, libvirt would prefer to use an API rather than > > spawning external commands, but can live with either. Thinking some more, the key tasks that libvirt needs to deal with are 1. Define a persistent config (Must not create any actual device) 2. Undefine a persistent config (Must not delete any actual device) 3. Create a device 4. Delete a device 5. Get list of all persistent configs 6. Enable autostart of a device 7. Disable autostart of a device For 1, 2, 5, 6 & 7 libvirt doesn't really need a command line tool. As long as there is a specification for the config file syntax and location we can directly read/write/stat files. This would be much more efficient and reliable for libvirt than spawning commands & parsing the output or exit status.

For 3 & 4 we'd desire the command to accept a config file, either as a file on disk for persistent devices, or inline on stdin for transient devices. > > There's also the question of systemd integration - so far everything > > in libvirt still works on non-systemd based distros, but this new > > tool looks like it requires systemd. Personally I'm not too bothered > > by this but others might be more concerned. > > Yes, Pavel brought up this issue offline as well and it needs more > consideration. The systemd support still needs work as well, I've > discovered it gets very confused when the mdev device is removed > outside of mdevctl, but I haven't yet been able to concoct a BindsTo= > line that can handle the hyphens in the uuid device name. I'd say > mdevctl is not intentionally systemd specific, it's simply a byproduct > of the systems it was developed on. Also, if libvirt were to focus > only on transient devices, then startup via systemctl doesn't make > sense, which probably means stopping via systemctl would also be unused > by libvirt. So I think this means we just need to make systemd an > optional feature of mdevctl (or drop it) and if libvirt doesn't use it, > that's fine. Thanks, I actually think the systemd integration is good, we just need to make sure things are still doable without it. This could be as simple as having an initscript that just iteates over the configs creating each one at startup.