All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Neo Jia <cjia@nvidia.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>,
	"Daniel P. Berrange" <berrange@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Andy Currid <ACurrid@nvidia.com>,
	"libvir-list@redhat.com" <libvir-list@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	"Song, Jike" <jike.song@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"bjsdjshi@linux.vnet.ibm.com" <bjsdjshi@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [libvirt] [RFC v2] libvirt vGPU QEMU integration
Date: Wed, 28 Sep 2016 16:49:52 -0600	[thread overview]
Message-ID: <20160928164952.29446151@t450s.home> (raw)
In-Reply-To: <20160928195959.GB26800@nvidia.com>

On Wed, 28 Sep 2016 12:59:59 -0700
Neo Jia <cjia@nvidia.com> wrote:

> On Wed, Sep 28, 2016 at 07:45:38PM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Thursday, September 29, 2016 3:23 AM
> > > 
> > > On Thu, Sep 22, 2016 at 03:26:38PM +0100, Daniel P. Berrange wrote:  
> > > > On Thu, Sep 22, 2016 at 08:19:21AM -0600, Alex Williamson wrote:  
> > > > > On Thu, 22 Sep 2016 09:41:20 +0530
> > > > > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> > > > >  
> > > > > > >>>>> My concern is that a type id seems arbitrary but we're specifying that
> > > > > > >>>>> it be unique.  We already have something unique, the name.  So why try
> > > > > > >>>>> to make the type id unique as well?  A vendor can accidentally create
> > > > > > >>>>> their vendor driver so that a given name means something very
> > > > > > >>>>> specific.  On the other hand they need to be extremely deliberate to
> > > > > > >>>>> coordinate that a type id means a unique thing across all their product
> > > > > > >>>>> lines.
> > > > > > >>>>>  
> > > > > > >>>>
> > > > > > >>>> Let me clarify, type id should be unique in the list of
> > > > > > >>>> mdev_supported_types. You can't have 2 directories in with same name.  
> > > > > > >>>
> > > > > > >>> Of course, but does that mean it's only unique to the machine I'm
> > > > > > >>> currently running on?  Let's say I have a Tesla P100 on my system and
> > > > > > >>> type-id 11 is named "GRID-M60-0B".  At some point in the future I
> > > > > > >>> replace the Tesla P100 with a Q1000 (made up).  Is type-id 11 on that
> > > > > > >>> new card still going to be a "GRID-M60-0B"?  If not then we've based
> > > > > > >>> our XML on the wrong attribute.  If the new device does not support
> > > > > > >>> "GRID-M60-0B" then we should generate an error, not simply initialize
> > > > > > >>> whatever type-id 11 happens to be on this new card.
> > > > > > >>>  
> > > > > > >>
> > > > > > >> If there are 2 M60 in the system then you would find '11' type directory
> > > > > > >> in mdev_supported_types of both M60. If you have P100, '11' type would
> > > > > > >> not be there in its mdev_supported_types, it will have different types.
> > > > > > >>
> > > > > > >> For example, if you replace M60 with P100, but XML is not updated. XML
> > > > > > >> have type '11'. When libvirt would try to create mdev device, libvirt
> > > > > > >> would have to find 'create' file in sysfs in following directory format:
> > > > > > >>
> > > > > > >>  --- mdev_supported_types
> > > > > > >>      |-- 11
> > > > > > >>      |   |-- create
> > > > > > >>
> > > > > > >> but now for P100, '11' directory is not there, so libvirt should throw
> > > > > > >> error on not able to find '11' directory.  
> > > > > > >
> > > > > > > This really seems like an accident waiting to happen.  What happens
> > > > > > > when the user replaces their M60 with an Intel XYZ device that happens
> > > > > > > to expose a type 11 mdev class gpu device?  How is libvirt supposed to
> > > > > > > know that the XML used to refer to a GRID-M60-0B and now it's an
> > > > > > > INTEL-IGD-XYZ?  Doesn't basing the XML entry on the name and removing
> > > > > > > yet another arbitrary requirement that we have some sort of globally
> > > > > > > unique type-id database make a lot of sense?  The same issue applies
> > > > > > > for simple debug-ability, if I'm reviewing the XML for a domain and the
> > > > > > > name is the primary index for the mdev device, I know what it is.
> > > > > > > Seeing type-id='11' is meaningless.
> > > > > > >  
> > > > > >
> > > > > > Let me clarify again, type '11' is a string that vendor driver would
> > > > > > define (see my previous reply below) it could be "11" or "GRID-M60-0B".
> > > > > > If 2 vendors used same string we can't control that. right?
> > > > > >
> > > > > >  
> > > > > > >>>> Lets remove 'id' from type id in XML if that is the concern. Supported
> > > > > > >>>> types is going to be defined by vendor driver, so let vendor driver
> > > > > > >>>> decide what to use for directory name and same should be used in device
> > > > > > >>>> xml file, it could be '11' or "GRID M60-0B":
> > > > > > >>>>
> > > > > > >>>>     <device>
> > > > > > >>>>       <name>my-vgpu</name>
> > > > > > >>>>       <parent>pci_0000_86_00_0</parent>
> > > > > > >>>>       <capability type='mdev'>
> > > > > > >>>>         <type='11'/>
> > > > > > >>>>         ...
> > > > > > >>>>       </capability>
> > > > > > >>>>     </device>  
> > > > >
> > > > > Then let's get rid of the 'name' attribute and let the sysfs directory
> > > > > simply be the name.  Then we can get rid of 'type' altogether so we
> > > > > don't have this '11' vs 'GRID-M60-0B' issue.  Thanks,  
> > > >
> > > > That sounds nice to me - we don't need two unique identifiers if
> > > > one will do.  
> > > 
> > > Hi Alex and Daniel,
> > > 
> > > I just had some internal discussions here within NVIDIA and found out that
> > > actually the name/label potentially might not be unique and the "id" will be.
> > > So I think we still would like to keep both so the id is the programmatic id
> > > and the name/label is a human readable string for it, which might get changed to
> > > be non-unique by outside of engineering.
> > > 
> > > Sorry for the change.
> > > 
> > > Thanks,
> > > Neo
> > >   
> > 
> > A curious question. How do we expect such a descriptive name/label used
> > by upper-level stack (e.g. openstack)? Should openstack define a vGPU
> > flavor just using ID (GRID-type11) or using both ID/name (GRID-type11-
> > M60-0B) for end customer to choose? If it's only for human information,
> > does it make sense e.g. providing only unique ID in sysfs while relying on 
> > vendor specific documentation to describe what the ID actually means?  
> 
> Hi Kevin,
> 
> The id is not visible to the upper-level stack, only the name / label will be
> shown to the end customer to choose, such as "GRID-M60-0B", as we might expose
> the same virtual device (name/label) with some internal difference which will 
> be tracked by the different unique id.
> 
> I think having the ability to allow libvirt or upper-level stack to display a
> human readable string for a given type of vgpu will make the user life easier.

