[libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve

[Roman Bogorodskiy]

  Michal Privoznik wrote:

> On 03/26/2017 11:01 AM, Roman Bogorodskiy wrote:
> >   Michal Privoznik wrote:
> > 
> >> On 03/22/2017 06:46 AM, Roman Bogorodskiy wrote:
> >>>   Michal Privoznik wrote:
> >>>
> >>>> For some drivers the domain's machine type makes no sense. They
> >>>> just don't use it. A great example is bhyve driver. Therefore it
> >>>> makes very less sense to report machine in domain capabilities
> >>>> XML.
> >>>>
> >>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> >>>> ---
> >>>>  docs/formatdomaincaps.html.in  | 3 ++-
> >>>>  src/conf/domain_capabilities.c | 3 ++-
> >>>>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> >>>> index 648e3d481..007cab62d 100644
> >>>> --- a/docs/formatdomaincaps.html.in
> >>>> +++ b/docs/formatdomaincaps.html.in
> >>>> @@ -70,7 +70,8 @@
> >>>>
> >>>>        <dt><code>machine</code></dt>
> >>>>        <dd>The domain's <a href="formatdomain.html#elementsOSBIOS">machine
> >>>> -          type</a>.</dd>
> >>>> +          type</a>. Since not every hypervisor has a sense of machine types
> >>>> +          this element might be omitted in such drivers.</dd>
> >>>>
> >>>>        <dt><code>arch</code></dt>
> >>>>        <dd>The domain's <a href="formatdomain.html#elementsOSBIOS">
> >>>> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> >>>> index bb6742359..7a3d2e6fb 100644
> >>>> --- a/src/conf/domain_capabilities.c
> >>>> +++ b/src/conf/domain_capabilities.c
> >>>> @@ -527,7 +527,8 @@ virDomainCapsFormatInternal(virBufferPtr buf,
> >>>>
> >>>>      virBufferEscapeString(buf, "<path>%s</path>\n", caps->path);
> >>>>      virBufferAsprintf(buf, "<domain>%s</domain>\n", virttype_str);
> >>>> -    virBufferAsprintf(buf, "<machine>%s</machine>\n", caps->machine);
> >>>> +    if (caps->machine)
> >>>> +        virBufferAsprintf(buf, "<machine>%s</machine>\n", caps->machine);
> >>>>      virBufferAsprintf(buf, "<arch>%s</arch>\n", arch_str);
> >>>>
> >>>>      if (caps->maxvcpus)
> >>>
> >>> This looks reasonable to me, ACK.
> >>>
> >>
> >> Thank you pushed. Now you can regenerate the output of the test in your 
> >> patch and push too.
> > 
> > Just noticed that it requires schema changes too. Any objections me
> > squashing in a change like this:
> > 
> > --- a/docs/schemas/domaincaps.rng
> > +++ b/docs/schemas/domaincaps.rng
> > @@ -17,13 +17,15 @@
> >          <element name='domain'>
> >            <text/>
> >          </element>
> > -        <element name='machine'>
> > -          <text/>
> > -        </element>
> >          <element name='arch'>
> >            <text/>
> >          </element>
> >          <optional>
> > +          <element name='machine'>
> > +            <text/>
> > +          </element>
> > +        </optional>
> > +        <optional>
> >            <ref name='vcpu'/>
> >          </optional>
> >          <optional>
> > 
> > as an additional commit to the series?
> 
> Ah. sorry for that. This looks reasonable, although if you want you can
> leave the element at its original position in the RNG file and just wrap
> it in <optional/> (and adjust indentation ofc). That's the order in
> which we print out the capabilities XML anyway.
> 
> Yeah, it should go as a separate commit that you can just push (e.g. as
> trivial).
> 
> Michal

Pushed, thanks!

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170326/379e806f/attachment-0001.sig>