1. Patchew
  2. Libvirt
  3. View series

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

Michal Privoznik posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/bd2913042ace0558455a16e2ed8981a6f41133e2.1490106542.git.mprivozn@redhat.com
docs/formatdomaincaps.html.in  | 3 ++-
src/conf/domain_capabilities.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
[libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve
Posted by Michal Privoznik 7 years, 1 month ago 
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@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)
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve
Posted by Roman Bogorodskiy 7 years, 1 month ago 
  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@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.

Roman Bogorodskiy
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve
Posted by Michal Privoznik 7 years, 1 month ago 
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@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.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve
Posted by Roman Bogorodskiy 7 years ago 
  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@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?

Roman Bogorodskiy
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve
Posted by Michal Privoznik 7 years ago 
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@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain_capabilities: Don't report machine type for bhyve
Posted by Roman Bogorodskiy 7 years ago 
  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@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.