Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2016-02-05 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

(1 comment)

Hey Dan, yes it was a copy+paste type, will get fixed in next patch

https://gerrit.ovirt.org/#/c/48450/7/vdsm_hooks/ide2sata/README
File vdsm_hooks/ide2sata/README:

Line 41: IDE disks
> so why don't you convert virtio disks to SATA?
Indeed, I'll try work on that after this hook


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-02-04 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7: Code-Review-1

(2 comments)

a doc nit, and a question

https://gerrit.ovirt.org/#/c/48450/7/vdsm_hooks/ide2sata/README
File vdsm_hooks/ide2sata/README:

Line 19: diskunmap
copy+paste typo, I presume.


Line 41: IDE disks
so why don't you convert virtio disks to SATA?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-02-01 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

Francesco and the rest of the team, sorry for the delay, I found an issue with 
the hook while using 3 IDE disks. I did the tests with 2 and worked fine, the 
problem comes when you set 3 disks. I'm currently working on a fix for this but 
before I continue I'd like to ask if it's ok/allowed to modify the DB after the 
VM gets powered off, the reason I'm asking is because when you shutdown the VM, 
the vm_device table will contain the 'address' for the third device as "{bus=0, 
controller=0, type=drive, target=0, unit=2}" (this is part of the fix) and this 
will prevent the VM to start when the hook is off or even not configured 
because an IDE bus cannot contain more then 2 units (0 and 1), I need to change 
that to be "{bus=1, controller=0, type=drive, target=0, unit=1}" to avoid that 
issue and also the device duplication, so basically I'm working on a 
'after_vm_destroy' script which connects to the 'engine' DB and modifies that, 
but before I continue I'd like to know if this will !
 be an accepted approach or not. Thanks!!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-27 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

correct. When we disable the hook, Engine should not be confused by the new 
devices. Please try a few times, not just once.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-27 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

Javier, thanks for the update. The testing you did looks ok, I just want to 
make sure the concern Nir raised is addressed.

We need to make sure that the translation this hook does is not confusing 
Engine, and that the added controller is handled OK.
This is especially important if we boot the VM with the hook enabled, and the 
we disable it. The VM must work fine afterwards.

Once that is verified, we could merge this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-27 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

Francesco, no problem at all, I'll setup a 3.6 hosted-engine env and give it a 
try. Just to be clear, the test would be to start a VM with IDE disk with hook 
enabled and then disable it and restart the VM and see if the VM works 
afterwards?. Thanks!!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-27 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

Hello Francesco, I did verify the hook in a 3.6 ovirt installation I had, 
unfortunately I don't have it anymore. I've installed the hook in the host and 
then configured the engine-config variable and installed a VM with that custom 
prop with IDE disks, thin and preallocated, and both worked fine, the interface 
changed to SATA. Please, let me know if you need something in particular. I 
think I can work on a 3.6 install again and test it once more.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-25 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-25 Thread apahim 
Amador Pahim has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-25 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

Javier, could you please share how verification was done, to address the good 
point Nir raised?
Other than that looks OK, so this is the only thing left that holds the merge.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-23 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7: Code-Review+1

My only concern is possible conflict with our code managing device indexes and 
pci addresses (one engine side I think).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-22 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7: Code-Review+1

at glance seems mostly OK, partial ACK, deeper review later.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-21 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 6:

(5 comments)

https://gerrit.ovirt.org/#/c/48450/6/vdsm.spec.in
File vdsm.spec.in:

Line 611: BuildArch:  noarch
Line 612: Requires:   qemu-kvm >= 0.14
Line 613: 
Line 614: %description hook-ide2sata
Line 615: VDSM hooks which changes IDE to SATA disks.
> changes IDE to SATA disks -> changes IDE disks to SATA.
Done
Line 616: 
Line 617: %if 0%{?with_gluster_mgmt}
Line 618: %package gluster
Line 619: Summary:Gluster Plugin for VDSM


https://gerrit.ovirt.org/#/c/48450/6/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 42: for disk in domxml.getElementsByTagName('disk'):
Line 43: device = disk.getAttribute('device')
Line 44: target = disk.getElementsByTagName('target')[0]
Line 45: bus = target.getAttribute('bus')
Line 46: dev = target.getAttribute('dev')
> This is needed only if this is a idea bus, and the device is disk or lun, s
Done
Line 47: if ((device == 'disk' or device == 'lun')
Line 48:and (bus == 'ide')):
Line 49: target.setAttribute('bus', 'sata')
Line 50: target.setAttribute('dev', 's%s' % dev[1:])


