Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

On Thu, 1 Sep 2016 18:47:06 +0200 Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: > On 31.08.2016 08:12, Tian, Kevin wrote: >>> From: Alex Williamson [mailto:alex.williamson@redhat.com] >>> Sent: Wednesday, August 31, 2016 12:17 AM >>> >>> Hi folks, >>> >>> At KVM Forum we had a BoF session primarily around the mediated device >>> sysfs interface. I'd like to share what I think we agreed on and the >>> "problem areas" that still need some work so we can get the thoughts >>> and ideas from those who weren't able to attend. >>> >>> DanPB expressed some concern about the mdev_supported_types sysfs >>> interface, which exposes a flat csv file with fields like "type", >>> "number of instance", "vendor string", and then a bunch of type >>> specific fields like "framebuffer size", "resolution", "frame rate >>> limit", etc. This is not entirely machine parsing friendly and sort of >>> abuses the sysfs concept of one value per file. Example output taken >>> from Neo's libvirt RFC: >>> >>> cat /sys/bus/pci/devices/0000:86:00.0/mdev_supported_types >>> # vgpu_type_id, vgpu_type, max_instance, num_heads, frl_config, framebuffer, >>> max_resolution >>> 11 ,"GRID M60-0B", 16, 2, 45, 512M, 2560x1600 >>> 12 ,"GRID M60-0Q", 16, 2, 60, 512M, 2560x1600 >>> 13 ,"GRID M60-1B", 8, 2, 45, 1024M, 2560x1600 >>> 14 ,"GRID M60-1Q", 8, 2, 60, 1024M, 2560x1600 >>> 15 ,"GRID M60-2B", 4, 2, 45, 2048M, 2560x1600 >>> 16 ,"GRID M60-2Q", 4, 4, 60, 2048M, 2560x1600 >>> 17 ,"GRID M60-4Q", 2, 4, 60, 4096M, 3840x2160 >>> 18 ,"GRID M60-8Q", 1, 4, 60, 8192M, 3840x2160 >>> >>> The create/destroy then looks like this: >>> >>> echo "$mdev_UUID:vendor_specific_argument_list" > >>> /sys/bus/pci/devices/.../mdev_create >>> >>> echo "$mdev_UUID:vendor_specific_argument_list" > >>> /sys/bus/pci/devices/.../mdev_destroy >>> >>> "vendor_specific_argument_list" is nebulous. >>> >>> So the idea to fix this is to explode this into a directory structure, >>> something like: >>> >>> ├── mdev_destroy >>> └── mdev_supported_types >>> ├── 11 >>> │ ├── create >>> │ ├── description >>> │ └── max_instances >>> ├── 12 >>> │ ├── create >>> │ ├── description >>> │ └── max_instances >>> └── 13 >>> ├── create >>> ├── description >>> └── max_instances >>> >>> Note that I'm only exposing the minimal attributes here for simplicity, >>> the other attributes would be included in separate files and we would >>> require vendors to create standard attributes for common device classes. >> >> I like this idea. All standard attributes are reflected into this hierarchy. >> In the meantime, can we still allow optional vendor string in create >> interface? libvirt doesn't need to know the meaning, but allows upper >> layer to do some vendor specific tweak if necessary. > > This is not the best idea IMO. Libvirt is there to shadow differences > between hypervisors. While doing that, we often hide differences between > various types of HW too. Therefore in order to provide good abstraction > we should make vendor specific string as small as possible (ideally an > empty string). I mean I see it as bad idea to expose "vgpu_type_id" from > example above in domain XML. What I think the better idea is if we let > users chose resolution and frame buffer size, e.g.: <video > resolution="1024x768" framebuffer="16"/> (just the first idea that came > to my mind while writing this e-mail). The point is, XML part is > completely free of any vendor-specific knobs. That's not really what you want though, a user actually cares whether they get an Intel of NVIDIA vGPU, we can't specify it as just a resolution and framebuffer size. The user also doesn't want the model changing each time the VM is started, so not only do you *need* to know the vendor, you need to know the vendor model. This is the only way to provide a consistent VM. So as we discussed at the BoF, the libvirt xml will likely reference the vendor string, which will be a unique identifier that encompasses all the additional attributes we expose. Really the goal of the attributes is simply so you don't need a per vendor magic decoder ring to figure out the basic features of a given vendor string. Thanks,