[libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages

Chen Hanxiao posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1501230797-16685-1-git-send-email-chen_han_xiao@126.com
src/util/virhostdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages
Posted by Chen Hanxiao 6 years, 8 months ago
From: Chen Hanxiao <chenhanxiao@gmail.com>

    Many vendor id's and product id's have leading zeros.
    Show them in error messages.

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
 src/util/virhostdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 579563c..0e6b5a3 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1390,7 +1390,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
         } else if (!autoAddress) {
             goto out;
         } else {
-            VIR_INFO("USB device %x:%x could not be found at previous"
+            VIR_INFO("USB device %04x:%04x could not be found at previous"
                      " address (bus:%u device:%u)",
                      vendor, product, bus, device);
         }
@@ -1418,12 +1418,12 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
         } else if (rc > 1) {
             if (autoAddress) {
                 virReportError(VIR_ERR_OPERATION_FAILED,
-                               _("Multiple USB devices for %x:%x were found,"
+                               _("Multiple USB devices for %04x:%04x were found,"
                                  " but none of them is at bus:%u device:%u"),
                                vendor, product, bus, device);
             } else {
                 virReportError(VIR_ERR_OPERATION_FAILED,
-                               _("Multiple USB devices for %x:%x, "
+                               _("Multiple USB devices for %04x:%04x, "
                                  "use <address> to specify one"),
                                vendor, product);
             }
@@ -1435,7 +1435,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
         usbsrc->autoAddress = true;
 
         if (autoAddress) {
-            VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
+            VIR_INFO("USB device %04x:%04x found at bus:%u device:%u (moved"
                      " from bus:%u device:%u)",
                      vendor, product,
                      usbsrc->bus, usbsrc->device,
-- 
2.7.4


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages
Posted by Chen Hanxiao 6 years, 8 months ago
At 2017-07-28 16:33:17, "Chen Hanxiao" <chen_han_xiao@126.com> wrote:
>From: Chen Hanxiao <chenhanxiao@gmail.com>
>
>    Many vendor id's and product id's have leading zeros.
>    Show them in error messages.
>
>Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>---

ping

Regards,
- Chen

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages
Posted by John Ferlan 6 years, 8 months ago

On 07/28/2017 04:33 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@gmail.com>
> 
>     Many vendor id's and product id's have leading zeros.
>     Show them in error messages.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
> ---
>  src/util/virhostdev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Looking at some other examples...

    if (usbsrc->vendor) {
        virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor);
        virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product);

and

    if (usbdev->vendor >= 0)
        virBufferAsprintf(buf, " vendor='0x%04X'", usbdev->vendor);

    if (usbdev->product >= 0)
        virBufferAsprintf(buf, " product='0x%04X'", usbdev->product);

Perhaps the best thing to do is be consistent with all of them...  Could
take a bit of searching, but cscope's egrep is pretty good w/
"vendor.*%.*x" (and X).

There's also a usage in libxl_conf, where "%x:%x" is used. So it may be
best to find all possible print's of vendor and make them all consistent.

John

> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 579563c..0e6b5a3 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -1390,7 +1390,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
>          } else if (!autoAddress) {
>              goto out;
>          } else {
> -            VIR_INFO("USB device %x:%x could not be found at previous"
> +            VIR_INFO("USB device %04x:%04x could not be found at previous"
>                       " address (bus:%u device:%u)",
>                       vendor, product, bus, device);
>          }
> @@ -1418,12 +1418,12 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
>          } else if (rc > 1) {
>              if (autoAddress) {
>                  virReportError(VIR_ERR_OPERATION_FAILED,
> -                               _("Multiple USB devices for %x:%x were found,"
> +                               _("Multiple USB devices for %04x:%04x were found,"
>                                   " but none of them is at bus:%u device:%u"),
>                                 vendor, product, bus, device);
>              } else {
>                  virReportError(VIR_ERR_OPERATION_FAILED,
> -                               _("Multiple USB devices for %x:%x, "
> +                               _("Multiple USB devices for %04x:%04x, "
>                                   "use <address> to specify one"),
>                                 vendor, product);
>              }
> @@ -1435,7 +1435,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
>          usbsrc->autoAddress = true;
>  
>          if (autoAddress) {
> -            VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
> +            VIR_INFO("USB device %04x:%04x found at bus:%u device:%u (moved"
>                       " from bus:%u device:%u)",
>                       vendor, product,
>                       usbsrc->bus, usbsrc->device,
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages
Posted by Chen Hanxiao 6 years, 8 months ago
At 2017-08-03 08:40:48, "John Ferlan" <jferlan@redhat.com> wrote:
>
>
>On 07/28/2017 04:33 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao <chenhanxiao@gmail.com>
>> 
>>     Many vendor id's and product id's have leading zeros.
>>     Show them in error messages.
>> 
>> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>> ---
>>  src/util/virhostdev.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>
>Looking at some other examples...
>
>    if (usbsrc->vendor) {
>        virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor);
>        virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product);
>
>and
>
>    if (usbdev->vendor >= 0)
>        virBufferAsprintf(buf, " vendor='0x%04X'", usbdev->vendor);
>
>    if (usbdev->product >= 0)
>        virBufferAsprintf(buf, " product='0x%04X'", usbdev->product);
>
>Perhaps the best thing to do is be consistent with all of them...  Could
>take a bit of searching, but cscope's egrep is pretty good w/
>"vendor.*%.*x" (and X).
>
>There's also a usage in libxl_conf, where "%x:%x" is used. So it may be
>best to find all possible print's of vendor and make them all consistent.
>

The %x:%x should be fixed.

x or X just show a different style.
Others like .4x, 04x have the same effect.
Maybe we should leave them untouched.

Regards,
- Chen

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages
Posted by John Ferlan 6 years, 8 months ago

On 08/07/2017 10:16 PM, Chen Hanxiao wrote:
> At 2017-08-03 08:40:48, "John Ferlan" <jferlan@redhat.com> wrote:
>>
>>
>> On 07/28/2017 04:33 AM, Chen Hanxiao wrote:
>>> From: Chen Hanxiao <chenhanxiao@gmail.com>
>>>
>>>     Many vendor id's and product id's have leading zeros.
>>>     Show them in error messages.
>>>
>>> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>>> ---
>>>  src/util/virhostdev.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>
>> Looking at some other examples...
>>
>>    if (usbsrc->vendor) {
>>        virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor);
>>        virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product);
>>
>> and
>>
>>    if (usbdev->vendor >= 0)
>>        virBufferAsprintf(buf, " vendor='0x%04X'", usbdev->vendor);
>>
>>    if (usbdev->product >= 0)
>>        virBufferAsprintf(buf, " product='0x%04X'", usbdev->product);
>>
>> Perhaps the best thing to do is be consistent with all of them...  Could
>> take a bit of searching, but cscope's egrep is pretty good w/
>> "vendor.*%.*x" (and X).
>>
>> There's also a usage in libxl_conf, where "%x:%x" is used. So it may be
>> best to find all possible print's of vendor and make them all consistent.
>>
> 
> The %x:%x should be fixed.
> 
> x or X just show a different style.
> Others like .4x, 04x have the same effect.
> Maybe we should leave them untouched.
> 
I think if you're modifying one then we should make them all consistent.
It leaves less questions that way.  IIRC the last time this came up I
think using %.4x was preferred (or X if you care about capital letters).

John

> Regards,
> - Chen
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list