On Sun, Mar 26, 2017 at 03:21:04PM -0400, Laine Stump wrote: > On 03/22/2017 11:27 AM, Erik Skultety wrote: > > Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx> > > --- > > docs/formatdomain.html.in | 46 ++++++++++++++++++++++++++++++++++++++++------ > > 1 file changed, 40 insertions(+), 6 deletions(-) > > I always like to put the docs changes in the same patch that modifies > the schema and XML parsing functions, but that's not a hard rule, so I'm > fine with this. > > > > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > > index 4a3123e989..1eb6c44b6f 100644 > > --- a/docs/formatdomain.html.in > > +++ b/docs/formatdomain.html.in > > @@ -3886,6 +3886,19 @@ > > </devices> > > ...</pre> > > > > + <p>or:</p> > > + > > +<pre> > > + ... > > + <devices> > > + <hostdev mode='subsystem' type='mdev' model='vfio-pci'> > > + <source> > > + <address uuid='c2177883-f1bb-47f0-914d-32a22e3a8804'> > > + </source> > > + </hostdev> > > + </devices> > > + ...</pre> > > + > > <dl> > > <dt><code>hostdev</code></dt> > > <dd>The <code>hostdev</code> element is the main container for describing > > @@ -3930,12 +3943,22 @@ > > <code>type</code> passes all LUNs presented by a single HBA to > > the guest. > > </dd> > > + <dt><code>mdev</code></dt> > > + <dd>For mediated devices (<span class="since">Since 3.2.0</span>) > > + the <code>model</code> attribute specifies the device API which > > + determines how the host's vfio driver will expose the device to the > > + guest. Currently, only <code>vfio-pci</code> model is supported. > > > either "only the vfio-pci model is supported" or "only model='vfio-pci' > is supported". > > > > + There are also some implications on the usage of guest's address type > > + depending on the <code>model</code> attribute, see the > > + <code>address</code> element below.</dd> > > I'm not really comfortable with this - it means that the code that > parses <address> has to have information from a higher level rather than > being self contained. Remind me why you decided to remove "type='mdev'" > (or whatever it was) from the <address> syntax? Because we're kind of abusing the <address> element when using it within source. If you look at our docs [1] we define it only for guest addresses where we actually require the type, but when we use it under source, we define what kind of attributes are expected. Internally, we have an enum for each address type we know (which we also check only when assigning guest addresses) which means that I would have to add the enum for mdev and also document it under the 'Device Addresses' paragraph (which I did before), but the only thing I documented then was that it's not supposed to be used as a guest address, except that the paragraph headline indicates it's indeed about guest addresses. Do you see the confusion in that, i.e. so do we or do we actually not support mdev as a guest address?? (that was a rhetorical question...) Also, besides our schema not being happy about that, you can happily define a domain with an source address element containing some additional rubbish attributes which will be silently ignored by the parser. [1] http://libvirt.org/formatdomain.html#elementsAddress > > > > </dl> > > <p> > > - Note: The <code>managed</code> attribute is only used with PCI devices > > s/PCI/type='pci'/ > > > - and is ignored by all the other device types, thus setting > > - <code>managed</code> explicitly with other than PCI device has the same > > - effect as omitting it. > > + Note: The <code>managed</code> attribute is only used with PCI and is > > + ignored by all the other device types, thus setting > > + <code>managed</code> explicitly with other than a PCI device has the > > + same effect as omitting it. Similarly, <code>model</code> attribute is > > + only supported by mediated devices and ignored by all other device > > + types. > > Do we want to ignore it, or log an error if someone specifies it when > they shouldn't? Doing the latter would prevent someone from thinking > they've done "A" when actually they've done "B". As I wrote above, our parser is quite permissive in semantics and since usually the case is that we ignore everything that does not make sense for us and only error out if we got a value different from what we expected or we're missing a mandatory attribute in general or there is a conflict in attributes, etc. If someone then later complains that they have a different hostdev and expected the model to do something, we can still confront them with the documentation which states that it will be ignored for unsupported device types. > > I'm still unsure about having no type specifier in <address>. I know > that's already the case for PCI addresses in <source>, but I actually > think that was an incorrect decision - it's cleaner if we can > parse/format <address> without reaching up into higher levels. Maybe I > can be convinced though :-) See my response above > > In the meantime, I think it would be better to get this all in and get > some solid testing outside of the few people directly involved, so ACK > to this patch too (which makes the entire series ACKed (with a few small > changes requested), and we can have one last debate about <address> > during the next couple days. Thanks a lot for review, hopefully I didn't forget to adjust any of the patches and pushed the series. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
