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

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(a)redhat.com&gt; >>>> --- >>>> 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