Bug 1963083 - [RFE] Support storing user data in VM checkpoint entity
Summary: [RFE] Support storing user data in VM checkpoint entity
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: Backup-Restore.VMs
Version: 4.4.5.11
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ovirt-4.4.8
: ---
Assignee: Eyal Shenitzky
QA Contact: Amit Sharir
bugs@ovirt.org
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-05-21 11:59 UTC by Yury.Panchenko
Modified: 2021-10-17 09:07 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-08-19 06:23:05 UTC
oVirt Team: Storage
Embargoed:
pm-rhel: ovirt-4.4+
asharir: testing_plan_complete+
pelauter: planning_ack+
pm-rhel: devel_ack+
pm-rhel: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 115828 0 master MERGED backup_vm.py: allow adding a description to a VM backup and checkpoint 2021-08-02 06:36:22 UTC
oVirt gerrit 115829 0 master MERGED core: allow specifing VM backup description 2021-08-01 11:32:14 UTC
oVirt gerrit 115830 0 master MERGED core: allow adding 'description' to VM checkpoint 2021-08-01 11:32:16 UTC
oVirt gerrit 115831 0 master MERGED core: initialize VM checkpoint description 2021-08-01 11:32:18 UTC

Description Yury.Panchenko 2021-05-21 11:59:24 UTC
Description of problem:
According to documentation http://ovirt.github.io/ovirt-engine-api-model/master/#types/checkpoint
Checkpoint has comment field.

This field is very usefull for backup applications. They can use it for understanding checkpoint affiliation.

Version-Release number of selected component (if applicable):
ovirt 4.4.5

Actual results:
Current api implementation doesn't allow to set any comment in a checkpoint.

Expected results:
Comment on a checkpoint can be set when user creates vm backup vi API

Comment 1 Nir Soffer 2021-05-21 14:19:45 UTC
This bug is invalid, specifying a solution instead of the problem. Fixing
the title to present the problem instead of a solution.

If I understand the issue correctly, you want to understand if a checkpoint
is related to specific backup, so you can remove it safely without affecting
checkpoints created by another backup application on the same system.

I think the right solution is to:

- Add the backup uuid that created the checkpoint to the checkpoint

- Allow backup application to specify the backup uuid
  (the system will validate that the input is a uuid type 4)

- If the user do not specify the uuid, the system will generate
  a new uuid (current implementation).

Does it solve the problem? If not, please explain the problem in more details.

Comment 2 Yury.Panchenko 2021-05-21 14:48:57 UTC
Hello Nir,

> If I understand the issue correctly, you want to understand if a checkpoint
> is related to specific backup, so you can remove it safely without affecting
> checkpoints created by another backup application on the same system.
That's right.

> Does it solve the problem? If not, please explain the problem in more details.
I'd prefer to use comment. 
We need to store various infomation like:
 - Backup job name
 - Backup creation date
 - Restore point information

The comment field is already implemented in bitmaps. I'd like to use it for those purposes.

Comment 3 Nir Soffer 2021-05-21 15:02:52 UTC
(In reply to Yury.Panchenko from comment #2)
> Hello Nir,
> 
> > If I understand the issue correctly, you want to understand if a checkpoint
> > is related to specific backup, so you can remove it safely without affecting
> > checkpoints created by another backup application on the same system.
> That's right.
> 
> > Does it solve the problem? If not, please explain the problem in more details.
> I'd prefer to use comment. 
> We need to store various infomation like:
>  - Backup job name

What are the requirements for this field?

>  - Backup creation date

We already have creation date created by the system when starting a backup.
We can expose it in the API.

>  - Restore point information

What is this field? Why it must be in ovirt db?

> The comment field is already implemented in bitmaps. I'd like to use it for
> those purposes.

Comment is not a place to store arbitrary data like this. This is text 
describing the backup for debugging purpose.

We don't want to store arbitrary data in bitmaps. There are stored in the 
image itself.

We can store limited amount of data in engine db, but I don't think storing
user data is the right way. I would like to store only an identifier that
make it possible to find user data on the user db.

Comment 4 Nir Soffer 2021-05-21 15:21:43 UTC
According to http://ovirt.github.io/ovirt-engine-api-model/master/#types/checkpoint
we already have:

- creation_date
- description
- comment
- id
- name
- parent_id

I don't know why we have both comment, description, and name, these fields were
not included in the original design, which was:

<checkpoint id="checkpoint-uuid">
  <link href="/ovirt-engine/api/vms/vm-uuid/checkpoints/checkpoint-uuid/disks" rel="disks"/>
  <parent_id>parent-checkpoint-uuid</parent_id>
  <creation_date>xxx</creation_date>
  <vm href="/ovirt-engine/api/vms/vm-uuid" id="vm-uuid"/>
</checkpoint>

I think we need to add:

- backup_id

And the name field can be set by the user, similar to vm, disk alias
or snapshot name. This should be limited length field that can be displayed
in the user interface.

Comment 5 Eyal Shenitzky 2021-05-24 14:28:16 UTC
The requested enhancement is to add a 'comment' field to the VM checkpoint entity and allow the user to set it when a VM backup is taken.

Comment 7 Amit Sharir 2021-07-19 08:27:44 UTC
The exact flow to verify is not specified (since this isn't really a bug, it is a new feature - I want to be sure I test it correctly).
Do I need to check this new enhancement only in the UI or do I also need to run some flow using API for verification?

Comment 8 Eyal Shenitzky 2021-07-22 06:55:05 UTC
(In reply to Amit Sharir from comment #7)
> The exact flow to verify is not specified (since this isn't really a bug, it
> is a new feature - I want to be sure I test it correctly).
> Do I need to check this new enhancement only in the UI or do I also need to
> run some flow using API for verification?

Using incremental backup is allowed only via the API.

This RFE will add the ability to add a 'comment' to a backup/checkpoint entity.

So you will be able to send the following request - 

POST /ovirt-engine/api/vms/123/backups

<backup>
    <disks>
       <disk id="456" />
       …​
    </disks>
    <comment>Eyal's backup</comment>
</backup>

And when you fetched the created backup from the API (or in the response for the request) you should see - 

<backup id="789">
    <disks>
       <disk id="456" />
       …​
       …​
    </disks>
    <status>initializing</status>
    <creation_date>
    <comment>Eyal's backup</comment>
</backup>

Same for the checkpoint that was created for that backup - 

<checkpoint id="456">
     <link href="/ovirt-engine/api/vms/vm-uuid/checkpoints/456/disks" rel="disks"/>
     <parent_id>parent-checkpoint-uuid</parent_id>
     <creation_date>xxx</creation_date>
     <comment>Eyal's backup</comment>
     <vm href="/ovirt-engine/api/vms/123" id="123"/>
</checkpoint>

Comment 9 Nir Soffer 2021-07-22 11:25:13 UTC
We should not add a "comment" field.

The field should be called something "user_data", to make it clear that this information
is controlled by the user that created the backup.

Comment 11 Eyal Shenitzky 2021-07-27 05:41:32 UTC
(In reply to Nir Soffer from comment #9)
> We should not add a "comment" field.
> 
> The field should be called something "user_data", to make it clear that this
> information
> is controlled by the user that created the backup.

As we discussed, we will use the 'description' field.

Comment 12 Eyal Shenitzky 2021-08-03 06:19:08 UTC
Note, field name changed from comment #8 from comment to description


This RFE will add the ability to add a 'description' to a backup/checkpoint entity.

So you will be able to send the following request - 

POST /ovirt-engine/api/vms/123/backups

<backup>
    <disks>
       <disk id="456" />
       …​
    </disks>
    <description>Eyal's backup</description>
</backup>

And when you fetched the created backup from the API (or in the response for the request) you should see - 

<backup id="789">
    <disks>
       <disk id="456" />
       …​
       …​
    </disks>
    <status>initializing</status>
    <creation_date>
    <description>Eyal's backup</description>
</backup>

Same for the checkpoint that was created for that backup - 

<checkpoint id="456">
     <link href="/ovirt-engine/api/vms/vm-uuid/checkpoints/456/disks" rel="disks"/>
     <parent_id>parent-checkpoint-uuid</parent_id>
     <creation_date>xxx</creation_date>
     <description>Eyal's backup</description>
     <vm href="/ovirt-engine/api/vms/123" id="123"/>
</checkpoint>

Comment 13 Amit Sharir 2021-08-08 12:50:04 UTC
Version: 
vdsm-4.40.80.4-1.el8ev.x86_64
ovirt-engine-4.4.8.3-0.10.el8ev.noarch


Verification conclusions:
The expected output matched the actual output.
The total flow mentioned was done with no errors/unexpected logs.
I was able to add the "description" attribute in my API call. I then fetched the created backup and checkpoints and validated that the "description" attribute appeared as expected.
 
Bug verified.

Comment 14 Sandro Bonazzola 2021-08-19 06:23:05 UTC
This bugzilla is included in oVirt 4.4.8 release, published on August 19th 2021.

Since the problem described in this bug report should be resolved in oVirt 4.4.8 release, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.


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