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

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

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

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,

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

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
participants (2)
-
Chen Hanxiao
-
John Ferlan