Bug 1071948 - [Docs][Technical][async]Set VM Payload via REST isn't correctly documented
Summary: [Docs][Technical][async]Set VM Payload via REST isn't correctly documented
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: Documentation
Version: 3.5.0
Hardware: Unspecified
OS: Linux
unspecified
high
Target Milestone: ---
: 3.5.0
Assignee: Andrew Burden
QA Contact: Tahlia Richardson
URL:
Whiteboard: virt
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-03-03 14:23 UTC by Shrikant
Modified: 2015-02-16 04:47 UTC (History)
15 users (show)

Fixed In Version:
Doc Type: Release Note
Doc Text:
Starting with version 3.3 the support for only one file in payloads has been removed, as it caused issues for clients of the API that use the generateDS.py script to generate Python code, including the Python SDK. Calls to the API that use the syntax specifying only one file are ignored, and the payload isn't saved. The correct way to specify the payload starting with 3.3 is using multiple "file" elements with inner "name" and "content" elements: <payloads> <payload type="cdrom"> <files> <file> <name>/etc/hosts</name> <content>127.0.0.1 localhost myvm.example.com</content> </file> </files> </payload> </payloads>
Clone Of:
Environment:
Last Closed: 2015-02-16 04:47:01 UTC
oVirt Team: ---
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Shrikant 2014-03-03 14:23:57 UTC
Description of problem: The REST call to set a VM payload for a VM returns successfully, without actually applying the changes to the VM.  

Version-Release number of selected component (if applicable): RHEV-M 3.2

How reproducible: Easily reproducible on any RHEV 3.3 setup.

Steps to Reproduce:
1. Just set the payload for a VM using the command: 
/usr/bin/curl --silent --fail --insecure --request  PUT --data "<vm><payloads><payload type='cdrom'><file name='Info'><content>Devices=eth0,eth1</content></file></payload></payloads></vm>" --user admin@internal:<password_removed> --basic --url https://<RHEVM_IP>:443/api/vms/5a3034fb-d51d-45ae-8d86-6d93a8933932 --show-error --header "Content-type:application/xml"

2. Check the payload using the command:
/usr/bin/curl --silent --fail --insecure --request  GET  --user admin@internal:<password_removed> --basic --url https://<RHEVM_IP>:443/api/vms/5a3034fb-d51d-45ae-8d86-6d93a8933932


Actual results:
It just shows the following:
    <payloads>
        <payload type="cdrom"/>
    </payloads>

Expected results:
It should accept the payload and show in the vm xml:
    <payloads>
        <payload type="cdrom">
            <file name="Info">
                <content>Devices=eth0,eth1</content>
            </file>
        </payload>
    </payloads>


Additional info: This same call works well with RHEVM version 3.2.

Comment 1 Juan Hernández 2014-03-03 14:44:39 UTC
The capability to specify only one file was considered incorrect, as it induced a problem in the Python bindings generated with the generateDS.py script, see bugs 1018782 and 1019618.

Currently, starting with 3.3, the correct way to set the payload is to use a "files" element containing multiple "file" elements. In addition the name of the file has been changed from an attribute to an inner element. So the correct way to do this now is the following:

curl \
-k \
-X PUT \
-H "Accept: application/xml" \
-H "Content-Type: application/xml" \
-d '
<vm>
  <payloads>
    <payload type="cdrom">
      <files>
        <file>
          <name>Info</name>
          <content>Devices=eth0,eth1</content>
        </file>
      </files>
    </payload>
  </payloads>
</vm>
' \
-u admin@internal:<password_removed> \
https://<RHEVM_IP>:443/api/vms/5a3034fb-d51d-45ae-8d86-6d93a8933932

This has been reported before, see bug 1068803.

Is it feasible for you to use this syntax with 3.3 and newer and the old syntax with 3.2?

Comment 2 Itamar Heim 2014-03-03 14:55:00 UTC
wasn't payload prior to 3.4 only via runonce (vm launch), without persistence?

Comment 3 Juan Hernández 2014-03-03 15:10:49 UTC
The payloads are persistent since the payload feature was added:

http://gerrit.ovirt.org/3243

The data is stored in the spec_params column of the vm_device table.

What wasn't persistent till 3.4 is the cloud-init data, which was introduced later. In 3.4 it has been renamed to vm-init and made persistent.

I may be wrong. Shahar, can you confirm?

Comment 4 Juan Hernández 2014-03-03 15:12:37 UTC
Shrikant, we still need to know if you can handle this difference between 3.2 and 3.3 or if we need to do something else to support you.

Comment 5 Shahar Havivi 2014-03-03 15:32:22 UTC
(In reply to Juan Hernández from comment #3)
> The payloads are persistent since the payload feature was added:
> 
> http://gerrit.ovirt.org/3243
> 
> The data is stored in the spec_params column of the vm_device table.
> 
> What wasn't persistent till 3.4 is the cloud-init data, which was introduced
> later. In 3.4 it has been renamed to vm-init and made persistent.
> 
> I may be wrong. Shahar, can you confirm?

Correct

Comment 6 Shrikant 2014-03-04 06:57:15 UTC
Thanks for the info. I can modify my script to use the new API. It will be better if such changes are documented (in rel notes) so that we can be prepared for them.
Also, the new api is not properly documented in the dev guide either.

Hopefully, there won't be any further changes to this interface in the future.

Comment 7 Juan Hernández 2014-03-04 08:33:55 UTC
I'm changing the component to the RESTAPI guide.

The section 14.1 of the developer guide needs to be updated, as it contains the description of the old syntax. I suggest the following wording for the description of the "payloads" element:

---8<---
Defines a set of payload elements to deliver content to a virtual machine upon boot. Each payload requires a type attribute, either cdrom or floppy, and a set of file elements. Within the each file element there are a name element the specifies the name and location of the file and a content element that defines the content to deliver to the file.
--->8---

The example in section 14.2 also needs to be updated:

---8<---
<payloads>
  <payload type='cdrom'>
    <files>
      <file>
        <name='/etc/hosts'/>
        <content>
          127.0.0.1 localhost myvm.example.com
        </content>
      </file>
    </files>
  </payload>
</payloads>
--->8---

I'm also requesting that this information be added to the 3.3.2 release notes.

Comment 8 Juan Hernández 2014-03-04 08:35:54 UTC
Dan, I can't request the rhev-3.3.z flag, can you?

Comment 9 Shrikant 2014-03-05 08:47:12 UTC
Some additional questions:
1. Will it be possible to backport this change to RHEV 3.2?
2. Can you make the API fail if it doesn't contain <files> element? This will help us retry the call using the new format instead of relying on the RHEV-M version.
3. Can you allow <file> element without <files> element to provide backward compatibility?
4. Can you allow multiple <file> elements without an enclosing <files> element? Since all the <file> elements will be under the same <payload> element, it should mean the same thing.
5. Does the new API depend on RHEV-M version or DataCenter version?

Comment 10 Juan Hernández 2014-03-05 09:31:09 UTC
(In reply to Shrikant from comment #9)
> Some additional questions:
> 1. Will it be possible to backport this change to RHEV 3.2?

We could do this, but you will still have the problem that with 3.2.6 or older you will have to use the old syntax and with 3.2.7 or newer you will have to use the new one, so it won't be very helpful.

> 2. Can you make the API fail if it doesn't contain <files> element? This
> will help us retry the call using the new format instead of relying on the
> RHEV-M version.

I think this makes sense, I opened bug 1072807 to track it.

> 3. Can you allow <file> element without <files> element to provide backward
> compatibility?

This is what would make more sense, in my opinion, but we have serious problem there, as it breaks the expectations of the generateDS.py too that we use to generate the Python SDK.

I'm opening bug 1072819 to track it, as this is a documentation bug now.

> 4. Can you allow multiple <file> elements without an enclosing <files>
> element? Since all the <file> elements will be under the same <payload>
> element, it should mean the same thing.

This won't solve the problem of backwards compatibility, as the new "file" element contains nested "name" and "content" elements instead of attributes.

> 5. Does the new API depend on RHEV-M version or DataCenter version?

The new API depends on the RHEV-M version only, not on the DataCenter version.

Comment 12 Andrew Burden 2014-10-22 13:36:45 UTC
Documentation Link
------------------------------
https://documentation-devel.engineering.redhat.com/site/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.5/html/Technical_Guide/chap-Virtual_Machines.html#Virtual_Machine_Elements 
(and next page for [7985])


What Changed 
------------------------------
Added descriptions of two elements (type and file) that are needed within the payloads element. These were also added to the xml representation of a VM.


Virtual Machine Elements [7984-713194]
XML Representation of a Virtual Machine [7985-713188]
Updated revision history: [34616-713199]

Versions Applicable: 3.5, 3.4, 3.3

Comment 13 Tahlia Richardson 2014-10-23 00:38:25 UTC
Changes from comment 7 appear in the document; no errors found. 

VERIFIED


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