Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia
> Sent: Wednesday, February 17, 2016 1:38 PM
> > > >
> > > >
> > >
> > > Hi Kevin,
> > >
> > > The answer is simple, having a UUID as part of the device name will give 
> > > you a
> > > unique sysfs path that will be opened by QEMU.
> > >
> > > vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
> > > virtual gpu devices per VM coming from same or different physical devices.
> >
> > That is not a problem. We can add physical device info too like 
> > vgpu-vendor-0-0,
> > vgpu-vendor-1-0, ...
> >
> > Please note Qemu doesn't care about the actual name. It just accepts a 
> > sysfs path
> > to open.
> 
> Hi Kevin,
> 
> No, I think you are making things even more complicated than it is required,
> also it is not generic anymore as you are requiring the QEMU to know more than
> he needs to.
> 
> The way you name those devices will require QEMU to know the relation
> between virtual devices and physical devices. I don't think that is good.

I don't think you get my point. Look at how device is assigned in Qemu today:

-device vfio-pci,host=02:00.0

Then in a recent patch from Alex, Qemu will accept sysfsdev as well:

-device vfio-pci,sysfsdev=/sys/devices/pci0000:00/0000:00:1c.0/0000:02:00.0

Then with vgu (one example from Alex's original post):

-device vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/address@hidden:00:02.0

Qemu doesn't need to know the relation between virtual/physical devices at
all. It's just a path regardless of how vgpu name is created (either with your
UUID proposal or my descriptive string proposal)

> 
> My flow is like this:
> 
> libvirt creats a VM object, it will have a UUID. then it will use the UUID to
> create virtual gpu devices, then it will pass the UUID to the QEMU (actually
> QEMU already has the VM UUID), then it will just open up the unique path.

If you look at above example, it's not UUID itself being passed to Qemu. It's
the sysfsdev path.

> 
> Also, you need to consider those 0-0 numbers are not generic as the UUID.

that encoding could be flexible to include any meaningful string. libvirt can
itself manages how UUID is mapped to an actual vgpu name.

> >
> > >
> > > If you are worried about losing meaningful name here, we can create a 
> > > sysfs file
> > > to capture the vendor device description if you like.
> > >
> >
> > Having the vgpu name descriptive is more informative imo. User can simply 
> > check
> > sysfs names to know raw information w/o relying on 3rd party agent to query
> > information around an opaque UUID.
> >
> 
> You are actually arguing against your own design here, unfortunately. If you
> look at your design carefully, it is your design actually require to have a 
> 3rd
> party code to figure out the VM and virtual gpu device relation as it is
> never documented in the sysfs.

No. It's not about figuring out the relation between VM and vGPU. It's about
figuring out some raw information about vGPU itself, such as:

address@hidden:00:02.0, I can immediately know it's the 1st instance created on
0000:00:02.0 device. While with an UUID, it doesn't speak anything useful here.

This is a typical way how device nodes are created within sysfs, e.g. on my
platform:

$ ls /sys/class/drm/
card0/          card0-DP-2/     card0-HDMI-A-2/ controlD64/
card0-DP-1/     card0-HDMI-A-1/ card0-VGA-1/    version

$ ls /sys/bus/pci/devices/
0000:00:00.0  0000:00:14.0  0000:00:1a.0  0000:00:1c.1  0000:00:1f.2
0000:00:02.0  0000:00:16.0  0000:00:1b.0  0000:00:1d.0  0000:00:1f.3
0000:00:03.0  0000:00:19.0  0000:00:1c.0  0000:00:1f.0  0000:02:00.0

We'd better keep such style when creating vgpu nodes in sysfs. UUID is
at most anther info suffixed to the default string (or in another file), if 
necessary.

> 
> In our current design, it doesn't require any 3rd party agent as the VM UUID 
> is
> part of the QEMU command already, and the VM UUID is already embedded within 
> the
> virtual device path.
> 
> Also, it doesn't require 3rd party to retrieve information as the virtual 
> device
> will just be a directory, we will have another file within each virtual gpu
> device directory, you can always cat the file to retrieve vendor information.
> 
> Let's use the UUID-$vgpu_idx as the virtual device directory name plus a 
> vendor
> description file within that directory, so we don't lose any additional
> information, also capture the VM and virtual device relation.
> 

I'm fine to have another node to provide more vendor specific information. But
I don't want to make UUID mandatory when creating a vGPU instance, as 
explained above. Today we can create VMs in KVM w/o using libvirt, and w/o 
the need of allocating any UUID. Same thing should be supported for vgpu
feature too.

Thanks
Kevin