Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-02 Thread mskrivan 
Michal Skrivanek has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

(1 comment)

however, I think we do have a problem conceptually with RAM snapshots and 
LUN/cinder.
We can't ensure consistent state unless we snapshot cinder/LUN as well (or make 
sure they are not there at all)
I wonder if we should block that or warn at least

https://gerrit.ovirt.org/#/c/49535/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 3137
Line 3138
Line 3139
Line 3140
Line 3141
> Until now, memory snapshot without disk did nothing. This looks like a bug,
I think disk-less mem snapshots are merely a bug. It does deserve a fix, though 
it's not a high priority


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-02 Thread ahadas 
Arik Hadas has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

> @Michal/Arik - shouldn't we support previewing a memory snapshot
 > without disks?

We should.
It seems like a826455f broke this flow - they didn't take into account this 
case where the memory should be saved when there are no disks to take snapshot 
for.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/49535/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 3137
Line 3138
Line 3139
Line 3140
Line 3141
This allows memory snapshot without any disks - does this snapshot make sense? 
can you preview this snapshot?

When using cinder disk, the memory snapshot is stored on another domain. Do we 
save this vm in the ovf store of both domains? What happens when you detach the 
domain containing the memory snapshot?

Having vm with memory snapshots on another domain (the user have no control on 
this domain) seems like unwanted behavior.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

Daniel, please check why jenkins is failing, explain if the failure is relevant 
to this patch, report ci failures to infra mailing list, and test failures to 
devel mailing list.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread derez 
Daniel Erez has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/49535/1//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2015-12-01 21:00:52 +0200
Line 4: Commit: Daniel Erez 
Line 5: CommitDate: 2015-12-01 21:09:15 +0200
Line 6: 
Line 7: vm: snapshot - enabling memory snapshot for diskless VM
Vm with cinder disk is not diskless  - how about:

enabling memory snapshot without disks

And then explain the possible configurations and settings that lead to this 
flow:

- Unchecking the disks in the snapshot dialog
- Vm with only cinder disks
- Vm without disks
Line 8: 
Line 9: Taking a memory snapshot of a VM without disks should be
Line 10: supported (or merely with Cinder/LUN disks for that matter).
Line 11: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/49535/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 3137
Line 3138
Line 3139
Line 3140
Line 3141
> Previewing a snapshot without disks can be previewed. Though it's probably 
Until now, memory snapshot without disk did nothing. This looks like a bug, but 
I don't think we should change it.

The only change needed is supporting memory snapshot with cinder disk. Do we 
have an easy way to tell that we have a cinder disk here? We can use the frozen 
boolean, but it is too sneaky.

Lets think about a better way to do this. There is no rush, cinder is a new 
feature, and I don't see any problem if will not support memory snapshot in the 
current build.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread derez 
Daniel Erez has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/49535/1//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2015-12-01 21:00:52 +0200
Line 4: Commit: Daniel Erez 
Line 5: CommitDate: 2015-12-01 21:09:15 +0200
Line 6: 
Line 7: vm: snapshot - enabling memory snapshot for diskless VM
> Vm with cinder disk is not diskless  - how about:
Done
Line 8: 
Line 9: Taking a memory snapshot of a VM without disks should be
Line 10: supported (or merely with Cinder/LUN disks for that matter).
Line 11: 


https://gerrit.ovirt.org/#/c/49535/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 3137
Line 3138
Line 3139
Line 3140
Line 3141
> This allows memory snapshot without any disks - does this snapshot make sen
Previewing a snapshot without disks can be previewed. Though it's probably not 
very useful, it does make sense in some scenarios.

We currently require a master domain in order to use Cinder. So, the ram disk 
and ovf will be saved on one of the data domains. Detaching is blocked by the 
engine in case some VMs are using it. And it's not related to this patch any 
way... :) 

According to the current behavior, we already have some logic for selecting the 
target storage domain (free space, etc). Again, not related to this patch at 
all :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread derez 
Daniel Erez has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

That's not exactly correct - previewing a memory snapshot without disks 
prevents running the VM. That's worse than merely 'did nothing'. Running the VM 
would fail with an ugly exception in vdsm.

@Michal/Arik - shouldn't we support previewing a memory snapshot without disks?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread derez 
Daniel Erez has uploaded a new change for review.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..

vm: snapshot - enabling memory snapshot for diskless VM

Taking a memory snapshot of a VM without disks should be
supported (or merely with Cinder/LUN disks for that matter).

Hence, removing the early return on 'VM -> snapshot'
in case 'newDrives' is empty.

Instead, proper logging will be added on consecutive patches.

Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Bug-Url: https://bugzilla.redhat.com/1287066
Signed-off-by: Daniel Erez 
---
M vdsm/virt/vm.py
1 file changed, 0 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/49535/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 8ec8b1d..8503f64 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -3134,11 +3134,6 @@
 # safely access the blockDev property until after prepareVolumePath
 vmDrives[vmDevName] = vmDrive
 
-# If all the drives are the current ones, return success
-if len(newDrives) == 0:
-self.log.debug('all the drives are already in use, success')
-return {'status': doneCode}
-
 preparedDrives = {}
 
 for vmDevName, vmDevice in newDrives.iteritems():


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vm: snapshot - enabling memory snapshot for diskless VM

2015-12-01 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: vm: snapshot - enabling memory snapshot for diskless VM
..


Patch Set 1:

* #1287066::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1287066::OK, public bug
* Check Product::#1287066::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches