[PATCH 06/33] qemu_validate.c: enhance 'machine type not supported' message

Daniel Henrique Barboza posted 33 patches 4 years ago
There is a newer version of this series
[PATCH 06/33] qemu_validate.c: enhance 'machine type not supported' message
Posted by Daniel Henrique Barboza 4 years ago
Add 'virt type' to allow for an easier time debugging.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/qemu/qemu_validate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index da41848761..95af0ecf3b 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1101,8 +1101,10 @@ qemuValidateDomainDef(const virDomainDef *def,
 
     if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("Emulator '%s' does not support machine type '%s'"),
-                       def->emulator, def->os.machine);
+                       _("Emulator '%s' does not support machine type '%s' "
+                         "for virt type '%s'"),
+                       def->emulator, def->os.machine,
+                       virDomainVirtTypeToString(def->virtType));
         return -1;
     }
 
-- 
2.34.1

Re: [PATCH 06/33] qemu_validate.c: enhance 'machine type not supported' message
Posted by Peter Krempa 4 years ago
On Thu, Jan 20, 2022 at 10:52:09 -0300, Daniel Henrique Barboza wrote:
> Add 'virt type' to allow for an easier time debugging.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  src/qemu/qemu_validate.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index da41848761..95af0ecf3b 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1101,8 +1101,10 @@ qemuValidateDomainDef(const virDomainDef *def,
>  
>      if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Emulator '%s' does not support machine type '%s'"),
> -                       def->emulator, def->os.machine);
> +                       _("Emulator '%s' does not support machine type '%s' "
> +                         "for virt type '%s'"),

No linebreak in the error message please.

https://www.libvirt.org/coding-style.html#error-message-format

> +                       def->emulator, def->os.machine,
> +                       virDomainVirtTypeToString(def->virtType));

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [PATCH 06/33] qemu_validate.c: enhance 'machine type not supported' message
Posted by Daniel Henrique Barboza 4 years ago

On 1/21/22 11:02, Peter Krempa wrote:
> On Thu, Jan 20, 2022 at 10:52:09 -0300, Daniel Henrique Barboza wrote:
>> Add 'virt type' to allow for an easier time debugging.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   src/qemu/qemu_validate.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index da41848761..95af0ecf3b 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -1101,8 +1101,10 @@ qemuValidateDomainDef(const virDomainDef *def,
>>   
>>       if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
>>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> -                       _("Emulator '%s' does not support machine type '%s'"),
>> -                       def->emulator, def->os.machine);
>> +                       _("Emulator '%s' does not support machine type '%s' "
>> +                         "for virt type '%s'"),
> 
> No linebreak in the error message please.
> 
> https://www.libvirt.org/coding-style.html#error-message-format

Is this acceptable?

      if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("Emulator '%s' does not support machine type '%s'"),
-                       def->emulator, def->os.machine);
+_("Emulator '%s' does not support machine type '%s' for virt type '%s'"),
+                       def->emulator, def->os.machine,
+                       virDomainVirtTypeToString(def->virtType));
          return -1;
      }


i.e. ignoring the identation of the error messsage to avoid going beyond the 80 char limit?



Thanks,


Daniel

> 
>> +                       def->emulator, def->os.machine,
>> +                       virDomainVirtTypeToString(def->virtType));
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> 

Re: [PATCH 06/33] qemu_validate.c: enhance 'machine type not supported' message
Posted by Peter Krempa 4 years ago
On Mon, Jan 24, 2022 at 08:41:38 -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 1/21/22 11:02, Peter Krempa wrote:
> > On Thu, Jan 20, 2022 at 10:52:09 -0300, Daniel Henrique Barboza wrote:
> > > Add 'virt type' to allow for an easier time debugging.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > > ---
> > >   src/qemu/qemu_validate.c | 6 ++++--
> > >   1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> > > index da41848761..95af0ecf3b 100644
> > > --- a/src/qemu/qemu_validate.c
> > > +++ b/src/qemu/qemu_validate.c
> > > @@ -1101,8 +1101,10 @@ qemuValidateDomainDef(const virDomainDef *def,
> > >       if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
> > >           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > -                       _("Emulator '%s' does not support machine type '%s'"),
> > > -                       def->emulator, def->os.machine);
> > > +                       _("Emulator '%s' does not support machine type '%s' "
> > > +                         "for virt type '%s'"),
> > 
> > No linebreak in the error message please.
> > 
> > https://www.libvirt.org/coding-style.html#error-message-format
> 
> Is this acceptable?
> 
>      if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Emulator '%s' does not support machine type '%s'"),
> -                       def->emulator, def->os.machine);
> +_("Emulator '%s' does not support machine type '%s' for virt type '%s'"),
> +                       def->emulator, def->os.machine,
> +                       virDomainVirtTypeToString(def->virtType));
>          return -1;
>      }
> 
> 
> i.e. ignoring the identation of the error messsage to avoid going beyond the 80 char limit?

No. The correct way is to ignore the 80 char line length limit exactly
as the coding style document says.

Re: [PATCH 06/33] qemu_validate.c: enhance 'machine type not supported' message
Posted by Daniel Henrique Barboza 4 years ago

On 1/24/22 08:43, Peter Krempa wrote:
> On Mon, Jan 24, 2022 at 08:41:38 -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 1/21/22 11:02, Peter Krempa wrote:
>>> On Thu, Jan 20, 2022 at 10:52:09 -0300, Daniel Henrique Barboza wrote:
>>>> Add 'virt type' to allow for an easier time debugging.
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>>> ---
>>>>    src/qemu/qemu_validate.c | 6 ++++--
>>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>>>> index da41848761..95af0ecf3b 100644
>>>> --- a/src/qemu/qemu_validate.c
>>>> +++ b/src/qemu/qemu_validate.c
>>>> @@ -1101,8 +1101,10 @@ qemuValidateDomainDef(const virDomainDef *def,
>>>>        if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
>>>>            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>>> -                       _("Emulator '%s' does not support machine type '%s'"),
>>>> -                       def->emulator, def->os.machine);
>>>> +                       _("Emulator '%s' does not support machine type '%s' "
>>>> +                         "for virt type '%s'"),
>>>
>>> No linebreak in the error message please.
>>>
>>> https://www.libvirt.org/coding-style.html#error-message-format
>>
>> Is this acceptable?
>>
>>       if (!virQEMUCapsIsMachineSupported(qemuCaps, def->virtType, def->os.machine)) {
>>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> -                       _("Emulator '%s' does not support machine type '%s'"),
>> -                       def->emulator, def->os.machine);
>> +_("Emulator '%s' does not support machine type '%s' for virt type '%s'"),
>> +                       def->emulator, def->os.machine,
>> +                       virDomainVirtTypeToString(def->virtType));
>>           return -1;
>>       }
>>
>>
>> i.e. ignoring the identation of the error messsage to avoid going beyond the 80 char limit?
> 
> No. The correct way is to ignore the 80 char line length limit exactly
> as the coding style document says.

Got it. Amended with identation.


Thanks,


Daniel


>