This patch drops the capability matching redundancy by simply converting
the string input to our internal types which are then in turn used for
the actual capability matching.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
src/conf/virnodedeviceobj.c | 50 +--------------------------------------------
1 file changed, 1 insertion(+), 49 deletions(-)
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ccad59a4b..5360df805 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -128,55 +128,7 @@ static bool
virNodeDeviceObjHasCapStr(const virNodeDeviceObj *obj,
const char *cap)
{
- virNodeDevCapsDefPtr caps = obj->def->caps;
- const char *fc_host_cap =
- virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
- const char *vports_cap =
- virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
- const char *mdev_types =
- virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES);
-
- while (caps) {
- if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type))) {
- return true;
- } else {
- switch (caps->data.type) {
- case VIR_NODE_DEV_CAP_PCI_DEV:
- if ((STREQ(cap, mdev_types)) &&
- (caps->data.pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV))
- return true;
- break;
-
- case VIR_NODE_DEV_CAP_SCSI_HOST:
- if ((STREQ(cap, fc_host_cap) &&
- (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
- (STREQ(cap, vports_cap) &&
- (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)))
- return true;
- break;
-
- case VIR_NODE_DEV_CAP_SYSTEM:
- case VIR_NODE_DEV_CAP_USB_DEV:
- case VIR_NODE_DEV_CAP_USB_INTERFACE:
- case VIR_NODE_DEV_CAP_NET:
- case VIR_NODE_DEV_CAP_SCSI_TARGET:
- case VIR_NODE_DEV_CAP_SCSI:
- case VIR_NODE_DEV_CAP_STORAGE:
- case VIR_NODE_DEV_CAP_FC_HOST:
- case VIR_NODE_DEV_CAP_VPORTS:
- case VIR_NODE_DEV_CAP_SCSI_GENERIC:
- case VIR_NODE_DEV_CAP_DRM:
- case VIR_NODE_DEV_CAP_MDEV_TYPES:
- case VIR_NODE_DEV_CAP_MDEV:
- case VIR_NODE_DEV_CAP_CCW_DEV:
- case VIR_NODE_DEV_CAP_LAST:
- break;
- }
- }
-
- caps = caps->next;
- }
- return false;
+ return virNodeDeviceObjHasCap(obj, virNodeDevCapTypeFromString(cap));
}
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/25/2018 10:23 AM, Erik Skultety wrote:
> This patch drops the capability matching redundancy by simply converting
> the string input to our internal types which are then in turn used for
> the actual capability matching.
>
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
> src/conf/virnodedeviceobj.c | 50 +--------------------------------------------
> 1 file changed, 1 insertion(+), 49 deletions(-)
>
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index ccad59a4b..5360df805 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -128,55 +128,7 @@ static bool
> virNodeDeviceObjHasCapStr(const virNodeDeviceObj *obj,
> const char *cap)
> {
> - virNodeDevCapsDefPtr caps = obj->def->caps;
> - const char *fc_host_cap =
> - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
> - const char *vports_cap =
> - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
> - const char *mdev_types =
> - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES);
> -
> - while (caps) {
> - if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type))) {
> - return true;
> - } else {
> - switch (caps->data.type) {
> - case VIR_NODE_DEV_CAP_PCI_DEV:
> - if ((STREQ(cap, mdev_types)) &&
> - (caps->data.pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV))
> - return true;
> - break;
> -
> - case VIR_NODE_DEV_CAP_SCSI_HOST:
> - if ((STREQ(cap, fc_host_cap) &&
> - (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
> - (STREQ(cap, vports_cap) &&
> - (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)))
> - return true;
> - break;
> -
> - case VIR_NODE_DEV_CAP_SYSTEM:
> - case VIR_NODE_DEV_CAP_USB_DEV:
> - case VIR_NODE_DEV_CAP_USB_INTERFACE:
> - case VIR_NODE_DEV_CAP_NET:
> - case VIR_NODE_DEV_CAP_SCSI_TARGET:
> - case VIR_NODE_DEV_CAP_SCSI:
> - case VIR_NODE_DEV_CAP_STORAGE:
> - case VIR_NODE_DEV_CAP_FC_HOST:
> - case VIR_NODE_DEV_CAP_VPORTS:
> - case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> - case VIR_NODE_DEV_CAP_DRM:
> - case VIR_NODE_DEV_CAP_MDEV_TYPES:
> - case VIR_NODE_DEV_CAP_MDEV:
> - case VIR_NODE_DEV_CAP_CCW_DEV:
> - case VIR_NODE_DEV_CAP_LAST:
> - break;
> - }
> - }
> -
> - caps = caps->next;
> - }
> - return false;
> + return virNodeDeviceObjHasCap(obj, virNodeDevCapTypeFromString(cap));
I wonder if we should check for the TypeFromString() conversion rather
than pass it to the other function directly.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jan 26, 2018 at 12:40:34PM +0100, Michal Privoznik wrote:
> On 01/25/2018 10:23 AM, Erik Skultety wrote:
> > This patch drops the capability matching redundancy by simply converting
> > the string input to our internal types which are then in turn used for
> > the actual capability matching.
> >
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> > src/conf/virnodedeviceobj.c | 50 +--------------------------------------------
> > 1 file changed, 1 insertion(+), 49 deletions(-)
> >
> > diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> > index ccad59a4b..5360df805 100644
> > --- a/src/conf/virnodedeviceobj.c
> > +++ b/src/conf/virnodedeviceobj.c
> > @@ -128,55 +128,7 @@ static bool
> > virNodeDeviceObjHasCapStr(const virNodeDeviceObj *obj,
> > const char *cap)
> > {
> > - virNodeDevCapsDefPtr caps = obj->def->caps;
> > - const char *fc_host_cap =
> > - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
> > - const char *vports_cap =
> > - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
> > - const char *mdev_types =
> > - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES);
> > -
> > - while (caps) {
> > - if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type))) {
> > - return true;
> > - } else {
> > - switch (caps->data.type) {
> > - case VIR_NODE_DEV_CAP_PCI_DEV:
> > - if ((STREQ(cap, mdev_types)) &&
> > - (caps->data.pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV))
> > - return true;
> > - break;
> > -
> > - case VIR_NODE_DEV_CAP_SCSI_HOST:
> > - if ((STREQ(cap, fc_host_cap) &&
> > - (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
> > - (STREQ(cap, vports_cap) &&
> > - (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)))
> > - return true;
> > - break;
> > -
> > - case VIR_NODE_DEV_CAP_SYSTEM:
> > - case VIR_NODE_DEV_CAP_USB_DEV:
> > - case VIR_NODE_DEV_CAP_USB_INTERFACE:
> > - case VIR_NODE_DEV_CAP_NET:
> > - case VIR_NODE_DEV_CAP_SCSI_TARGET:
> > - case VIR_NODE_DEV_CAP_SCSI:
> > - case VIR_NODE_DEV_CAP_STORAGE:
> > - case VIR_NODE_DEV_CAP_FC_HOST:
> > - case VIR_NODE_DEV_CAP_VPORTS:
> > - case VIR_NODE_DEV_CAP_SCSI_GENERIC:
> > - case VIR_NODE_DEV_CAP_DRM:
> > - case VIR_NODE_DEV_CAP_MDEV_TYPES:
> > - case VIR_NODE_DEV_CAP_MDEV:
> > - case VIR_NODE_DEV_CAP_CCW_DEV:
> > - case VIR_NODE_DEV_CAP_LAST:
> > - break;
> > - }
> > - }
> > -
> > - caps = caps->next;
> > - }
> > - return false;
> > + return virNodeDeviceObjHasCap(obj, virNodeDevCapTypeFromString(cap));
>
> I wonder if we should check for the TypeFromString() conversion rather
> than pass it to the other function directly.
Well, since the conversion function returns -1 on unknown types and none of our
device cap enum types can ever be equal to -1, since that would make it
non-deterministic, but I agree that by adding a check explicitly we can save a
few cycles, may I assume this to be an ACK with the following squashed in?
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 2f37c4a05..ccea10793 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -124,7 +124,12 @@ static bool
virNodeDeviceObjHasCapStr(const virNodeDeviceObj *obj,
const char *cap)
{
- return virNodeDeviceObjHasCap(obj, virNodeDevCapTypeFromString(cap));
+ int type;
+
+ if ((type = virNodeDevCapTypeFromString(cap)) < 0)
+ return false;
+
+ return virNodeDeviceObjHasCap(obj, type);
}
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/29/2018 09:42 AM, Erik Skultety wrote:
> On Fri, Jan 26, 2018 at 12:40:34PM +0100, Michal Privoznik wrote:
>> On 01/25/2018 10:23 AM, Erik Skultety wrote:
>>> This patch drops the capability matching redundancy by simply converting
>>> the string input to our internal types which are then in turn used for
>>> the actual capability matching.
>>>
>>> Signed-off-by: Erik Skultety <eskultet@redhat.com>
>>> ---
>>> src/conf/virnodedeviceobj.c | 50 +--------------------------------------------
>>> 1 file changed, 1 insertion(+), 49 deletions(-)
>>>
>>> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
>>> index ccad59a4b..5360df805 100644
>>> --- a/src/conf/virnodedeviceobj.c
>>> +++ b/src/conf/virnodedeviceobj.c
>>> @@ -128,55 +128,7 @@ static bool
>>> virNodeDeviceObjHasCapStr(const virNodeDeviceObj *obj,
>>> const char *cap)
>>> {
>>> - virNodeDevCapsDefPtr caps = obj->def->caps;
>>> - const char *fc_host_cap =
>>> - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
>>> - const char *vports_cap =
>>> - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
>>> - const char *mdev_types =
>>> - virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES);
>>> -
>>> - while (caps) {
>>> - if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type))) {
>>> - return true;
>>> - } else {
>>> - switch (caps->data.type) {
>>> - case VIR_NODE_DEV_CAP_PCI_DEV:
>>> - if ((STREQ(cap, mdev_types)) &&
>>> - (caps->data.pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV))
>>> - return true;
>>> - break;
>>> -
>>> - case VIR_NODE_DEV_CAP_SCSI_HOST:
>>> - if ((STREQ(cap, fc_host_cap) &&
>>> - (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
>>> - (STREQ(cap, vports_cap) &&
>>> - (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)))
>>> - return true;
>>> - break;
>>> -
>>> - case VIR_NODE_DEV_CAP_SYSTEM:
>>> - case VIR_NODE_DEV_CAP_USB_DEV:
>>> - case VIR_NODE_DEV_CAP_USB_INTERFACE:
>>> - case VIR_NODE_DEV_CAP_NET:
>>> - case VIR_NODE_DEV_CAP_SCSI_TARGET:
>>> - case VIR_NODE_DEV_CAP_SCSI:
>>> - case VIR_NODE_DEV_CAP_STORAGE:
>>> - case VIR_NODE_DEV_CAP_FC_HOST:
>>> - case VIR_NODE_DEV_CAP_VPORTS:
>>> - case VIR_NODE_DEV_CAP_SCSI_GENERIC:
>>> - case VIR_NODE_DEV_CAP_DRM:
>>> - case VIR_NODE_DEV_CAP_MDEV_TYPES:
>>> - case VIR_NODE_DEV_CAP_MDEV:
>>> - case VIR_NODE_DEV_CAP_CCW_DEV:
>>> - case VIR_NODE_DEV_CAP_LAST:
>>> - break;
>>> - }
>>> - }
>>> -
>>> - caps = caps->next;
>>> - }
>>> - return false;
>>> + return virNodeDeviceObjHasCap(obj, virNodeDevCapTypeFromString(cap));
>>
>> I wonder if we should check for the TypeFromString() conversion rather
>> than pass it to the other function directly.
>
> Well, since the conversion function returns -1 on unknown types and none of our
> device cap enum types can ever be equal to -1, since that would make it
> non-deterministic, but I agree that by adding a check explicitly we can save a
> few cycles, may I assume this to be an ACK with the following squashed in?
>
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index 2f37c4a05..ccea10793 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -124,7 +124,12 @@ static bool
> virNodeDeviceObjHasCapStr(const virNodeDeviceObj *obj,
> const char *cap)
> {
> - return virNodeDeviceObjHasCap(obj, virNodeDevCapTypeFromString(cap));
> + int type;
> +
> + if ((type = virNodeDevCapTypeFromString(cap)) < 0)
> + return false;
> +
> + return virNodeDeviceObjHasCap(obj, type);
> }
>
Yup, looks good to me.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.