Again, we need a stable and unique string to go into the XML.  That
should represent a consistent device regardless of host driver versions
or specific hardware.  I would consider that string to be user visible.

Also, can you define exactly what you mean by "unique"?  What's the
purpose of the label "GRID-M60-0B" if it's not unique?  Does type 11
mean "GRID-M60-0B" as implemented on a specific card and type 12 might
mean "GRID-M60-0B" as implemented on a different card?  Do you want
your users to be able to instantiate their VM on any "GRID-M60-0B"
mdev, or does it need to be a type 11 GRID-M60-0B mdev?  Thanks,

Alex

  parent reply	other threads:[~2016-09-28 22:49 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19 20:35 [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration Kirti Wankhede
2016-09-19 21:36 ` Alex Williamson
2016-09-19 21:50   ` Paolo Bonzini
2016-09-19 22:25     ` Alex Williamson
2016-09-20 14:35       ` Kirti Wankhede
2016-09-20 14:41         ` Daniel P. Berrange
2016-09-20 14:49           ` Paolo Bonzini
2016-09-20 14:58             ` Daniel P. Berrange
2016-09-20 15:05               ` Paolo Bonzini
2016-09-20 15:14                 ` Daniel P. Berrange
2016-09-20 16:31                   ` Kirti Wankhede
2016-09-20 16:36                     ` Daniel P. Berrange
2016-09-20 16:42                       ` Kirti Wankhede
2016-09-20 16:44                         ` Daniel P. Berrange
2016-09-20 16:46                     ` Daniel P. Berrange
2016-09-20 17:21                   ` Paolo Bonzini
2016-09-21  8:34                     ` Daniel P. Berrange
2016-09-20 14:52         ` Alex Williamson
2016-09-20  1:25   ` Tian, Kevin
2016-09-20 14:21   ` Kirti Wankhede
2016-09-20 14:43     ` Alex Williamson
2016-09-20 16:23       ` Kirti Wankhede
2016-09-20 16:50         ` Alex Williamson
2016-09-21 18:34           ` Kirti Wankhede
2016-09-21 19:03             ` Alex Williamson
2016-09-22  4:11               ` Kirti Wankhede
2016-09-22 14:19                 ` Alex Williamson
2016-09-22 14:26                   ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2016-09-28 19:22                     ` Neo Jia
2016-09-28 19:45                       ` Tian, Kevin
2016-09-28 19:59                         ` Neo Jia
2016-09-28 20:31                           ` Laine Stump
2016-09-28 20:47                             ` Neo Jia
2016-09-28 22:49                           ` Alex Williamson [this message]
2016-09-28 19:55                       ` Alex Williamson
2016-09-28 20:06                         ` Neo Jia
2016-09-28 22:39                           ` Alex Williamson
2016-09-29  8:03                       ` Daniel P. Berrange
2016-09-29  8:12                         ` Neo Jia
2016-09-29 14:22               ` [Qemu-devel] " Kirti Wankhede
2016-09-21  4:10         ` Tian, Kevin
2016-09-21  4:43           ` Alex Williamson
2016-09-22  2:43             ` Tian, Kevin
2016-09-22 19:25         ` Tian, Kevin
2016-09-23 18:34           ` Kirti Wankhede
2016-09-21  3:56     ` Tian, Kevin
2016-09-21  4:36       ` Alex Williamson
2016-09-22  2:33         ` Tian, Kevin
2016-09-22  3:01           ` Alex Williamson
2016-09-22  3:42             ` Tian, Kevin
     [not found]         ` <AADFC41AFE54684AB9EE6CBC0274A5D18DF86F5F@SHSMSX101.ccr.corp.intel.com>
2016-09-22  2:59           ` Tian, Kevin
2016-09-20  1:37 ` Tian, Kevin
2016-09-20  9:47 ` Daniel P. Berrange
2016-09-28 19:48   ` Neo Jia
2016-09-29  8:06     ` Daniel P. Berrange
2016-09-29 14:35       ` Tian, Kevin
2016-09-29 14:38         ` Daniel P. Berrange
2016-09-29 14:42           ` Tian, Kevin
2016-09-30  5:19             ` Kirti Wankhede
2016-10-03  8:20               ` Kirti Wankhede
2016-10-07  5:16                 ` Kirti Wankhede
2016-10-07 19:09                   ` Alex Williamson

Reply instructions:

You may reply publicly 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=20160928164952.29446151@t450s.home \
    --to=alex.williamson@redhat.com \
    --cc=ACurrid@nvidia.com \
    --cc=berrange@redhat.com \
    --cc=bjsdjshi@linux.vnet.ibm.com \
    --cc=cjia@nvidia.com \
    --cc=jike.song@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=kraxel@redhat.com \
    --cc=kwankhede@nvidia.com \
    --cc=libvir-list@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.