Bug 833788 - OVIRT-CLI: collection-based-options definition doesn't look like cli format
OVIRT-CLI: collection-based-options definition doesn't look like cli format
Status: CLOSED CURRENTRELEASE
Product: oVirt
Classification: Community
Component: ovirt-engine-cli (Show other bugs)
unspecified
Unspecified Unspecified
high Severity high
: ---
: 3.2
Assigned To: Michael Pasternak
infra
: TestBlocker
Depends On:
Blocks: 845235
  Show dependency treegraph
 
Reported: 2012-06-20 07:17 EDT by Elena
Modified: 2014-01-12 20:45 EST (History)
12 users (show)

See Also:
Fixed In Version: 3.1.0.8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 845235 (view as bug list)
Environment:
Last Closed: 2013-02-15 01:46:30 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Elena 2012-06-20 07:17:03 EDT
Current format for power management options is the following:
--power_management-options-option "{option.name=secure;option.value=false}" 

It doesn't look like cli format. 

Simon suggested about the following format:
--power_management-options-option secure=false,slot=1
Comment 1 Elena 2012-06-24 08:44:45 EDT
Same for storage-logical_unit:
* --storage-logical_unit: collection
       {
         logical_unit.address: string
         logical_unit.port: int
         .....
       }
Comment 2 Michael Pasternak 2012-06-24 08:48:51 EDT
it's not implemented peer object instance, all collection-based-options
in cli are modelled in same manner.
Comment 3 Michael Pasternak 2012-07-10 05:25:03 EDT
(In reply to comment #0)
> Current format for power management options is the following:
> --power_management-options-option "{option.name=secure;option.value=false}" 
> 
> It doesn't look like cli format. 
> 
> Simon suggested about the following format:
> --power_management-options-option secure=false,slot=1

i can't see any difference with current impl:

"{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..."

note this collection of options, not the collection of option
parameters, that's the reason for {...},{...}

any other suggestions?
Comment 5 Simon Grinberg 2012-07-10 10:18:46 EDT
(In reply to comment #3)
Michael, according to Elena the format is: 
> > --power_management-options-option "{option.name=secure;option.value=false}" 

which is clearly not a CLI standard 


You say it is:

> 
> "{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..."
> 

The above looks much better though usually in a command line you try to avoid any special shell characters.

However when you say collection of options do you mean the following? 

--power_management-options "{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..."

If so then what is --power_management-options-option used for? 
This is confusing.
Comment 6 Michael Pasternak 2012-07-11 05:16:25 EDT
(In reply to comment #5)
> (In reply to comment #3)
> Michael, according to Elena the format is: 
> > > --power_management-options-option "{option.name=secure;option.value=false}" 
> 
> which is clearly not a CLI standard 

what is "CLI standard"?

note this is not about option format, but about actual 
option argument.

> 
> 
> You say it is:
> 
> > 
> > "{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..."
> > 
> 
> The above looks much better though usually in a command line you try to
> avoid any special shell characters.

if you mean ';', its inside argument, not command ...

> 
> However when you say collection of options do you mean the following? 
> 
> --power_management-options "{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..."

i mean "{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..."

> 
> If so then what is --power_management-options-option used for? 
> This is confusing.

--power_management-options-option - is a cli option, the problem here is that
options-option is collection actually, this bad xsd modelling in api schema
we drag from 2.2 api for backward compatibility

"{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..." - is a argument for the option
which can be in any format,

of course if you have better idea - i'm open
Comment 7 Michael Pasternak 2012-07-11 05:28:14 EDT
bottom line:

--power_management-options-option is cli option representing ref. to collection
of power_management options  

"{x=a1;y=b1;z=c1;...},{x=a2;y=b2;z=c2;...},..." is an argument representing 
collection of power_management options
Comment 8 Geert Jansen 2012-07-12 08:42:12 EDT
The format that Simon proposed is more readable, but since this is automatically generated i don't think it makes sense to add exceptions and instead having a generic format that can be mapped to any XML construct is better.

So for me, what Michael proposed is good. I don't think there are good command-line standards for arguments taking a list of key/value pairs. I would implement two small changes:

 * I would remove "-option" as it is redundant. So --power_management-options instead of --power_manager-options-option
 * Instead of option.name=x;option.value=y, i would leave out "option".
Comment 9 Michael Pasternak 2012-07-12 10:10:23 EDT
Hi Geert,

Thanks a lot for your comment, the reason for "-option" suffix in cli
option and "option." prefix in the argument key is specifying FQ path to actual collection in the object as not all collections in the api schema where modelled in a same way, 

i tried to implement generic drill down collection lookup, but it's
not that easy, take for instance PowerManagement, because it was modelled
in schema as:

  <xs:complexType name="Options">
    <xs:sequence>
      <xs:element name="option" type="Option" minOccurs="0" maxOccurs="unbounded">
        <xs:annotation>
          <xs:appinfo>
            <jaxb:property name="Options"/>
          </xs:appinfo>
        </xs:annotation>
      </xs:element>
    </xs:sequence>
  </xs:complexType>

actual collection in python entity is:
  Host.PowerManagement.Options.option=[]

and not:
  Host.PowerManagement.Options

that the reason for --power_management-options-option cli option, 

of course i can try different heuristics (like wrapping options to
singular format), but it's not always works like in custom_properties.custom_property collection for example.
Comment 10 Simon Grinberg 2012-07-17 12:51:34 EDT
Guys I still think it's ugly considering the intended users for the CLI who are admins and automation people not programmers like in the SDK use case.

And Michael, excuse me for not using a generalized form as in your answers. But it took me some time to understand what you mean due to that. 

Geert you are suggesting something like:
--power_management-options-option "{name=secure;value=false}"
Which is much better then 
--power_management-options-option "{option.name=secure;option.value=false}"

But really what you want to say here is:
--power_management-options-option "secure=false"

Now I understand that what Michael is suggesting is mote generic, since it supports options that are more then key/name=value pairs. But are there really such options? 

The attempt to generalize makes the end user's life much less convenient.
There is no CLI standard but in most cases you see more of what I'm suggesting then the generic form. Just look at the KVM command line or the actual fence_x scripts that the options are mapped to.

I fully understand the problem that it makes for automation of the CLI creation, but it may worth it to invest time in adding exceptions per use cases and thus simplifying end users life.
Comment 11 Michael Pasternak 2012-08-15 04:40:04 EDT
well, after struggling xsd schema compatibility issues for a while, i came to conclusion that only possible way to implement this feature in a generic way is:

--<type>.<collection_obj>.<collection> "<collection_item>.<coll_item_prop>=val"

for instance:

create host --power_management-options-option "option.name=n1,option.value=v1" --power_management-options-option "option.name=n2,option.value=v2"
Comment 12 Michael Pasternak 2012-08-16 07:20:48 EDT
fixed in 3.1.0.8
Comment 13 Ilia Meerovich 2012-09-20 07:02:21 EDT
The problem that it is working in 2 ways:
1. --os-boot boot.dev=hd
2. --os-boot dev=hd

[RHEVM shell (connected)]# create vm  --description 'Test VM LOCA' --cluster-name Default --name 'restvm_LOCAL' --display-type 'spice' --display-monitors 1 --type 'desktop' --template-id '00000000-0000-0000-0000-000000000000' --memory 1073741824 --os-boot boot.dev=hd --os-type 'unassigned' --cpu-topology-cores 1 --cpu-topology-sockets 1

id                        : 12d568b1-46c8-4205-8f92-d8723964dd87
name                      : restvm_LOCAL
description               : Test VM LOCA
cluster-id                : 99408929-82cf-4dc7-a532-9d998063fa95
cpu-topology-cores        : 1
cpu-topology-sockets      : 1
creation_time             : 2012-09-20T13:50:59.809+03:00
display-allow_reconnect   : False
display-monitors          : 1
display-type              : spice
high_availability-enabled : False
high_availability-priority: 0
memory                    : 1073741824
memory_policy-guaranteed  : 1073741824
origin                    : rhev
os-boot-dev               : hd
os-type                   : unassigned
placement_policy-affinity : migratable
quota-id                  : 00000000-0000-0000-0000-000000000000
stateless                 : False
status-state              : down
template-id               : 00000000-0000-0000-0000-000000000000
type                      : desktop
usb-enabled               : False

[RHEVM shell (connected)]# create vm  --description 'Test VM LOCA' --cluster-name Default --name 'restvm_LOCAL1' --display-type 'spice' --display-monitors 1 --type 'desktop' --template-id '00000000-0000-0000-0000-000000000000' --memory 1073741824 --os-boot dev=hd --os-type 'unassigned' --cpu-topology-cores 1 --cpu-topology-sockets 1

id                        : 5081a5b9-cf14-498d-9796-cafeeba1a43e
name                      : restvm_LOCAL1
description               : Test VM LOCA
cluster-id                : 99408929-82cf-4dc7-a532-9d998063fa95
cpu-topology-cores        : 1
cpu-topology-sockets      : 1
creation_time             : 2012-09-20T13:51:16.357+03:00
display-allow_reconnect   : False
display-monitors          : 1
display-type              : spice
high_availability-enabled : False
high_availability-priority: 0
memory                    : 1073741824
memory_policy-guaranteed  : 1073741824
origin                    : rhev
os-boot-dev               : hd
os-type                   : unassigned
placement_policy-affinity : migratable
quota-id                  : 00000000-0000-0000-0000-000000000000
stateless                 : False
status-state              : down
template-id               : 00000000-0000-0000-0000-000000000000
type                      : desktop
usb-enabled               : False

[RHEVM shell (connected)]#

Note You need to log in before you can comment on or make changes to this bug.