Bug 1258097

Summary: If block disk alias or description is too long, disk metadata will be truncated, causing various failures
Product: [oVirt] vdsm Reporter: Nir Soffer <nsoffer>
Component: GeneralAssignee: Nir Soffer <nsoffer>
Status: CLOSED CURRENTRELEASE QA Contact: Aharon Canan <acanan>
Severity: high Docs Contact:
Priority: high    
Version: 4.16.0CC: acanan, amureini, bazulay, bugs, ebenahar, ecohen, gklein, kgoldbla, lsurette, mgoldboi, nsoffer, rbalakri, tnisan, ycui, yeylon
Target Milestone: ovirt-3.6.0-rcFlags: rule-engine: ovirt-3.6.0+
ylavi: planning_ack+
rule-engine: devel_ack+
acanan: testing_ack+
Target Release: 4.17.10   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: storage
Fixed In Version: v4.17.9 Doc Type: Bug Fix
Doc Text:
Cause: Vdsm was not validating the size of the description field, causing corruption of vdsm volume metadata. Consequence: Corrupted volume metadata would fail various operation on that volume. Fix: Vdsm validate and truncate if needed the description field, keeping volume meta data valid even if given invalid (too long) description. Result: Too long description does not cause volume metadata corruption.
Story Points: ---
Clone Of:
: 1258547 (view as bug list) Environment:
Last Closed: 2016-01-13 14:40:41 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Storage RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1258547    

Description Nir Soffer 2015-08-29 02:26:20 UTC
Description of problem:

Disk alias maximum size is 255 characters, and description is 500 characters.
These values are stored in disk metadata in the shared storage since ovirt
3.5. However, on block storage we have only about 250 bytes free.

When entering long values into these fields, the disk metadata is truncated
silently. The truncated data may become invalid, failing various operations
with the disk, and the alias and description becomes unreadable.

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

How reproducible:
Always

Steps to Reproduce:
1. Create disk with alias or description of 250-300 ascii characters
   (Vdsm does not support non-ascii alias and description)

Actual results:
Disk is created on storage, but metadata is truncated and invalid.

Expected results:
Disk should not be created, and MetadataOverflowError should be returned to engine.

Additional info:
This porblem exists since first vdsm version, but it became a bigger problem
since ovirt 3.5 since disk alias is stored now in the disk metadata on 
the shared storage.

There is a mismatch between the ovirt-engine database size limits to the size
of disk metadata. In storage domain format 3, we cannot store the full disk
alias and description in the shared storage.

This issue should be fixed both in engine (to support older vdsm versions) and
in vdsm (to support older engines). This bug is only about the vdsm side.
The engine side bug is tracked in a separate bug.

Comment 1 Natalie Gavrielov 2015-11-18 16:27:50 UTC
Hi Nir,

If I understand correctly, this issue can only be tested with vdsm (4.17.9) working with an older engine version (which?)

Comment 2 Kevin Alon Goldblatt 2015-11-19 14:15:11 UTC
Tested with the following code:
-----------------------------------
rhevm-3.6.0.3-0.1.el6.noarch
vdsm-4.17.10.1-0.el7ev.noarch

Verified with the following steps:
----------------------------------
1. Created iscsi disk with alias of 280 characters and no description - an error is generated and the disk is not created as expected
2. Created iscsi disk with alias of 2 characters and a description of 28 characters >>>>> the disk is created. It should have failed according to the steps to reproduce is the Description!


Moving to assigned!