Line 44: target = disk.getElementsByTagName('target')[0]
Line 45: bus = target.getAttribute('bus')
Line 46: dev = target.getAttribute('dev')
Line 47: if ((device == 'disk' or device == 'lun')
Line 48:and (bus == 'ide')):
> This conditional can be simpler and more clear:
Done
Line 49: target.setAttribute('bus', 'sata')
Line 50: target.setAttribute('dev', 's%s' % dev[1:])
Line 51: 
Line 52: 


Line 82: 
Line 83: 
Line 85: 
Line 86: 
> This is not a correct domain xml - please include the  element.
Done
Line 87: '''
Line 88: 
Line 89: xmldom = minidom.parseString(text)
Line 90: print("\nDisk device definition BEFORE hook execution:\n\n%s"


Line 90: print("\nDisk device definition BEFORE hook execution:\n\n%s"
Line 91:   % xmldom.toxml(encoding='UTF-8'))
Line 92: change2Sata(xmldom)
Line 93: print("\nDisk device definition AFTER hook execution: \n\n%s"
Line 94:   % xmldom.toxml(encoding='UTF-8'))
> This does not test much.
Sorry, I haven't had time to work on a test module for this hook, I think it 
can go as it is for now, please, let me know if that's ok with you. Thanks!!
Line 95: 
Line 96: if __name__ == '__main__':
Line 97: try:
Line 98: if '--test' in sys.argv:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2016-01-21 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 7:

* 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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-24 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 6:

(5 comments)

https://gerrit.ovirt.org/#/c/48450/6/vdsm.spec.in
File vdsm.spec.in:

Line 611: BuildArch:  noarch
Line 612: Requires:   qemu-kvm >= 0.14
Line 613: 
Line 614: %description hook-ide2sata
Line 615: VDSM hooks which changes IDE to SATA disks.
changes IDE to SATA disks -> changes IDE disks to SATA.
Line 616: 
Line 617: %if 0%{?with_gluster_mgmt}
Line 618: %package gluster
Line 619: Summary:Gluster Plugin for VDSM


https://gerrit.ovirt.org/#/c/48450/6/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 42: for disk in domxml.getElementsByTagName('disk'):
Line 43: device = disk.getAttribute('device')
Line 44: target = disk.getElementsByTagName('target')[0]
Line 45: bus = target.getAttribute('bus')
Line 46: dev = target.getAttribute('dev')
This is needed only if this is a idea bus, and the device is disk or lun, so 
better create it inside the if block.
Line 47: if ((device == 'disk' or device == 'lun')
Line 48:and (bus == 'ide')):
Line 49: target.setAttribute('bus', 'sata')
Line 50: target.setAttribute('dev', 's%s' % dev[1:])


Line 44: target = disk.getElementsByTagName('target')[0]
Line 45: bus = target.getAttribute('bus')
Line 46: dev = target.getAttribute('dev')
Line 47: if ((device == 'disk' or device == 'lun')
Line 48:and (bus == 'ide')):
This conditional can be simpler and more clear:

if bus == ide and device in ("disk", "lun"):
...
Line 49: target.setAttribute('bus', 'sata')
Line 50: target.setAttribute('dev', 's%s' % dev[1:])
Line 51: 
Line 52: 


Line 82: 
Line 83: 
Line 85: 
Line 86: 
This is not a correct domain xml - please include the  element.
Line 87: '''
Line 88: 
Line 89: xmldom = minidom.parseString(text)
Line 90: print("\nDisk device definition BEFORE hook execution:\n\n%s"


Line 90: print("\nDisk device definition BEFORE hook execution:\n\n%s"
Line 91:   % xmldom.toxml(encoding='UTF-8'))
Line 92: change2Sata(xmldom)
Line 93: print("\nDisk device definition AFTER hook execution: \n\n%s"
Line 94:   % xmldom.toxml(encoding='UTF-8'))
This does not test much.

Why not include a real unitest module in this directory, testing the entire 
hook?

You can write xml to disk, run the hook, and check that it modified the stored 
xml.
Line 95: 
Line 96: if __name__ == '__main__':
Line 97: try:
Line 98: if '--test' in sys.argv:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-24 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 4:

(5 comments)

https://gerrit.ovirt.org/#/c/48450/4/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 1: #!/usr/bin/env python2
Line 2: 
Line 3: #
Line 4: # Copyright 2014 Red Hat, Inc.
> 2015
Done
Line 5: #
Line 6: # This program is free software; you can redistribute it and/or modify
Line 7: # it under the terms of the GNU General Public License as published by
Line 8: # the Free Software Foundation; either version 2 of the License, or


Line 41: def createControllerElement(domxml):
Line 42: devices = domxml.getElementsByTagName('devices')[0]
Line 43: controller = domxml.createElement('controller')
Line 44: controller.setAttribute('type', 'sata')
Line 45: devices.appendChild(controller)
> What if there is already such controller in the vm?
Testing further, libvirt automatically creates the controller with type sata 
once the bus changes from ide to sata, no need for this function then
Line 46: 
Line 47: return controller
Line 48: 
Line 49: 


Line 42: devices = domxml.getElementsByTagName('devices')[0]
Line 43: controller = domxml.createElement('controller')
Line 44: controller.setAttribute('type', 'sata')
Line 45: devices.appendChild(controller)
Line 46: 
> The whitespace above is not needed.
Done
Line 47: return controller
Line 48: 
Line 49: 
Line 50: def change2Sata(domxml):


Line 55: dev = target.getAttribute('dev')
Line 56: if ((device == 'disk' or device == 'lun')
Line 57:and (bus == 'ide')):
Line 58: target.setAttribute('bus', 'sata')
Line 59: target.setAttribute('dev', 'sd%s' % dev[-1])
> It would be good idea to document this in readme.
Done
Line 60: 
Line 61: 
Line 62: def main():
Line 63: if 'ide2sata' in os.environ:


Line 61: 
Line 62: def main():
Line 63: if 'ide2sata' in os.environ:
Line 64: sataConfig = os.environ['ide2sata']
Line 65: if sataConfig == 'on':
> This can simplfied:
Done
Line 66: domxml = hooking.read_domxml()
Line 67: createControllerElement(domxml)
Line 68: change2Sata(domxml)
Line 69: hooking.write_domxml(domxml)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-24 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 6:

* 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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-23 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/48450/4/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 55: dev = target.getAttribute('dev')
Line 56: if ((device == 'disk' or device == 'lun')
Line 57:and (bus == 'ide')):
Line 58: target.setAttribute('bus', 'sata')
Line 59: target.setAttribute('dev', 'sd%s' % dev[-1])
> This assumes that drives have only one letter, which may be correct for ide
I'll revert back the device name characters to the original patch set where the 
setAttribute didn't care how many characters will follow the 'sd'. One question 
regarding the hot-plug/unplug tests, IDE disks cannot be hot-plugged/unplugged 
while the VM is running, you have to shutdown the VM, activate/deactivate the 
disks and then start the VM again. Is there anything I need to test here? 
thanks!!
Line 60: 
Line 61: 
Line 62: def main():
Line 63: if 'ide2sata' in os.environ:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-23 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-23 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-23 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 4:

(5 comments)

Partial review.

https://gerrit.ovirt.org/#/c/48450/4/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 1: #!/usr/bin/env python2
Line 2: 
Line 3: #
Line 4: # Copyright 2014 Red Hat, Inc.
2015
Line 5: #
Line 6: # This program is free software; you can redistribute it and/or modify
Line 7: # it under the terms of the GNU General Public License as published by
Line 8: # the Free Software Foundation; either version 2 of the License, or


Line 41: def createControllerElement(domxml):
Line 42: devices = domxml.getElementsByTagName('devices')[0]
Line 43: controller = domxml.createElement('controller')
Line 44: controller.setAttribute('type', 'sata')
Line 45: devices.appendChild(controller)
What if there is already such controller in the vm?
Line 46: 
Line 47: return controller
Line 48: 
Line 49: 


Line 42: devices = domxml.getElementsByTagName('devices')[0]
Line 43: controller = domxml.createElement('controller')
Line 44: controller.setAttribute('type', 'sata')
Line 45: devices.appendChild(controller)
Line 46: 
The whitespace above is not needed.
Line 47: return controller
Line 48: 
Line 49: 
Line 50: def change2Sata(domxml):


Line 55: dev = target.getAttribute('dev')
Line 56: if ((device == 'disk' or device == 'lun')
Line 57:and (bus == 'ide')):
Line 58: target.setAttribute('bus', 'sata')
Line 59: target.setAttribute('dev', 'sd%s' % dev[-1])
> I'll revert back the device name characters to the original patch set where
It would be good idea to document this in readme.
Line 60: 
Line 61: 
Line 62: def main():
Line 63: if 'ide2sata' in os.environ:


Line 61: 
Line 62: def main():
Line 63: if 'ide2sata' in os.environ:
Line 64: sataConfig = os.environ['ide2sata']
Line 65: if sataConfig == 'on':
This can simplfied:

if os.environ.get('ide2sata') == 'on':
hack the xml ...
Line 66: domxml = hooking.read_domxml()
Line 67: createControllerElement(domxml)
Line 68: change2Sata(domxml)
Line 69: hooking.write_domxml(domxml)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-17 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-17 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/48450/4/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 55: dev = target.getAttribute('dev')
Line 56: if ((device == 'disk' or device == 'lun')
Line 57:and (bus == 'ide')):
Line 58: target.setAttribute('bus', 'sata')
Line 59: target.setAttribute('dev', 'sd%s' % dev[-1])
This assumes that drives have only one letter, which may be correct for ide 
drives, but I think the intent was more clear in the first version, using 
dev[1:], which does not make any assumption on the number of drives.

Did you check if hotpulug and hotunplug code works correctly with this hook?
Line 60: 
Line 61: 
Line 62: def main():
Line 63: if 'ide2sata' in os.environ:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
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]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread apahim 
Amador Pahim has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 2: Code-Review-1

(1 comment)

Just a minor issue. The rest looks great.
Thank you.

https://gerrit.ovirt.org/#/c/48450/2/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 74: 
   : 
   : 
   : 
   : 
   : 
   : 
   : 
   : 
   : 
   : 
   : 
407b6a18-4519-4d25-97ba-15fbcf46709c
   : 
   : 
   : 
   : 
   : 
   : 
   : 
407b6a18-4519-4d25-97ba-15fbcf46709c
   : 
   : 
   : 
   : 
This will not pass in pep8 check


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread ciudavitacos 
Javier Coscia has uploaded a new change for review.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..

Hook: ide2sata: To switch IDE disks to SATA

Add the ability to use SATA/AHCI disks while creating them as IDE.

Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Signed-off-by: Javier Coscia 
---
M configure.ac
M vdsm.spec.in
M vdsm_hooks/Makefile.am
A vdsm_hooks/ide2sata/Makefile.am
A vdsm_hooks/ide2sata/README
A vdsm_hooks/ide2sata/before_vm_start.py
6 files changed, 181 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/48450/1

diff --git a/configure.ac b/configure.ac
index e68f1b4..ec31bb8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -377,6 +377,7 @@
vdsm_hooks/hostusb/Makefile
vdsm_hooks/httpsisoboot/Makefile
vdsm_hooks/hugepages/Makefile
+   vdsm_hooks/ide2sata/Makefile
vdsm_hooks/ipv6/Makefile
vdsm_hooks/isolatedprivatevlan/Makefile
vdsm_hooks/macbind/Makefile
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 35a0d25..b516497 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -604,6 +604,14 @@
 VDSM hook used for applying IPv6 configuration through custom network
 properties
 
