Bug 1481115 - Downloading an image with backing files from the sdk is triggering disk deletion
Summary: Downloading an image with backing files from the sdk is triggering disk deletion
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: BLL.Storage
Version: 4.1.5.2
Hardware: Unspecified
OS: Unspecified
unspecified
urgent
Target Milestone: ovirt-4.1.6
: 4.1.6
Assignee: Daniel Erez
QA Contact: Natalie Gavrielov
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-08-14 06:59 UTC by Daniel Erez
Modified: 2017-09-19 10:02 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
When downloading a disk with snapshots (from a file domain), we currently (4.1) support download of the active image only. In order to specify the size of the disk [1], we currently (4.1) need to fetch the 'apparentsize' value from vdsm by using Volume.getSize API [2]. The retrieved value is the total size of the (active) image that can be downloaded. [1] When downloading a disk, we require disk's size for the Range header. See example: https://raw.githubusercontent.com/oVirt/ovirt-engine-sdk/master/sdk/examples/download_disk.py. I.e. "size = disk.provisioned_size". [2] E.g. # vdsm-client Volume getSize volumeID=<volumeID> storagepoolID=<storagepoolID> storagedomainID=<storagedomainID> imageID=<imageID> E.g. Output: { "truesize": "0", "apparentsize": "1073741824" }
Clone Of:
Environment:
Last Closed: 2017-09-19 10:02:20 UTC
oVirt Team: Storage
Embargoed:
rule-engine: ovirt-4.1+
rule-engine: blocker+


Attachments (Terms of Use)
logs: vdsm, imageio-daemon, engine, imageio-proxy (1.41 MB, application/zip)
2017-08-29 10:33 UTC, Natalie Gavrielov
no flags Details


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 80581 0 master MERGED core: TransferImage - VerifyUntrustedVolume only for upload 2017-08-14 12:39:01 UTC
oVirt gerrit 80582 0 master MERGED core: TransferImage - rollback only for upload 2017-08-14 13:15:21 UTC
oVirt gerrit 80583 0 master MERGED core: TransferImage - move disk to illegal only on upload failure 2017-08-14 13:15:24 UTC
oVirt gerrit 80632 0 ovirt-engine-4.1 MERGED core: TransferImage - VerifyUntrustedVolume only for upload 2017-08-15 11:45:37 UTC
oVirt gerrit 80633 0 ovirt-engine-4.1 MERGED core: TransferImage - rollback only for upload 2017-08-15 11:45:43 UTC
oVirt gerrit 80634 0 ovirt-engine-4.1 MERGED core: TransferImage - move disk to illegal only on upload failure 2017-08-15 11:45:48 UTC

Description Daniel Erez 2017-08-14 06:59:13 UTC
Description of problem:
Upon completion of a disk with snapshots download from the sdk,
The engine invokes (redundantly) VerifyUntrustedVolumeVDSCommand.
Consequently, TransferDiskImageCommand is finished with failure, 
which triggers a rollback; I.e. deletion of the disk. 

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

How reproducible:
100%

Steps to Reproduce:
1. Create a VM.
2. Create a disk and attach to the VM.
3. Create a snapshot.
4. Invoke image download from the SDK

Actual results:
TransferDiskImageCommand is finished with failure, and the disk is deleted by engine.

Expected results:
1. VerifyUntrustedVolumeVDS should not be triggered for download operation as it’s redundant.
2. Should either successfully download the active volume, or, the operation should be blocked (as we don’t support snapshot collapsing on download).

Additional info:

* The error message:

2017-08-13 19:22:08,475+03 ERROR [org.ovirt.engine.core.bll.storage.disk.image.TransferDiskImageCommand] (DefaultQuartzScheduler1) [a998c16c-f5ac-4047-bdfe-7d6a71c18aa9] Failed to verify transferred image: {}: org.ovirt.engine.core.common.errors.EngineException: EngineException: org.ovirt.engine.core.vdsbroker.vdsbroker.VDSErrorException: VDSGenericException: VDSErrorException: Failed to VerifyUntrustedVolumeVDS, error = Image verification failed: u"reason='fdb5d608-aa1e-4db1-b563-ce0a4de01120' is defined as a backingfile, while backingfile is not allowed for an untrusted volume.", code = 484 (Failed with error unexpected and code 16)


* An example for a download script using the python sdk:

https://github.com/oVirt/ovirt-engine-sdk/blob/master/sdk/examples/download_disk.py

Comment 1 Daniel Erez 2017-08-15 13:35:57 UTC
Merged the following suggested solution:
* Download operation does not invoke VerifyUntrustedVolumeVDS.
* No rollback for download failure (i.e. source file is not deleted).
* 
When downloading a disk with snapshots (from a file domain), we currently (4.1) support download of the active image only. In order to specify the size of the disk [1], we currently (4.1) need to fetch the 'apparentsize' value from vdsm by using Volume.getSize API [2]. The retrieved value is the total size of the (active) image that can be downloaded.

[1] When downloading a disk, we require disk's size for the Range header. 
See example: https://raw.githubusercontent.com/oVirt/ovirt-engine-sdk/master/sdk/examples/download_disk.py
I.e. "size = disk.provisioned_size".

[2]
E.g.
# vdsm-client Volume getSize volumeID=<volumeID> storagepoolID=<storagepoolID> storagedomainID=<storagedomainID> imageID=<imageID>

E.g. Output:
{
    "truesize": "0", 
    "apparentsize": "1073741824"
}

Comment 2 rhev-integ 2017-08-15 16:50:40 UTC
INFO: Bug status wasn't changed from MODIFIED to ON_QA due to the following reason:

[Tag 'ovirt-engine-4.1.5.2' doesn't contain patch 'https://gerrit.ovirt.org/80633']
gitweb: https://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=shortlog;h=refs/tags/ovirt-engine-4.1.5.2

For more info please contact: infra

Comment 3 Yaniv Kaul 2017-08-16 14:35:04 UTC
Is this in the build or not?

Comment 4 Allon Mureinik 2017-08-21 15:42:40 UTC
(In reply to Yaniv Kaul from comment #3)
> Is this in the build or not?

It missed the build, unfortunately.
If there's another respin, I'd love to have this one in, but I won't respin just for it.

Comment 5 Natalie Gavrielov 2017-08-29 10:33:52 UTC
Created attachment 1319400 [details]
logs: vdsm, imageio-daemon, engine, imageio-proxy

Daniel,

If I understand correctly, download disk with snapshots is supposed work now - just without the side effect of the disk deletion.

When I try to download a disk (using pretty much the same script) that has snapshots on a file domain I get the following:
$ ./download-disk.py 
Can't open '/usr/share/abrt/conf.d/plugins/python.conf': No such file or directory
Can't open '/etc/abrt/plugins/python.conf': No such file or directory
Traceback (most recent call last):
  File "./download-disk.py", line 132, in <module>
    mydisk.write(r.read())
  File "/usr/lib64/python2.7/httplib.py", line 581, in read
    s = self._safe_read(self.length)
  File "/usr/lib64/python2.7/httplib.py", line 690, in _safe_read
    raise IncompleteRead(''.join(s), amt)
httplib.IncompleteRead: IncompleteRead(524288 bytes read, 7864320 more expected)

Note that for disks that have no snapshots - download works fine and for a disk with snapshots on a block domain - download starts but fails after 1GB (will open a bug ASAP). 

From engine.log:
2017-08-29 10:56:01,941+03 WARN  [org.ovirt.engine.api.restapi.util.LinkHelper] (default task-15) [] Can't find relative path for class "org.ovirt.engine.api.resource.ImagesResource", will return null

Anything I need to change in the script for it to work?

Comment 6 Daniel Erez 2017-08-29 13:00:26 UTC
(In reply to Natalie Gavrielov from comment #5)
> Created attachment 1319400 [details]
> logs: vdsm, imageio-daemon, engine, imageio-proxy
> 
> Daniel,
> 
> If I understand correctly, download disk with snapshots is supposed work now
> - just without the side effect of the disk deletion.
> 
> When I try to download a disk (using pretty much the same script) that has
> snapshots on a file domain I get the following:
> $ ./download-disk.py 
> Can't open '/usr/share/abrt/conf.d/plugins/python.conf': No such file or
> directory
> Can't open '/etc/abrt/plugins/python.conf': No such file or directory
> Traceback (most recent call last):
>   File "./download-disk.py", line 132, in <module>
>     mydisk.write(r.read())
>   File "/usr/lib64/python2.7/httplib.py", line 581, in read
>     s = self._safe_read(self.length)
>   File "/usr/lib64/python2.7/httplib.py", line 690, in _safe_read
>     raise IncompleteRead(''.join(s), amt)
> httplib.IncompleteRead: IncompleteRead(524288 bytes read, 7864320 more
> expected)
> 
> Note that for disks that have no snapshots - download works fine and for a
> disk with snapshots on a block domain - download starts but fails after 1GB
> (will open a bug ASAP). 
> 
> From engine.log:
> 2017-08-29 10:56:01,941+03 WARN 
> [org.ovirt.engine.api.restapi.util.LinkHelper] (default task-15) [] Can't
> find relative path for class "org.ovirt.engine.api.resource.ImagesResource",
> will return null
> 
> Anything I need to change in the script for it to work?

Yes, as explained in comment 1, you should use the apparentsize instead of provisioned_size (see comment 1 for full details).
Note: this is an intermediate solution and will be improved in 4.2 (i.e. the relevant disk size will be provided by in the response header).

Comment 7 Natalie Gavrielov 2017-08-29 13:07:06 UTC
(In reply to Daniel Erez from comment #6)
> Yes, as explained in comment 1, you should use the apparentsize instead of
> provisioned_size (see comment 1 for full details).
> Note: this is an intermediate solution and will be improved in 4.2 (i.e. the
> relevant disk size will be provided by in the response header).

So, you mean just to switch between disk.provisioned_size and disk.apparentsize?

Comment 8 Daniel Erez 2017-08-29 13:09:24 UTC
(In reply to Natalie Gavrielov from comment #7)
> (In reply to Daniel Erez from comment #6)
> > Yes, as explained in comment 1, you should use the apparentsize instead of
> > provisioned_size (see comment 1 for full details).
> > Note: this is an intermediate solution and will be improved in 4.2 (i.e. the
> > relevant disk size will be provided by in the response header).
> 
> So, you mean just to switch between disk.provisioned_size and
> disk.apparentsize?

No, you need to fetch apparentsize value from vdsm. See full details in comment 1.

Comment 9 Natalie Gavrielov 2017-08-29 15:41:06 UTC
Scenario tested:
1. Create a vm with disk on file storage
2. Create snapshot
3. Get the apparentsize of the leaf (the active volume) using vdsm-client
4. Download image using python-sdk with the example script: download-image.py, but instead of the line: size = disk.provisioned_size supply the apparentsize size received from vdsm-client.

VerifyUntrustedVolumeVDS is not triggered now, and the leaf volume was downloaded.

Builds:
rhevm-4.1.6-0.1.el7.noarch
ovirt-imageio-proxy-1.0.0-0.el7ev.noarch
ovirt-imageio-common-1.0.0-0.el7ev.noarch


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