Comment 3 Red Hat Bugzilla Rules Engine 2015-11-19 14:15:13 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 4 Nir Soffer 2015-11-20 12:47:12 UTC
(In reply to Kevin Alon Goldblatt from comment #2)
> Tested with the following code:
> -----------------------------------
> rhevm-3.6.0.3-0.1.el6.noarch
> vdsm-4.17.10.1-0.el7ev.noarch
> 
> Verified with the following steps:
> ----------------------------------
> 1. Created iscsi disk with alias of 280 characters and no description - an
> error is generated and the disk is not created as expected

How did you create the disk? using engine ui? vdsClient? Which error did you get?

> 2. Created iscsi disk with alias of 2 characters and a description of 28
> characters >>>>> the disk is created. It should have failed according to the
> steps to reproduce is the Description!

Why do you think we cannot use alias of 2 characters and description of 28
characters?

The description of the bug is not correct about the required solution. 
We cannot fail disk creation with long alias or description, since this 
would break compatibility with older engines, or when copying old disks
with long alias or description.

The behavior is as follows (described in http://gerrit.ovirt.org/46330)

1. If the description field is longer then 280 characters, it is
   truncated to allow saving the metadata

2. Newer engine will never send long description

The description line looks like this:

{"DiskAlias":"alias","DiskDescription":"description"}

This line cannot exceed 280 bytes. If it does, we truncate it.

How to test this:

1. Use ovirt-3.5 engine - ovirt 3.6 will fail if you try to set too
   long description.
2. Create a disk with long alias or description or both, so the description
   line exceeds the limit (280 bytes).
3. Engine request should succeed
4. Check the <volume uuid>.meta file (on NFS). the description line should
   be truncated. All other fields should not be truncated.
5. Do the same on 3.5 setup. The description line would not be tuncated, but
   other lines in the meta file will be truncated or missing.

Checking metadata on file based storage:

/rhev/data-center/mnt/<server>:<remote_path>/<domainid>/images/<imageid>/<volumeid>.meta

Checking metadata on block storage:

Look into the metadata lv using less:

less -f /dev/<domainid>/metadata

Locate the disk by the image uuid. Here is an example:

DOMAIN=6c77adb1-74fc-4fa9-
a0ac-3b5a4b789318
CTIME=1448023306
FORMAT=COW
DISKTYPE=2
LEGALITY=LEGAL
SIZE=2097152
VOLTYPE=LEAF
DESCRIPTION={"DiskAlias":"alias","DiskDescription":"description"}
IMAGE=a9612109-5b18-4dc0-ad21-748fc9f8fb7d
PUUID=00000000-0000-0000-0000-000000000000
MTIME=0
POOL_UUID=
TYPE=SPARSE

Comment 5 Red Hat Bugzilla Rules Engine 2015-11-20 12:47:16 UTC
Bug tickets that are moved to testing must have target release set to make sure tester knows what to test. Please set the correct target release before moving to ON_QA.

Comment 6 Nir Soffer 2015-11-20 12:53:54 UTC
(In reply to Red Hat Bugzilla Rules Engine from comment #5)
> Bug tickets that are moved to testing must have target release set to make
> sure tester knows what to test. Please set the correct target release before
> moving to ON_QA.

Dear bot,

Please avoid modifying bugs. You may warn about incorrect settings, but
you cannot change what a human did.

Thanks for wasting my time

Comment 7 Kevin Alon Goldblatt 2015-11-22 09:01:16 UTC
(In reply to Nir Soffer from comment #4)
> (In reply to Kevin Alon Goldblatt from comment #2)
> > Tested with the following code:
> > -----------------------------------
> > rhevm-3.6.0.3-0.1.el6.noarch
> > vdsm-4.17.10.1-0.el7ev.noarch
> > 
> > Verified with the following steps:
> > ----------------------------------
> > 1. Created iscsi disk with alias of 280 characters and no description - an
> > error is generated and the disk is not created as expected
> 
> How did you create the disk? using engine ui? vdsClient? Which error did you
> get?
> 

I created the disk via the GUI and this was the expected Correct behaviour when the action failed with an error in the GUI

> > 2. Created iscsi disk with alias of 2 characters and a description of 28
> > characters >>>>> the disk is created. It should have failed according to the
> > steps to reproduce is the Description!
> 
> Why do you think we cannot use alias of 2 characters and description of 28
> characters?
> 

Correction to #2. The iscsi disk was created via the GUI  with and Alias of 2 characters and a description of 280 characters. Accroting to the steps to reproduce in the description this action should have failed but did not.
> The description of the bug is not correct about the required solution. 
> We cannot fail disk creation with long alias or description, since this 
> would break compatibility with older engines, or when copying old disks
> with long alias or description.
> 
> The behavior is as follows (described in http://gerrit.ovirt.org/46330)
> 
> 1. If the description field is longer then 280 characters, it is
>    truncated to allow saving the metadata
> 
> 2. Newer engine will never send long description
> 
> The description line looks like this:
> 
> {"DiskAlias":"alias","DiskDescription":"description"}
> 
> This line cannot exceed 280 bytes. If it does, we truncate it.
> 
> How to test this:
> 
> 1. Use ovirt-3.5 engine - ovirt 3.6 will fail if you try to set too
>    long description.
> 2. Create a disk with long alias or description or both, so the description
>    line exceeds the limit (280 bytes).
> 3. Engine request should succeed
> 4. Check the <volume uuid>.meta file (on NFS). the description line should
>    be truncated. All other fields should not be truncated.
> 5. Do the same on 3.5 setup. The description line would not be tuncated, but
>    other lines in the meta file will be truncated or missing.
> 
> Checking metadata on file based storage:
> 
> /rhev/data-center/mnt/<server>:<remote_path>/<domainid>/images/<imageid>/
> <volumeid>.meta
> 
> Checking metadata on block storage:
> 
> Look into the metadata lv using less:
> 
> less -f /dev/<domainid>/metadata
> 
> Locate the disk by the image uuid. Here is an example:
> 
> DOMAIN=6c77adb1-74fc-4fa9-
> a0ac-3b5a4b789318
> CTIME=1448023306
> FORMAT=COW
> DISKTYPE=2
> LEGALITY=LEGAL
> SIZE=2097152
> VOLTYPE=LEAF
> DESCRIPTION={"DiskAlias":"alias","DiskDescription":"description"}
> IMAGE=a9612109-5b18-4dc0-ad21-748fc9f8fb7d
> PUUID=00000000-0000-0000-0000-000000000000
> MTIME=0
> POOL_UUID=
> TYPE=SPARSE

Comment 8 Red Hat Bugzilla Rules Engine 2015-11-22 09:01:20 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 9 Kevin Alon Goldblatt 2015-11-22 09:02:37 UTC
(In reply to Kevin Alon Goldblatt from comment #7)
> (In reply to Nir Soffer from comment #4)
> > (In reply to Kevin Alon Goldblatt from comment #2)
> > > Tested with the following code:
> > > -----------------------------------
> > > rhevm-3.6.0.3-0.1.el6.noarch
> > > vdsm-4.17.10.1-0.el7ev.noarch
> > > 
> > > Verified with the following steps:
> > > ----------------------------------
> > > 1. Created iscsi disk with alias of 280 characters and no description - an
> > > error is generated and the disk is not created as expected
> > 
> > How did you create the disk? using engine ui? vdsClient? Which error did you
> > get?
> > 
> 
> I created the disk via the GUI and this was the expected Correct behaviour
> when the action failed with an error in the GUI
> 
> > > 2. Created iscsi disk with alias of 2 characters and a description of 28
> > > characters >>>>> the disk is created. It should have failed according to the
> > > steps to reproduce is the Description!
> > 
> > Why do you think we cannot use alias of 2 characters and description of 28
> > characters?
> > 
> 
> Correction to #2. The iscsi disk was created via the GUI  with and Alias of
> 2 characters and a description of 280 characters. According to the steps to
> reproduce in the description this action should have failed but did not.
> > The description of the bug is not correct about the required solution. 
> > We cannot fail disk creation with long alias or description, since this 
> > would break compatibility with older engines, or when copying old disks
> > with long alias or description.
> > 
> > The behavior is as follows (described in http://gerrit.ovirt.org/46330)
> > 
> > 1. If the description field is longer then 280 characters, it is
> >    truncated to allow saving the metadata
> > 
> > 2. Newer engine will never send long description
> > 
> > The description line looks like this:
> > 
> > {"DiskAlias":"alias","DiskDescription":"description"}
> > 
> > This line cannot exceed 280 bytes. If it does, we truncate it.
> > 
> > How to test this:
> > 
> > 1. Use ovirt-3.5 engine - ovirt 3.6 will fail if you try to set too
> >    long description.
> > 2. Create a disk with long alias or description or both, so the description
> >    line exceeds the limit (280 bytes).
> > 3. Engine request should succeed
> > 4. Check the <volume uuid>.meta file (on NFS). the description line should
> >    be truncated. All other fields should not be truncated.
> > 5. Do the same on 3.5 setup. The description line would not be tuncated, but
> >    other lines in the meta file will be truncated or missing.
> > 
> > Checking metadata on file based storage:
> > 
> > /rhev/data-center/mnt/<server>:<remote_path>/<domainid>/images/<imageid>/
> > <volumeid>.meta
> > 
> > Checking metadata on block storage:
> > 
> > Look into the metadata lv using less:
> > 
> > less -f /dev/<domainid>/metadata
> > 
> > Locate the disk by the image uuid. Here is an example:
> > 
> > DOMAIN=6c77adb1-74fc-4fa9-
> > a0ac-3b5a4b789318
> > CTIME=1448023306
> > FORMAT=COW
> > DISKTYPE=2
> > LEGALITY=LEGAL
> > SIZE=2097152
> > VOLTYPE=LEAF
> > DESCRIPTION={"DiskAlias":"alias","DiskDescription":"description"}
> > IMAGE=a9612109-5b18-4dc0-ad21-748fc9f8fb7d
> > PUUID=00000000-0000-0000-0000-000000000000
> > MTIME=0
> > POOL_UUID=
> > TYPE=SPARSE

Comment 10 Nir Soffer 2015-11-22 10:48:28 UTC
Kevin, stop changing this bug status, in particular, do not set it to assigned, and it merged. If you want to change the status, please
consult Allon.

This bug is not about engine behavior. Engine success or failure is not 
relevant to this issue.

Please verify this bug as explained in comment 4.

Comment 11 Red Hat Bugzilla Rules Engine 2015-11-22 10:48:33 UTC
Bug tickets that are moved to testing must have target release set to make sure tester knows what to test. Please set the correct target release before moving to ON_QA.

Comment 12 Nir Soffer 2015-11-22 10:49:12 UTC
Kevin, stop changing this bug status, in particular, do not set it to assigned, and it merged. If you want to change the status, please
consult Allon.

This bug is not about engine behavior. Engine success or failure is not 
relevant to this issue.

Please verify this bug as explained in comment 4.

Comment 13 Elad 2015-12-09 08:15:24 UTC
An attempt to add a disk (via Webadmin) with 420 characters long alias fails with the following exception in engine [1].
This is an engine issue and it blocks us from verifying the vdsm issue discussed here.
Opened a bug: bug 1289855

Comment 14 Elad 2015-12-09 08:16:01 UTC
[1] 

2015-12-09 07:52:35,552 INFO  [org.ovirt.engine.core.bll.AddImageFromScratchCommand] (ajp-/127.0.0.1:8702-11) [c2d1b05] Running command: AddImageFromScratchCommand internal: true. Entities affected :  ID: 504c7595
-7ca5-459c-be6f-44a3d7f1b5d7 Type: Storage
2015-12-09 07:52:35,566 INFO  [org.ovirt.engine.core.utils.transaction.TransactionSupport] (ajp-/127.0.0.1:8702-11) [c2d1b05] transaction rolled back
2015-12-09 07:52:35,567 ERROR [org.ovirt.engine.core.bll.AddImageFromScratchCommand] (ajp-/127.0.0.1:8702-11) [c2d1b05] Command 'org.ovirt.engine.core.bll.AddImageFromScratchCommand' failed: CallableStatementCallb
ack; SQL [{call insertbasedisk(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)}]; ERROR: value too long for type character varying(255)
  Where: SQL statement "INSERT INTO base_disks( disk_id, disk_interface, wipe_after_delete, propagate_errors, disk_alias, disk_description, shareable, boot, sgio, alignment, last_alignment_scan, disk_storage_type,
 cinder_volume_type) VALUES(  $1 ,  $2 ,  $3 ,  $4 ,  $5 ,  $6 ,  $7 ,  $8 ,  $9 ,  $10 ,  $11 ,  $12 ,  $13 )"
PL/pgSQL function "insertbasedisk" line 2 at SQL statement; nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(255)
  Where: SQL statement "INSERT INTO base_disks( disk_id, disk_interface, wipe_after_delete, propagate_errors, disk_alias, disk_description, shareable, boot, sgio, alignment, last_alignment_scan, disk_storage_type,
 cinder_volume_type) VALUES(  $1 ,  $2 ,  $3 ,  $4 ,  $5 ,  $6 ,  $7 ,  $8 ,  $9 ,  $10 ,  $11 ,  $12 ,  $13 )"
PL/pgSQL function "insertbasedisk" line 2 at SQL statement
2015-12-09 07:52:35,567 ERROR [org.ovirt.engine.core.bll.AddImageFromScratchCommand] (ajp-/127.0.0.1:8702-11) [c2d1b05] Exception: org.springframework.dao.DataIntegrityViolationException: CallableStatementCallback
; SQL [{call insertbasedisk(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)}]; ERROR: value too long for type character varying(255)
  Where: SQL statement "INSERT INTO base_disks( disk_id, disk_interface, wipe_after_delete, propagate_errors, disk_alias, disk_description, shareable, boot, sgio, alignment, last_alignment_scan, disk_storage_type,
 cinder_volume_type) VALUES(  $1 ,  $2 ,  $3 ,  $4 ,  $5 ,  $6 ,  $7 ,  $8 ,  $9 ,  $10 ,  $11 ,  $12 ,  $13 )"
PL/pgSQL function "insertbasedisk" line 2 at SQL statement; nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(255)
  Where: SQL statement "INSERT INTO base_disks( disk_id, disk_interface, wipe_after_delete, propagate_errors, disk_alias, disk_description, shareable, boot, sgio, alignment, last_alignment_scan, disk_storage_type, cinder_volume_type) VALUES(  $1 ,  $2 ,  $3 ,  $4 ,  $5 ,  $6 ,  $7 ,  $8 ,  $9 ,  $10 ,  $11 ,  $12 ,  $13 )"
PL/pgSQL function "insertbasedisk" line 2 at SQL statement
        at org.springframework.jdbc.support.SQLStateSQLExceptionTranslator.doTranslate(SQLStateSQLExceptionTranslator.java:101) [spring-jdbc.jar:3.1.1.RELEASE]
        at

Comment 15 Nir Soffer 2015-12-22 21:17:54 UTC
This bug does not depend on anything, and can be verified *now*. Please
check comment 4, explaining in great details how to verify it.

Note that there was an error in the description, the maximum size of the 
description field is 210 bytes and not 280 as seen in comment 4.

There is no need to try with 420 characters alias or description, trying
with 174 ascii characters alias or description should trigger the
the truncation of the vdsm side, since engine is creating a json text
with the alias and description:

   {"DiskAlias":"","DiskDescription":""}

With empty description, alias can be up to 173 bytes. Longer value will
cause truncation of the json text on vdsm side.

It is also possible to test this using vdsClient setVolumeDescription. Pass 
a string longer than 210 characters and you will see the string truncated
on storage.

Comment 16 Aharon Canan 2015-12-28 16:36:27 UTC
Verified with vdsClient (vdsm-4.17.14-0.el7ev.noarch) as engine truncated it as well.

[root@green-vdsc ~]# vdsClient -s 0 setVolumeDescription 3adcbe29-39bf-4037-a489-9dc06cf74751 00000001-0001-0001-0001-0000000000a8 6d0e00fe-01c7-4fc9-ad61-6395a1d044e9 42fa2c30-d05b-4bfa-ab51-c223b362d762 "AharonCananAharonCanan1AharonCananAharonCanan2AharonCananAharonCanan3AharonCananAharonCanan4AharonCananAharonCanan5AharonCananAharonCanan6AharonCananAharonCanan7AharonCananAharonCanan8AharonCananAharonCanan9AharonCananAharonCanan10AharonCananAharonCanan11AharonCananAharonCanan12AharonCananAharonCanan13"

[root@camel-vdsc ~]# less /rhev/data-center/mnt/10.35.64.11\:_vol_RHEV_Storage_acanan01/3adcbe29-39bf-4037-a489-9dc06cf74751/images/6d0e00fe-01c7-4fc9-ad61-6395a1d044e9/42fa2c30-d05b-4bfa-ab51-c223b362d762.meta |grep DESCRI
DESCRIPTION=AharonCananAharonCanan1AharonCananAharonCanan2AharonCananAharonCanan3AharonCananAharonCanan4AharonCananAharonCanan5AharonCananAharonCanan6AharonCananAharonCanan7AharonCananAharonCanan8AharonCananAharonCanan9Aha

Comment 17 Sandro Bonazzola 2016-01-13 14:40:41 UTC
oVirt 3.6.0 has been released, closing current release