+%package hook-ide2sata
+Summary:Change IDE to SATA for disk/lun devices
+BuildArch:  noarch
+Requires:   qemu-kvm >= 0.14
+
+%description hook-ide2sata
+VDSM hooks which changes IDE to SATA disks.
+
 %if 0%{?with_gluster}
 %package gluster
 Summary:Gluster Plugin for VDSM
@@ -1318,6 +1326,10 @@
 %{_libexecdir}/%{vdsm_name}/hooks/before_vm_migrate_destination/50_hugepages
 %{_libexecdir}/%{vdsm_name}/hooks/after_vm_destroy/50_hugepages
 
+%files hook-ide2sata
+%defattr(-, root, root, -)
+%{_libexecdir}/%{vdsm_name}/hooks/before_vm_start/50_ide2sata
+
 %files hook-isolatedprivatevlan
 %defattr(-, root, root, -)
 %{_libexecdir}/%{vdsm_name}/hooks/before_vm_start/50_isolatedprivatevlan
diff --git a/vdsm_hooks/Makefile.am b/vdsm_hooks/Makefile.am
index dda39bc..165e850 100644
--- a/vdsm_hooks/Makefile.am
+++ b/vdsm_hooks/Makefile.am
@@ -46,6 +46,7 @@
hostusb \
httpsisoboot \
hugepages \
+   ide2sata \
isolatedprivatevlan \
macbind \
nestedvt \
diff --git a/vdsm_hooks/ide2sata/Makefile.am b/vdsm_hooks/ide2sata/Makefile.am
new file mode 100644
index 000..8cf7ab0
--- /dev/null
+++ b/vdsm_hooks/ide2sata/Makefile.am
@@ -0,0 +1,31 @@
+#
+# Copyright 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+#
+EXTRA_DIST = \
+   before_vm_start.py
+
+install-data-local:
+   $(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_start
+   $(INSTALL_SCRIPT) $(srcdir)/before_vm_start.py \
+   $(DESTDIR)$(vdsmhooksdir)/before_vm_start/50_ide2sata
+
+uninstall-local:
+   $(RM) $(DESTDIR)$(vdsmhooksdir)/before_vm_start/50_ide2sata
+
diff --git a/vdsm_hooks/ide2sata/README b/vdsm_hooks/ide2sata/README
new file mode 100644
index 000..42e08e1
--- /dev/null
+++ b/vdsm_hooks/ide2sata/README
@@ -0,0 +1,39 @@
+ide2sata vdsm hook:
+==
+This hook goes through all of the VM's disks definition and manipulate
+its XML file acccording to the input. This can be used to enable SATA 
+bus for the disks devices.
+
+Syntax:
+ide2sata=(off|on)
+
+Where:
+'on' is bus change to SATA and off leaves the bus as IDE.
+
+Example:
+ide2sata=on
+
+Installation:
+- Use the engine-config to append the proper custom property:
+$ sudo engine-config -s UserDefinedVMProperties='ide2sata=^(off|on)$'
+- Verify that the diskunmap custom property was properly added:
+$ sudo engine-config -g UserDefinedVMProperties
+
+Usage:
+In the VM configuration window, open the custom properites tab, select
+ide2sata and select 'on', changing the bus from IDE to SATA for all disks 
and LUNs.
+Only devices using IDE interface will be affected.
+
+Expected Result:
+For every DISK or LUN device, this configuration will include
+"bus=sata" + will replace the disk name to start with 's' + 
+adding the Sata controller in the 'devices' section
+
+
+...
+
+

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread apahim 
Amador Pahim has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 1: Code-Review-1

(9 comments)

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

Line 9: Add
Why is this useful? Could you please explain why is this important?


https://gerrit.ovirt.org/#/c/48450/1/vdsm_hooks/ide2sata/README
File vdsm_hooks/ide2sata/README:

Line 4:  
please remove the trailing whitespace


Line 29:  
please remove the trailing whitespace


https://gerrit.ovirt.org/#/c/48450/1/vdsm_hooks/ide2sata/before_vm_start.py
File vdsm_hooks/ide2sata/before_vm_start.py:

Line 38: import hooking
Line 39: 
Line 40: 
Line 41: def createControllerElement(domxml):
Line 42: 
not needed blank line
Line 43: devices = domxml.getElementsByTagName('devices')[0]
Line 44: controller = domxml.createElement('controller')
Line 45: controller.setAttribute('index', '0')
Line 46: controller.setAttribute('type', 'sata')


Line 45: controller.setAttribute('index', '0')
'index' is optional. libvirt will fill that for you if you don't set it. Not 
sure about the consequences in hard code it to '0'. I'd drop this line.


Line 49: return controller
Line 50: 
Line 51: 
Line 52: def change2Sata(domxml):
Line 53: 
not needed blank line
Line 54: for disk in domxml.getElementsByTagName('disk'):
Line 55: device = disk.getAttribute('device')
Line 56: target = disk.getElementsByTagName('target')[0]
Line 57: bus = target.getAttribute('bus')


Line 62: s
Maybe target.setAttribute('dev', 'sd%s' % dev[-1]) reflects better what you're 
doing here, but this is just a personal preference.


Line 72: hooking.write_domxml(domxml)
Line 73: 
Line 74: 
Line 75: def test():
Line 76: 
not needed blank line
Line 77: with open('vm.xml', 'r') as vmxml:
Line 78: xmldom = minidom.parseString(vmxml.read())
Line 79: 
Line 80: print("\nDisk device definition BEFORE hook execution:\n%s"


Line 73: 
Line 74: 
Line 75: def test():
Line 76: 
Line 77: with open('vm.xml', 'r') as vmxml:
Where is the vm.xml file? You should either include it in your commit or, 
better, instead of loading an external file, just create a variable with the 
xml to test your hook.
Line 78: xmldom = minidom.parseString(vmxml.read())
Line 79: 
Line 80: print("\nDisk device definition BEFORE hook execution:\n%s"
Line 81:   % xmldom.toxml(encoding='UTF-8'))


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread apahim 
Amador Pahim has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread automation 
automat...@ovirt.org has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


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-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Hook: ide2sata: To switch IDE disks to SATA

2015-11-11 Thread ciudavitacos 
Javier Coscia has posted comments on this change.

Change subject: Hook: ide2sata: To switch IDE disks to SATA
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0088dae191cf4560a00ee62023a54f5ab746a3c9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Javier Coscia 
Gerrit-Reviewer: Amador Pahim 
Gerrit-Reviewer: Javier Coscia 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches