Piotr Kliczewski has uploaded a new change for review.
Change subject: schema: customizable hook name ......................................................................
schema: customizable hook name
getCaps response contains hook names which by design can be any string. We need to support following naming scheme in validation.
Change-Id: Icb20039ef1d3bba32518dd23e6b30506cb77e0ec Signed-off-by: Piotr Kliczewski piotr.kliczewski@gmail.com --- M lib/api/vdsm-api.yml M lib/api/vdsmapi.py M tests/vdsmapi_test.py 3 files changed, 224 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/59251/1
diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml index b537e8e..87efa86 100644 --- a/lib/api/vdsm-api.yml +++ b/lib/api/vdsm-api.yml @@ -6,9 +6,9 @@ name: AutoNumaBalancingStatus type: enum values: - DISABLE: Auto numa balancing is not active - ENABLE: Auto numa balancing is active - UNKNOWN: Can't get the status(maybe not support in this kernel + 0: Auto numa balancing is not active + 1: Auto numa balancing is active + 2: Can't get the status(maybe not support in this kernel version)
BalloonInfo: &BalloonInfo @@ -1390,7 +1390,7 @@ name: HookScriptAdditional properties: - description: A key for hook script information - name: 50_vmfex + name: custom_name type: *HookScriptInfo type: object
@@ -4190,7 +4190,8 @@ - description: The Maximum Transmission Unit size for the bridge device name: mtu - type: uint + type: string + datatype: uint
- description: The default IPv6 gateway for the bridge device @@ -4226,15 +4227,16 @@ - description: Whether Spanning Tree Protocol status for the bridge device should be turned on name: stp - type: boolean + type: string
- - description: A map containing information about the + - defaultvalue: no-default + description: A map containing information about the bridge specific options. name: opts type: *StringMap added: '3.4'
- - defaultvalue: needs updating + - defaultvalue: no-default description: host traffic QoS params. name: hostQos type: *HostQoSParams @@ -4247,7 +4249,8 @@ - string added: '3.4'
- - description: Always True to distinguish this type + - defaultvalue: no-default + description: Always True to distinguish this type from NetInfoBridgelessNetwork name: bridged type: boolean @@ -4256,6 +4259,21 @@ the bridge device name: addr type: string + + - defaultvalue: no-default + description: The network device associated with the network + name: iface + type: string + + - defaultvalue: no-default + description: Specify Switch. Legacy is used if not passed + name: switch + type: string + + - defaultvalue: no-default + description: Whether use stateless autoconfiguration + name: ipv6autoconf + type: boolean type: object
NetInfoBridgeMap: &NetInfoBridgeMap @@ -5393,7 +5411,12 @@ name: SoftwarePackage type: enum values: + glusterfs: Glusterfs binary and glusterfs translator modules common glusterfs-cli: GlusterFS a distributed file-system cli + glusterfs-geo-replication: Glusterfs geo-replication support + glusterfs-fuse: FUSE based clients and glusterfsd + glusterfs-rdma: Supports to ib-verbs library neede by Glusterfs + glusterfs-server: Glusterfs server daemon kernel: The Linux kernel librbd1: RADOS block device client library libvirt: Low level virtualization API diff --git a/lib/api/vdsmapi.py b/lib/api/vdsmapi.py index 36d28e2..8cd4d69 100644 --- a/lib/api/vdsmapi.py +++ b/lib/api/vdsmapi.py @@ -297,6 +297,8 @@ unknown_props = [key for key in arg if key not in prop_names] if unknown_props: + if 'custom_name' in prop_names: + return self._report_inconsistency('Following parameters %s were not' ' recognized' % (unknown_props)) # iterate over properties diff --git a/tests/vdsmapi_test.py b/tests/vdsmapi_test.py index f80a27c..8599990 100644 --- a/tests/vdsmapi_test.py +++ b/tests/vdsmapi_test.py @@ -479,3 +479,193 @@ sub_id = '|virt|VM_status|426aef82-ea1d-4442-91d3-fd876540e0f0'
_events_schema.events_schema().verify_event_params(sub_id, params) + + def test_get_caps(self): + ret = {'HBAInventory': {'iSCSI': [{'InitiatorName': 'iqn.1994-05.co'}], + 'FC': []}, + 'packages2': {'kernel': {'release': '201.fc23.x86_64', + 'buildtime': 1463837389.0, + 'version': '4.5.5'}, + 'glusterfs-rdma': {'release': '1.fc23', + 'buildtime': 1460984315, + 'version': '3.7.11'}, + 'glusterfs-fuse': {'release': '1.fc23', + 'buildtime': 1460984315, + 'version': '3.7.11'}, + 'spice-server': {'release': '1.fc23', + 'buildtime': 1444215420, + 'version': '0.12.6'}, + 'librbd1': {'release': '2.fc23', + 'buildtime': 1463433911, + 'version': '0.94.7'}, + 'vdsm': {'release': '73.git2105bb3.fc23', + 'buildtime': 1465561223, + 'version': '4.18.999'}, + 'qemu-kvm': {'release': '10.fc23', + 'buildtime': 1464278628, + 'version': '2.4.1'}, + 'glusterfs': {'release': '1.fc23', + 'buildtime': 1460984315, + 'version': '3.7.11'}, + 'libvirt': {'release': '1.fc23', + 'buildtime': 1462407395, + 'version': '1.2.18.3'}, + 'qemu-img': {'release': '10.fc23', + 'buildtime': 1464278628, + 'version': '2.4.1'}, + 'mom': {'release': '1.fc23', + 'buildtime': 1464783301, + 'version': '0.5.4'}, + 'glusterfs-geo-replication': {'release': '1.fc23', + 'buildtime': 14605, + 'version': '3.7.1'}, + 'glusterfs-server': {'release': '1.fc23', + 'buildtime': 1460984315, + 'version': '3.7.11'}, + 'glusterfs-cli': {'release': '1.fc23', + 'buildtime': 1460984315, + 'version': '3.7.11'}}, + 'numaNodeDistance': {'0': [10]}, + 'cpuModel': 'Intel(R) Core(TM) i7-3770 CPU @ 3.40GHz', + 'liveMerge': 'true', + 'hooks': {'before_nic_hotplug': + {'50_vmfex': {'md5': 'e05994261acaea7dcf4b88ea'}}, + 'before_device_migrate_destination': + {'50_vmfex': {'md5': 'e05994261acaea7dcf4b88ea'}}, + 'before_device_create': + {'50_vmfex': {'md5': 'e05994261acaea7dcf4b88ea'}}, + 'my_custom_hook': + {'my_name.py': {'md5': 'e05994261acaea7dcf4b88ea'}}}, + 'supportsIPv6': True, + 'vmTypes': ['kvm'], + 'selinux': {'mode': '1'}, + 'liveSnapshot': 'true', + 'kdumpStatus': 0, + 'networks': {'ovirtmgmt': {'iface': 'ovirtmgmt', + 'ipv4addrs': ['192.168.1.106/24'], + 'addr': '192.168.1.106', + 'ipv6addrs': [], + 'netmask': '255.255.255.0', + 'mtu': '1500', + 'dhcpv4': True, + 'switch': 'legacy', + 'dhcpv6': False, + 'stp': 'off', + 'bridged': True, + 'ipv6gateway': '::', + 'gateway': '192.168.1.1', + 'ports': ['eno1']}}, + 'kernelArgs': 'BOOT_IMAGE=/vmlinuz-4.5.5-201.fc23.x86_64 ro', + 'bridges': {'ovirtmgmt': + {'ipv6autoconf': True, + 'addr': '192.168.1.106', + 'ipv6addrs': [], + 'mtu': '1500', + 'dhcpv4': True, + 'netmask': '255.255.255.0', + 'dhcpv6': False, + 'stp': 'off', + 'ipv4addrs': ['192.168.1.106/24'], + 'ipv6gateway': '::', + 'gateway': '192.168.1.1', + 'opts': + {'multicast_last_member_count': '2', + 'vlan_protocol': '0x8100', + 'hash_elasticity': '4', + 'multicast_query_response_interval': '1000', + 'group_fwd_mask': '0x0', + 'multicast_snooping': '1', + 'multicast_startup_query_interval': '3125', + 'hello_timer': '0', + 'multicast_querier_interval': '25500', + 'max_age': '2000', + 'hash_max': '512', + 'stp_state': '0', + 'topology_change_detected': '0', + 'priority': '32768', + 'multicast_membership_interval': '26000', + 'root_path_cost': '0', + 'root_port': '0', + 'multicast_querier': '0', + 'multicast_startup_query_count': '2', + 'nf_call_iptables': '0', + 'hello_time': '200', + 'topology_change': '0', + 'bridge_id': '8000.b8ca3aa977e2', + 'topology_change_timer': '0', + 'ageing_time': '30000', + 'nf_call_ip6tables': '0', + 'gc_timer': '2191', + 'root_id': '8000.b8ca3aa977e2', + 'nf_call_arptables': '0', + 'group_addr': '1:80:c2:0:0:0', + 'multicast_last_member_interval': '100', + 'default_pvid': '1', + 'multicast_query_interval': '12500', + 'multicast_query_use_ifaddr': '0', + 'tcn_timer': '0', + 'multicast_router': '1', + 'vlan_filtering': '0', + 'forward_delay': '0'}, + 'ports': ['eno1']}}, + 'uuid': '4C4C4544-0046-4E10-8032-B2C04F385A31', + 'onlineCpus': '0,1,2,3,4,5,6,7', + 'dnss': ['192.168.1.1'], + 'nics': {'eno1': {'ipv6autoconf': False, + 'addr': '', + 'speed': 1000, + 'ipv6addrs': [], + 'mtu': '1500', + 'dhcpv4': False, + 'netmask': '', + 'dhcpv6': False, + 'ipv4addrs': [], + 'hwaddr': 'b8:ca:3a:a9:77:e2', + 'ipv6gateway': '::', + 'gateway': ''}}, + 'software_revision': '73', + 'hostdevPassthrough': 'false', + 'clusterLevels': ['3.5', '3.6', '4.0'], + 'cpuFlags': 'fpu,vme,de,pse,tsc,msr,pae,mce,cx8,apic,sep', + 'ISCSIInitiatorName': 'iqn.1994-05.com.redhat:7d366003913', + 'netConfigDirty': 'False', + 'supportedENGINEs': ['3.5', '3.6', '4.0'], + 'autoNumaBalancing': 0, + 'additionalFeatures': ['GLUSTER_SNAPSHOT', 'GLUSTER_GEO_RE'], + 'reservedMem': '321', + 'bondings': {'bond0': {'ipv6autoconf': True, + 'addr': '', + 'ipv6addrs': [], + 'switch': 'legacy', + 'active_slave': '', + 'mtu': '1500', + 'dhcpv4': False, + 'netmask': '', + 'dhcpv6': False, + 'ipv4addrs': [], + 'hwaddr': '3a:02:ff:17:ac:74', + 'slaves': [], + 'ipv6gateway': '::', + 'gateway': '', + 'opts': {'mode': '0'}}}, + 'software_version': '4.18', + 'memSize': '15934', + 'cpuSpeed': '1600.125', + 'numaNodes': {'0': {'totalMemory': '15934', + 'cpus': [0, 1, 2, 3, 4, 5, 6, 7]}}, + 'cpuSockets': '1', + 'vlans': {}, + 'lastClientIface': 'ovirtmgmt', + 'cpuCores': '4', + 'kvmEnabled': 'true', + 'guestOverhead': '65', + 'version_name': 'Snow Man', + 'cpuThreads': '8', + 'emulatedMachines': ['pc-q35-2.0', 'pc-q35-2.1'], + 'rngSources': ['hwrng', 'random'], + 'operatingSystem': {'release': '1', + 'version': '23', + 'name': 'Fedora'}} + + _schema.schema().verify_retval( + vdsmapi.MethodRep('Host', 'getCapabilities'), ret)
gerrit-hooks has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Piotr Kliczewski has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 1: Verified+1
The changed covered by unit test. Verification was done during build process.
gerrit-hooks has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/59251/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
Line 4228: for the bridge device should be turned on Line 4229: name: stp Line 4230: type: string Line 4231: Line 4232: - defaultvalue: no-default What does this mean? if there is not default, why do we need a defaultvalue key?
Maybe we are missing optional: true? or required: false? Line 4233: description: A map containing information about the Line 4234: bridge specific options. Line 4235: name: opts Line 4236: type: *StringMap
Piotr Kliczewski has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/59251/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
Line 4228: for the bridge device should be turned on Line 4229: name: stp Line 4230: type: string Line 4231: Line 4232: - defaultvalue: no-default
What does this mean? if there is not default, why do we need a defaultvalue
We need the keep to know that it is optional. If we add additional key we make the logic more complicated.
no-default is used when we want to mark that there is no default value and it is different than null/None. Line 4233: description: A map containing information about the Line 4234: bridge specific options. Line 4235: name: opts Line 4236: type: *StringMap
Nir Soffer has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/59251/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
Line 4228: for the bridge device should be turned on Line 4229: name: stp Line 4230: type: string Line 4231: Line 4232: - defaultvalue: no-default
We need the keep to know that it is optional. If we add additional key we m
This is bad, we will have to improve this.
The fact that an option is required or optional is critical, having a default value in the schema is not needed at all.
Not related to this patch of course, just make it harder to understand the changes. Line 4233: description: A map containing information about the Line 4234: bridge specific options. Line 4235: name: opts Line 4236: type: *StringMap
Nir Soffer has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/59251/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
Line 7: type: enum Line 8: values: Line 9: 0: Auto numa balancing is not active Line 10: 1: Auto numa balancing is active Line 11: 2: Can't get the status(maybe not support in this kernel Maybe create AutoNumaBalanchingStatusLegacy enum with the numbers, change the code to accept both string and number, so we can deprecate the numeric version in the future. It is pain to debug apis calls using numbers instead of text.
We have the same issue with storage apis. Line 12: version) Line 13: Line 14: BalloonInfo: &BalloonInfo Line 15: added: '3.1'
Line 1389: description: A mapping of hook script information Line 1390: name: HookScriptAdditional Line 1391: properties: Line 1392: - description: A key for hook script information Line 1393: name: custom_name What is custom_name? do we check this value during verification? Line 1394: type: *HookScriptInfo Line 1395: type: object Line 1396: Line 1397: HookScriptInfoMap: &HookScriptInfoMap
Nir Soffer has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/59251/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
Line 1389: description: A mapping of hook script information Line 1390: name: HookScriptAdditional Line 1391: properties: Line 1392: - description: A key for hook script information Line 1393: name: custom_name
What is custom_name? do we check this value during verification?
Maybe any_string? Line 1394: type: *HookScriptInfo Line 1395: type: object Line 1396: Line 1397: HookScriptInfoMap: &HookScriptInfoMap
Piotr Kliczewski has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/59251/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
Line 7: type: enum Line 8: values: Line 9: 0: Auto numa balancing is not active Line 10: 1: Auto numa balancing is active Line 11: 2: Can't get the status(maybe not support in this kernel
Maybe create AutoNumaBalanchingStatusLegacy enum with the numbers, change t
In the storage api is even worse because there is numeric and string based in use at the same time. Line 12: version) Line 13: Line 14: BalloonInfo: &BalloonInfo Line 15: added: '3.1'
Line 1389: description: A mapping of hook script information Line 1390: name: HookScriptAdditional Line 1391: properties: Line 1392: - description: A key for hook script information Line 1393: name: custom_name
Maybe any_string?
We need some string that would tell us that here we may have anything. any_string should be as good as custom_name. Line 1394: type: *HookScriptInfo Line 1395: type: object Line 1396: Line 1397: HookScriptInfoMap: &HookScriptInfoMap
gerrit-hooks has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 3:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Piotr Kliczewski has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 3: Verified+1
Verified by updating vdsm and starting and stopping a vm.
Piotr Kliczewski has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 3:
ping
From Dan Kenigsberg danken@redhat.com:
Dan Kenigsberg has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 9: Code-Review+2
raising
From Dan Kenigsberg danken@redhat.com:
Dan Kenigsberg has posted comments on this change.
Change subject: schema: customizable hook name ......................................................................
Patch Set 10: Code-Review+2 Verified+1
Copying score after trivial rebase
From Dan Kenigsberg danken@redhat.com:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: schema: customizable hook name ......................................................................
schema: customizable hook name
getCaps response contains hook names which by design can be any string. We need to support following naming scheme in validation.
Change-Id: Icb20039ef1d3bba32518dd23e6b30506cb77e0ec Signed-off-by: Piotr Kliczewski piotr.kliczewski@gmail.com --- M lib/vdsm/api/vdsm-api.yml M lib/vdsm/api/vdsmapi.py M tests/vdsmapi_test.py 3 files changed, 215 insertions(+), 10 deletions(-)
Approvals: Jenkins CI: Passed CI tests Dan Kenigsberg: Verified; Looks good to me, approved
vdsm-patches@lists.fedorahosted.org