Change in vdsm[master]: raise ongoing creations count

Saggi Mizrahi has uploaded a new change for review.

Change subject: raise ongoing creations count
......................................................................

raise ongoing creations count

Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
---
M vdsm/vm.py
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/679/1
--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi

Adam Litke has posted comments on this change.

Change subject: raise ongoing creations count
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/vm.py
Line 267:     _ongoingCreations = threading.BoundedSemaphore(10)
Cool!  So now we can create 10 VMs at the same time.  Is there a reason that it was limited to one at a time originally?  Did you need to change anything to make this safe?

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke

Eduardo has posted comments on this change.

Change subject: raise ongoing creations count
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/vm.py
Line 267:     _ongoingCreations = threading.BoundedSemaphore(10)
A previous patch of Dan raised it to 3 and failed preintegration. Please look it.

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Eduardo

Saggi Mizrahi has posted comments on this change.

Change subject: raise ongoing creations count
......................................................................

Patch Set 1: (2 inline comments)

....................................................
File vdsm/vm.py
Line 267:     _ongoingCreations = threading.BoundedSemaphore(10)
I know this is why I put it in the end. But we've been running 100 VMs at a time and it appears to work. Anyway I don't care if this don't get pushed in. It will just make vm startup slower but will not hinder functionality.

Line 267:     _ongoingCreations = threading.BoundedSemaphore(10)
I really don't know, this lock is actually in the wrong place as is. I just changed it so we could do it as running a 1000 VMs 1 at a time is a lot slower with the 1 limit. It worked for us. I don't really know the limit is here. Just thought I'd suggest the fix.

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

Ayal Baron has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

why isn't this configurable?

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 3:

It's a great thing, but as I can remember the problem with this in previous round was for VMs with multiple disks. I assume  you checked creation of 100 VMs with single disk each. But if it works for VMs with several disks, you got my ACK

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/vm.py
Line 267:     _ongoingCreations = threading.BoundedSemaphore(10)
Increasing this limit is a good idea. However, it makes sense to relate the maximum value with the number of host cpus - there is no much point in stressing the host to run a new qemu process while all its cpus are busy doing this.

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 4: Looks good to me, but someone else must approve

Please open  a ticket to pre-integration team, to check simultaneously creation of VMs with multiple disks

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

Ayal Baron has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 4: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 5: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/proc.py
Line 21: def getProcessorCount():
why do we need this file, when we already have caps.CpuInfo().cores() ?

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 6: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 6: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Raise ongoing creations count
......................................................................

Patch Set 7: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Raise ongoing creations count
......................................................................

Raise ongoing creations count

Multi-core hosts are generally more capable to start up multiple qemu
processes efficiently. (And, the RHEL-5 kernel bug that limited
_ongoingCreations to 1 is long irrelevant)

Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
---
M vdsm.spec.in
M vdsm/vm.py
2 files changed, 19 insertions(+), 17 deletions(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/679
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I83b4f956619ee6cdfde5fdbec9d52484c355b33f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Saggi Mizrahi