KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Libvirt Devel" <libvir-list@redhat.com>,
	"Kirti Wankhede" <kwankhede@nvidia.com>,
	"Erik Skultety" <eskultet@redhat.com>,
	"Pavel Hrdina" <phrdina@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Sylvain Bauza" <sbauza@redhat.com>,
	"Christophe de Dinechin" <dinechin@redhat.com>,
	"Matthew Rosato" <mjrosato@linux.ibm.com>
Subject: Re: mdevctl: A shoestring mediated device management and persistence utility
Date: Wed, 26 Jun 2019 08:37:20 -0600
Message-ID: <20190626083720.42a2b5d4@x1.home> (raw)
In-Reply-To: <20190626115806.3435c45c.cohuck@redhat.com>

On Wed, 26 Jun 2019 11:58:06 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 25 Jun 2019 16:52:51 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > Hi,
> > 
> > Based on the discussions we've had, I've rewritten the bulk of
> > mdevctl.  I think it largely does everything we want now, modulo
> > devices that will need some sort of 1:N values per key for
> > configuration in the config file versus the 1:1 key:value setup we
> > currently have (so don't consider the format final just yet).  
> 
> We might want to factor out that config format handling while we're
> trying to finalize it.
> 
> cc:ing Matt for his awareness. I'm currently not quite sure how to
> handle those vfio-ap "write several values to an attribute one at a
> time" requirements. Maybe 1:N key:value is the way to go; maybe we
> need/want JSON or something like that.

Maybe we should just do JSON for future flexibility.  I assume there
are lots of helpers that should make it easy even from a bash script.
I'll look at that next.

> > We now support "transient" devices and there's no distinction or
> > difference in handling of such devices whether they're created by
> > mdevctl or externally.  All devices will also have systemd management,
> > though systemd is no longer required, it's used if available.  The
> > instance name used for systemd device units has also changed to allow
> > us to use BindsTo= such that services are not only created, but are
> > also removed if the device is removed.  Unfortunately it's not a simple
> > UUID via the systemd route any longer.  
> 
> That's a bit unfortunate; however, making it workable without systemd
> certainly is a good thing :)

The "decoder ring" is simply that the instance value takes the systemd
device path of the mdev device itself.  The mdev device is named by the
uuid and is created under the parent device, so we just need to get the
realpath of the parent, append the uuid, and encode it with
systemd-escape.  It's not magic, but it's  not trivial on the command
line either.  We could add a command to mdevctl to print this, but it
doesn't make much sense to call into mdevctl for that and not simply
control the device via mdevctl.

> > Since the original posting, the project has moved from my personal
> > github to here:
> > 
> > https://github.com/mdevctl/mdevctl
> > 
> > Please see the README there for overview of the new commands and
> > example of their usage.  There is no attempt to maintain backwards
> > compatibility with previous versions, this tool is in its infancy.
> > Also since the original posting, RPM packaging is included, so simply
> > run 'make rpm' and install the resulting package.  
> 
> Nice.
> 
> > 
> > Highlights of this version include proper argument parsing via getopts
> > so that options can be provided in any order.  I'm still using the
> > format 'mdevctl {command} [options]' but now it's consistent that all
> > the options come after the command, in any order.  I think this is
> > relatively consistent with a variety of other tools.  
> 
> Parsing via getops is also very nice.
> 
> > 
> > Devices are no longer automatically persisted, we handle them as
> > transient, but we also can promote them to persistent through the
> > 'define' command.  The define, undefine, and modify commands all
> > operate only on the config file, so that we can define separate from
> > creating.  When promoting from a transient to defined device, we can
> > use the existing device to create the config.  Both the type and the
> > startup of a device can be modified in the config, without affecting
> > the running device.
> > 
> > Starting an mdev device no longer relies exclusively on a saved config,
> > the device can be fully specified via options to create a transient
> > device.  Specifying only a uuid is also sufficient for a defined
> > device.  Some consideration has also been given to uuid collisions.
> > The mdev interface in the kernel prevents multiple mdevs with the same
> > uuid running concurrently, but mdevctl allows mdevs to be defined with
> > the same uuid under separate parent devices.  Some options therefore
> > allow both a uuid and parent to be specified and require this if the
> > uuid alone is ambiguous.  Clearly starting two such devices at the same
> > time will fail and is left to higher level tools to manage, just like
> > the ability to define more devices than there are available instances on
> > the host system.  
> 
> I still have to look into the details of this.
> 
> > 
> > The stop and list commands are largely the same ideas as previous
> > though the semantics are completely different.  Listing running devices
> > now notes which are defined versus transient.  Perhaps it might also be
> > useful when listing defined devices to note which are running.  
> 
> Yes, I think it would be useful.
> 
> > 
> > The sbin/libexec split of mdevctl has been squashed.  There are some
> > commands in the script that are currently only intended to be used from
> > udev or systemd, these are simply excluded from the help.  It's
> > possible we may want to promote the start-parent-mdevs command out of
> > this class, but the rest are specifically systemd helpers.
> > 
> > I'll include the current help test message below for further semantic
> > details, but please have a look-see, or better yet give it a try.  
> 
> Had a quick look, sent two pull requests with some minor tweaks. Still
> have to do a proper review, and actually try the thing on an s390.

Pull requests merged, thanks!

> > 
> > PS - I'm looking at adding udev change events when a device registers
> > or unregisters with the mdev core, which should help us know when to
> > trigger creation of persistent, auto started devices.  That support is
> > included here with the MDEV_STATE="registered|unregistered" environment
> > values.  Particularly, kvmgt now supports dynamic loading an unloading,
> > so as long as the enable_gvt=1 option is provided to the i915 driver
> > mdev support can come and go independent of the parent device.  The
> > change uevents are necessary to trigger on that, so I'd appreciate any
> > feedback on those as well.  Until then, the persistence of mdevctl
> > really depends on mdev support on the parent device being _completely_
> > setup prior to processing the udev rules.  
> 
> I'll need to think about this. Do you have some preliminary patches
> somewhere?

I posted what I'm working with, you're cc'd, but for the benefit of the
rest of the list: https://lkml.org/lkml/2019/6/26/574
Thanks,

Alex

  reply index

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23 23:20 Alex Williamson
2019-05-24 10:11 ` Cornelia Huck
2019-05-24 14:43   ` Alex Williamson
2019-06-07 16:06   ` Halil Pasic
2019-06-11 19:45     ` Cornelia Huck
2019-06-11 20:28       ` Alex Williamson
2019-06-12  7:14         ` Cornelia Huck
2019-06-12 15:54           ` Halil Pasic
2019-06-13 10:02             ` Cornelia Huck
2019-06-12 15:07       ` Halil Pasic
2019-06-13 16:17 ` Christophe de Dinechin
2019-06-13 16:35   ` Alex Williamson
2019-06-14  9:54     ` [libvirt] " Christophe de Dinechin
2019-06-14 14:23       ` Alex Williamson
2019-06-14 15:06         ` Christophe de Dinechin
2019-06-14 16:04           ` Alex Williamson
2019-06-17 16:03             ` Cornelia Huck
2019-06-17 14:00 ` Daniel P. Berrangé
2019-06-17 14:54   ` Alex Williamson
2019-06-17 15:10     ` Daniel P. Berrangé
2019-06-17 17:05       ` Alex Williamson
2019-06-18  8:44         ` Daniel P. Berrangé
2019-06-18 11:01         ` Cornelia Huck
     [not found]           ` <CALOCmukPWiXiM+mN0hCTvSwfdHy5UdERU8WnvOXiBrMQ9tH3VA@mail.gmail.com>
2019-06-18 22:12             ` Alex Williamson
2019-06-19  7:28               ` Daniel P. Berrangé
2019-06-19  9:46                 ` Cornelia Huck
2019-06-19 18:46                   ` Alex Williamson
2019-06-20  8:24                     ` Daniel P. Berrangé
     [not found]               ` <CALOCmu=6Xmw-_-SVXujCEcgPY2CQiBQKgfUMJ45WnZ_9XORyUw@mail.gmail.com>
2019-06-19  9:57                 ` Cornelia Huck
2019-06-19 19:53                 ` Alex Williamson
2019-06-25 22:52 ` Alex Williamson
2019-06-26  9:58   ` Cornelia Huck
2019-06-26 14:37     ` Alex Williamson [this message]
2019-06-27  1:53       ` Alex Williamson
2019-06-27 12:26         ` Cornelia Huck
2019-06-27 15:00           ` Matthew Rosato
2019-06-27 15:38             ` Alex Williamson
2019-06-27 16:13               ` Matthew Rosato
2019-06-27 21:15               ` Alex Williamson
2019-06-28  1:57                 ` Alex Williamson
2019-06-28  9:06                   ` Cornelia Huck
2019-06-28 14:01                     ` Matthew Rosato
2019-06-28 17:05                     ` Alex Williamson
2019-07-01  8:20                       ` Cornelia Huck
2019-07-01 14:40                         ` Alex Williamson
2019-07-01 17:13                           ` Cornelia Huck

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190626083720.42a2b5d4@x1.home \
    --to=alex.williamson@redhat.com \
    --cc=berrange@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=dinechin@redhat.com \
    --cc=eskultet@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwankhede@nvidia.com \
    --cc=libvir-list@redhat.com \
    --cc=mjrosato@linux.ibm.com \
    --cc=phrdina@redhat.com \
    --cc=sbauza@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox