Change in vdsm[master]: vm: Add qcow2_compat on create

[Maor Lipchuk]

Maor Lipchuk has uploaded a new change for review.

Change subject: vm: Add qcow2_compat on create
......................................................................

vm: Add qcow2_compat on create

Pass qcow2 compat on create.

Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Signed-off-by: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
---
M vdsm/virt/vm.py
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/77/64377/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 1d9a3d5..90bf998 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2701,7 +2701,8 @@
         try:
             qemuimg.create(transientPath, format=qemuimg.FORMAT.QCOW2,
                            backing=diskParams['path'],
-                           backingFormat=driveFormat)
+                           backingFormat=driveFormat,
+                           qcow2_compat=self._dom)
             os.fchmod(transientHandle, 0o660)
         except Exception:
             os.unlink(transientPath)  # Closing after deletion is correct


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 6:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/64377/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2701:         try:
Line 2702:             qemuimg.create(transientPath, format=qemuimg.FORMAT.QCOW2,
Line 2703:                            backing=diskParams['path'],
Line 2704:                            backingFormat=driveFormat,
Line 2705:                            qcow2_compat=self._dom.qcow2_compat())
self._dom is not a storage domain, this is a libvirt.virDomain object.

Hopefully diskParams contains a domain id, so we can produce a domain manifest object.
Line 2706:             os.fchmod(transientHandle, 0o660)
Line 2707:         except Exception:
Line 2708:             os.unlink(transientPath)  # Closing after deletion is correct
Line 2709:             self.log.exception("Failed to create the transient disk for
"


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 7:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 8:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 9:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 10:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 11:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 12:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 13:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 14:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 15:

* update_tracker: OK
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 15: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64377/15/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2701:         try:
Line 2702:             qemuimg.create(transientPath, format=qemuimg.FORMAT.QCOW2,
Line 2703:                            backing=diskParams['path'],
Line 2704:                            backingFormat=driveFormat,
Line 2705:                            qcow2Compat=self._dom.qcow2_compat())
I commented before - you need a storage domain manifest object, vm._dom is a
libvirt.virDomain object.
Line 2706:             os.fchmod(transientHandle, 0o660)
Line 2707:         except Exception:
Line 2708:             os.unlink(transientPath)  # Closing after deletion is correct
Line 2709:             self.log.exception("Failed to create the transient disk for
"


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 15:

(1 comment)

https://gerrit.ovirt.org/#/c/64377/15/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2701:         try:
Line 2702:             qemuimg.create(transientPath, format=qemuimg.FORMAT.QCOW2,
Line 2703:                            backing=diskParams['path'],
Line 2704:                            backingFormat=driveFormat,
Line 2705:                            qcow2Compat=self._dom.qcow2_compat())
>  I commented before - you need a storage domain manifest object,
> vm._dom is  
Done
Line 2706:             os.fchmod(transientHandle, 0o660)
Line 2707:         except Exception:
Line 2708:             os.unlink(transientPath)  # Closing after deletion is correct
Line 2709:             self.log.exception("Failed to create the transient disk for
"


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 16:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 17:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 18:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 18:

(2 comments)

https://gerrit.ovirt.org/#/c/64377/18/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2698
Line 2699
Line 2700
Line 2701
Line 2702
Please fix the format here to use one parameter per line, and then add the compat after
format, not after backingFormat.


Line 2698:         transientHandle, transientPath = tempfile.mkstemp(
Line 2699:             dir=config.get('vars',
'transient_disks_repository'),
Line 2700:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2701: 
Line 2702:         sdDom = sdCache.produce(sdUUID=diskParams['domainID'])
We should produce the safer manifest object for this.
Line 2703:         try:
Line 2704:             qemuimg.create(transientPath, format=qemuimg.FORMAT.QCOW2,
Line 2705:                            backing=diskParams['path'],
Line 2706:                            backingFormat=driveFormat,


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 19:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 19: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/64377/19/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 21: 
Line 22: # stdlib imports
Line 23: from collections import namedtuple
Line 24: from contextlib import contextmanager
Line 25: from storage.sdc import sdCache
We already importing sdc,  see LiveMergeCleanupThread.teardown_top_volume.
Line 26: from xml.dom.minidom import parseString as _domParseStr
Line 27: import itertools
Line 28: import logging
Line 29: import os


Line 2699:         transientHandle, transientPath = tempfile.mkstemp(
Line 2700:             dir=config.get('vars',
'transient_disks_repository'),
Line 2701:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2702: 
Line 2703:         sdDom =
sdCache.produce(sdUUID=diskParams['domainID']).manifest
Produce manifest, this is a safer and easier to test object.
Line 2704:         try:
Line 2705:             qemuimg.create(transientPath,
Line 2706:                            format=qemuimg.FORMAT.QCOW2,
Line 2707:                            backing=diskParams['path'],


Line 2702: 
Line 2703:         sdDom =
sdCache.produce(sdUUID=diskParams['domainID']).manifest
Line 2704:         try:
Line 2705:             qemuimg.create(transientPath,
Line 2706:                            format=qemuimg.FORMAT.QCOW2,
Put qcow2Compat here (this is the third time I'm commenting about this in this
patch).
Line 2707:                            backing=diskParams['path'],
Line 2708:                            backingFormat=driveFormat,
Line 2709:                            qcow2Compat=sdDom.qcow2_compat())
Line 2710:             os.fchmod(transientHandle, 0o660)


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 19:

(4 comments)

https://gerrit.ovirt.org/#/c/64377/18/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2698
Line 2699
Line 2700
Line 2701
Line 2702
>  Please fix the format here to use one parameter per line, and then
> add the  
Done


https://gerrit.ovirt.org/#/c/64377/19/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 21: 
Line 22: # stdlib imports
Line 23: from collections import namedtuple
Line 24: from contextlib import contextmanager
Line 25: from storage.sdc import sdCache
>  We already importing sdc,  see
> LiveMergeCleanupThread.teardown_top_volume. 
Done
Line 26: from xml.dom.minidom import parseString as _domParseStr
Line 27: import itertools
Line 28: import logging
Line 29: import os


Line 2699:         transientHandle, transientPath = tempfile.mkstemp(
Line 2700:             dir=config.get('vars',
'transient_disks_repository'),
Line 2701:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2702: 
Line 2703:         sdDom =
sdCache.produce(sdUUID=diskParams['domainID']).manifest
>  Produce manifest, this is a safer and easier to test object.
Done
Line 2704:         try:
Line 2705:             qemuimg.create(transientPath,
Line 2706:                            format=qemuimg.FORMAT.QCOW2,
Line 2707:                            backing=diskParams['path'],


Line 2702: 
Line 2703:         sdDom =
sdCache.produce(sdUUID=diskParams['domainID']).manifest
Line 2704:         try:
Line 2705:             qemuimg.create(transientPath,
Line 2706:                            format=qemuimg.FORMAT.QCOW2,
>  Put qcow2Compat here (this is the third time I'm commenting about
> this in t 
First time I see that in this patch, I think you are referring to a
different patch of copy.
Any how, will fix that in the next upload
Line 2707:                            backing=diskParams['path'],
Line 2708:                            backingFormat=driveFormat,
Line 2709:                            qcow2Compat=sdDom.qcow2_compat())
Line 2710:             os.fchmod(transientHandle, 0o660)


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 20:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 20:

(2 comments)

https://gerrit.ovirt.org/#/c/64377/20/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2698:         transientHandle, transientPath = tempfile.mkstemp(
Line 2699:             dir=config.get('vars',
'transient_disks_repository'),
Line 2700:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2701: 
Line 2702:         sdDom =
sdc.sdCache.produce_manifest(sdUUID=diskParams['domainID'])
procude_manifest accepts one positional argument, calling it using kwargs style
(arg=value) does not make the code better.

Please see the result of "git grep produce_manifest" and use use the same
style.
Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            backing=diskParams['path'],


Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            backing=diskParams['path'],
Line 2707:                            qcow2Compat=sdDom.qcow2_compat(),
This is the compat of the image, not the backing file. Please use the same order of the
parameters as in qemuimg.create.
Line 2708:                            backingFormat=driveFormat)
Line 2709:             os.fchmod(transientHandle, 0o660)
Line 2710:         except Exception:
Line 2711:             os.unlink(transientPath)  # Closing after deletion is correct


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 20:

(2 comments)

https://gerrit.ovirt.org/#/c/64377/20/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2698:         transientHandle, transientPath = tempfile.mkstemp(
Line 2699:             dir=config.get('vars',
'transient_disks_repository'),
Line 2700:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2701: 
Line 2702:         sdDom =
sdc.sdCache.produce_manifest(sdUUID=diskParams['domainID'])
>  procude_manifest accepts one positional argument, calling it using
> kwargs s 
Done
Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            backing=diskParams['path'],


Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            backing=diskParams['path'],
Line 2707:                            qcow2Compat=sdDom.qcow2_compat(),
>  This is the compat of the image, not the backing file. Please use the
> same  
Done
Line 2708:                            backingFormat=driveFormat)
Line 2709:             os.fchmod(transientHandle, 0o660)
Line 2710:         except Exception:
Line 2711:             os.unlink(transientPath)  # Closing after deletion is correct


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21: Code-Review+1

Waiting for Francesco ack.

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/64377/21/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2707
Line 2708
Line 2709
Line 2710
Line 2711
Otherwise we leak a file descriptor on failures.


Line 2698:         transientHandle, transientPath = tempfile.mkstemp(
Line 2699:             dir=config.get('vars',
'transient_disks_repository'),
Line 2700:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2701: 
Line 2702:         sdDom = sdc.sdCache.produce_manifest(diskParams['domainID'])
This line must be inside the try block...
Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            qcow2Compat=sdDom.qcow2_compat(),


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21:

(1 comment)

https://gerrit.ovirt.org/#/c/64377/21/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2698:         transientHandle, transientPath = tempfile.mkstemp(
Line 2699:             dir=config.get('vars',
'transient_disks_repository'),
Line 2700:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2701: 
Line 2702:         sdDom = sdc.sdCache.produce_manifest(diskParams['domainID'])
>  This line must be inside the try block... 
It seems like the
tempfile at line 2698 should be in the try block, and the sdDom should be outside of
it...

I hate to make large try blocks, maybe as an alternative I will just fetch the domain
before line 2698?
Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            qcow2Compat=sdDom.qcow2_compat(),


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 21:

(1 comment)

https://gerrit.ovirt.org/#/c/64377/21/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2698:         transientHandle, transientPath = tempfile.mkstemp(
Line 2699:             dir=config.get('vars',
'transient_disks_repository'),
Line 2700:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2701: 
Line 2702:         sdDom = sdc.sdCache.produce_manifest(diskParams['domainID'])
>  It seems like the tempfile at line 2698 should be in the try block,
> and the 
The way the termpfile is created and closed is correct:

    open file
    try:
       use it
    except ...
       clean up
    finally:
       close it

If you cannot open the file, we raise before the try, and there is nothing to close.

Please move the sd creation into the try.
Line 2703:         try:
Line 2704:             qemuimg.create(transientPath,
Line 2705:                            format=qemuimg.FORMAT.QCOW2,
Line 2706:                            qcow2Compat=sdDom.qcow2_compat(),


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 22:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 22: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/64377/22/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2694:             qemuimg.FORMAT.QCOW2 if diskParams['format'] ==
'cow' else
Line 2695:             qemuimg.FORMAT.RAW
Line 2696:         )
Line 2697: 
Line 2698:         sdDom = sdc.sdCache.produce_manifest(diskParams['domainID'])
Please see my comment in the previous version.
Line 2699:         transientHandle, transientPath = tempfile.mkstemp(
Line 2700:             dir=config.get('vars',
'transient_disks_repository'),
Line 2701:             prefix="%s-%s." % (diskParams['domainID'],
diskParams['volumeID']))
Line 2702: 


-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 23:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 23: Code-Review+1

Waiting for Francesco ack.

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 23: Code-Review+2

looks ok to me from virt perspective

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 23:

please verify

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 23: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has submitted this change and it was merged.

Change subject: vm: Add qcow2_compat on create
......................................................................


vm: Add qcow2_compat on create

Pass qcow2 compat on create.

Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Signed-off-by: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/64377
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani <fromani(a)redhat.com&gt;
---
M vdsm/virt/vm.py
1 file changed, 4 insertions(+), 1 deletion(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Francesco Romani: Looks good to me, approved
  Maor Lipchuk: Verified



-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: vm: Add qcow2_compat on create
......................................................................


Patch Set 24:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/64377
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I331eee6a12853c2fa65e62f42c51504f88723538
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipchuk(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No