Bug 1295429

Summary: Remove file=path workaround for live snapshot on block storage due to libvirt prior to 1.2.2
Product: [oVirt] vdsm Reporter: Allon Mureinik <amureini>
Component: CoreAssignee: Ala Hino <ahino>
Status: CLOSED CURRENTRELEASE QA Contact: Aharon Canan <acanan>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 4.17.14CC: ahino, bugs, fromani, michal.skrivanek
Target Milestone: ovirt-4.0.0-rcKeywords: CodeChange
Target Release: 4.18.0Flags: rule-engine: ovirt-4.0.0+
rule-engine: planning_ack+
amureini: devel_ack+
rule-engine: testing_ack+
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-07-05 07:48:40 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:

Description Allon Mureinik 2016-01-04 13:16:01 UTC
Description of problem:
libvirt versions prior to 1.2.2 do not handle type='block' properly, and require VDSM to send the legacy file=path syntax too. Since we now (checked on commit 04f610b644d605fe9fb6f7ecc2a1178fbf30668d) require libvirt 1.2.8 (at least, depending on the platform), this workaround in vm.py is redundant, and should be removed.

The relevant piece of code, in vm.py circa line 3066:

       def _diskSnapshot(vmDev, newPath, sourceType):
            """Libvirt snapshot XML"""

            disk = vmxml.Element('disk', name=vmDev, snapshot='external',
                                 type=sourceType)
            # Libvirt versions before 1.2.2 do not understand 'type' and treat
            # all snapshots as if they are type='file'.  In order to ensure
            # proper handling of block snapshots in modern libvirt versions,
            # we specify type='block' and dev=path for block volumes but we
            # always speficy the file=path for backwards compatibility.
            args = {'type': sourceType, 'file': newPath}
            if sourceType == 'block':
                args['dev'] = newPath
            disk.appendChildWithArgs('source', **args)
            return disk

Comment 1 Sandro Bonazzola 2016-05-02 09:55:55 UTC
Moving from 4.0 alpha to 4.0 beta since 4.0 alpha has been already released and bug is not ON_QA.

Comment 2 Yaniv Lavi 2016-05-23 13:17:30 UTC
oVirt 4.0 beta has been released, moving to RC milestone.

Comment 3 Yaniv Lavi 2016-05-23 13:24:03 UTC
oVirt 4.0 beta has been released, moving to RC milestone.

Comment 4 Aharon Canan 2016-06-13 09:21:27 UTC
Do we have something to test here or only regression?

Comment 5 Allon Mureinik 2016-06-13 12:13:51 UTC
(In reply to Aharon Canan from comment #4)
> Do we have something to test here or only regression?
Just regression testing.
I'd especially be interested in a live migration from an old VDSM to a new VDSM (in a 3.6 cluster, to keep things simple).

Comment 6 Ala Hino 2016-06-14 10:56:49 UTC
Needinfo answered by Allon in comment #5

Comment 7 Sandro Bonazzola 2016-07-05 07:48:40 UTC
oVirt 4.0.0 has been released, closing current release.