[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15886880#comment-15886880
 ] 

ASF subversion and git services commented on CLOUDSTACK-8826:
-

Commit d9bd01266ffb23f591283c058beff6e65d1cc4aa in cloudstack's branch 
refs/heads/4.9 from [~rajanik]
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=d9bd012 ]

Merge pull request #1829 from syed/hvm-volume-attach-restart-fix

CLOUDSTACK-9363: Fix HVM VM restart bug in XenServerHere is the longer 
description of the problem:

By default XenServer limits HVM guests to only 4 disks. Two of those are 
reserved for the ROOT disk (deviceId=0) and CD ROM (device ID=3) which means 
that we can only attach 2 data disks. This limit however is removed when 
Xentools is installed on the guest. The information that a guest has Xentools 
installed and can handle more than 4 disks is stored in the VM metadata on 
XenServer. When a VM is shut down, Cloudstack removes the VM and all the 
metadata associated with the VM from XenServer. Now, when you start the VM 
again, even if it has Xentools installed, it will default to only 4 attachable 
disks.

Now this problem manifests itself when you have a HVM VM and you stop and start 
it with more than 2 data disks attached. The VM fails to start and the only way 
to start the VM is to detach the extra disks and then reattach them after the 
VM start.

In this fix, I am removing the check which is done before creating a `VBD` 
which enforces this limit. This will not affect current workflow and will fix 
the HVM issue.

@koushik-das this is related to the "autodetect" feature that you introduced a 
while back (https://issues.apache.org/jira/browse/CLOUDSTACK-8826). I would 
love your review on this fix.

* pr/1829:
  Fix HVM VM restart bug in XenServer

Signed-off-by: Rajani Karuturi 


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15886852#comment-15886852
 ] 

ASF subversion and git services commented on CLOUDSTACK-8826:
-

Commit d9bd01266ffb23f591283c058beff6e65d1cc4aa in cloudstack's branch 
refs/heads/master from [~rajanik]
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=d9bd012 ]

Merge pull request #1829 from syed/hvm-volume-attach-restart-fix

CLOUDSTACK-9363: Fix HVM VM restart bug in XenServerHere is the longer 
description of the problem:

By default XenServer limits HVM guests to only 4 disks. Two of those are 
reserved for the ROOT disk (deviceId=0) and CD ROM (device ID=3) which means 
that we can only attach 2 data disks. This limit however is removed when 
Xentools is installed on the guest. The information that a guest has Xentools 
installed and can handle more than 4 disks is stored in the VM metadata on 
XenServer. When a VM is shut down, Cloudstack removes the VM and all the 
metadata associated with the VM from XenServer. Now, when you start the VM 
again, even if it has Xentools installed, it will default to only 4 attachable 
disks.

Now this problem manifests itself when you have a HVM VM and you stop and start 
it with more than 2 data disks attached. The VM fails to start and the only way 
to start the VM is to detach the extra disks and then reattach them after the 
VM start.

In this fix, I am removing the check which is done before creating a `VBD` 
which enforces this limit. This will not affect current workflow and will fix 
the HVM issue.

@koushik-das this is related to the "autodetect" feature that you introduced a 
while back (https://issues.apache.org/jira/browse/CLOUDSTACK-8826). I would 
love your review on this fix.

* pr/1829:
  Fix HVM VM restart bug in XenServer

Signed-off-by: Rajani Karuturi 


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15251316#comment-15251316
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-212753295
  
@pdion891 This wasn't tested with HVM VMs.




> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15250556#comment-15250556
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user simongodard commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-212573040
  
I confirm what @pdion891 described in the previous comment. 
http://markmail.org/thread/4nmyra6aofxtu3o2


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[Simon Godard (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15250458#comment-15250458
 ] 

Simon Godard commented on CLOUDSTACK-8826:
--

This bug fix looks like it broke a simple HVM VM start when more than 2 volumes 
are attached. Using 'autodetect' as the device Id is also rejected by XenServer 
when using HVM.

I tested with a PV VM and it seems to work fine.

> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15249874#comment-15249874
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user pdion891 commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-212423092
  
@koushik-das  does this fix got tested with HVM vm having more than 4 VDI?  
because we are experiencing issue where an HVM vm on XenServer 6.5 having 4 vdi 
(1 root + 3 datadisk) fail to start after a shutdown and it seams to be related 
to this part of code.  This is currently working on 4.4.x version of ACS.



> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902345#comment-14902345
 ] 

ASF subversion and git services commented on CLOUDSTACK-8826:
-

Commit 86325cbc95abfff038e7929ac8c8d3b3b3bd0267 in cloudstack's branch 
refs/heads/master from [~koushikd]
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=86325cb ]

Merge pull request #792 from koushik-das/CLOUDSTACK-8826

CLOUDSTACK-8826: XenServer - Use device id passed as part of attach volume API 
properly

If device id passed as part of API and available then use it otherwise fallback 
on XS to automatically assign one.
For ISO device id used is 3 and it is processed before any other entry to avoid 
conflict.

Signed-off-by: Koushik Das 


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902343#comment-14902343
 ] 

ASF subversion and git services commented on CLOUDSTACK-8826:
-

Commit f5b9a96d24cb0c64c57eb8f1802813ce0f8b3671 in cloudstack's branch 
refs/heads/master from [~koushikd]
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=f5b9a96 ]

CLOUDSTACK-8826: XenServer - Use device id passed as part of attach volume API 
properly
If device id passed as part of API and available then use it otherwise fallback 
on XS to automatically assign one.
For ISO device id used is 3 and it is processed before any other entry to avoid 
conflict.


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902347#comment-14902347
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/792


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902344#comment-14902344
 ] 

ASF subversion and git services commented on CLOUDSTACK-8826:
-

Commit 86325cbc95abfff038e7929ac8c8d3b3b3bd0267 in cloudstack's branch 
refs/heads/master from [~koushikd]
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=86325cb ]

Merge pull request #792 from koushik-das/CLOUDSTACK-8826

CLOUDSTACK-8826: XenServer - Use device id passed as part of attach volume API 
properly

If device id passed as part of API and available then use it otherwise fallback 
on XS to automatically assign one.
For ISO device id used is 3 and it is processed before any other entry to avoid 
conflict.

Signed-off-by: Koushik Das 


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14902333#comment-14902333
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-142236155
  
Merging as 2 LGTMs.


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14900553#comment-14900553
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/792#discussion_r39962919
  
--- Diff: 
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
 ---
@@ -3407,6 +3390,18 @@ public void handleSrAndVdiDetach(final String iqn, 
final Connection conn) throws
 removeSR(conn, sr);
 }
 
+protected void destroyUnattachedVBD(Connection conn, VM vm) {
+try {
+for (VBD vbd : vm.getVBDs(conn)) {
+if (Types.VbdType.DISK.equals(vbd.getType(conn)) && 
!vbd.getCurrentlyAttached(conn)) {
+vbd.destroy(conn);
+}
+}
+} catch (final Exception e) {
+s_logger.debug("destroyUnattachedVBD failed due to " + 
e.toString());
--- End diff --

@DaanHoogland Now logging the full exception


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14805064#comment-14805064
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/792#discussion_r39827592
  
--- Diff: 
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
 ---
@@ -3407,6 +3390,18 @@ public void handleSrAndVdiDetach(final String iqn, 
final Connection conn) throws
 removeSR(conn, sr);
 }
 
+protected void destroyUnattachedVBD(Connection conn, VM vm) {
+try {
+for (VBD vbd : vm.getVBDs(conn)) {
+if (Types.VbdType.DISK.equals(vbd.getType(conn)) && 
!vbd.getCurrentlyAttached(conn)) {
+vbd.destroy(conn);
+}
+}
+} catch (final Exception e) {
+s_logger.debug("destroyUnattachedVBD failed due to " + 
e.toString());
--- End diff --

@DaanHoogland Are you suggesting to log the full exception? Currently only 
the message is getting logged. The destroyUnattachedVBD is a best-effort call 
to cleanup VBDs.


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14805316#comment-14805316
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/792#discussion_r39839158
  
--- Diff: 
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
 ---
@@ -3407,6 +3390,18 @@ public void handleSrAndVdiDetach(final String iqn, 
final Connection conn) throws
 removeSR(conn, sr);
 }
 
+protected void destroyUnattachedVBD(Connection conn, VM vm) {
+try {
+for (VBD vbd : vm.getVBDs(conn)) {
+if (Types.VbdType.DISK.equals(vbd.getType(conn)) && 
!vbd.getCurrentlyAttached(conn)) {
+vbd.destroy(conn);
+}
+}
+} catch (final Exception e) {
+s_logger.debug("destroyUnattachedVBD failed due to " + 
e.toString());
--- End diff --

@koushik-das I would catch only specific exceptions and give them specific 
messages and report on results in a finally block. the present state is that 
any numberformat, memory, stackoverflow or devidebyzero will be caught here, 
where it is not relevant. My initial remark was to make sure we know what kind 
of exception is caught at least. This is not always clear from the message 
which may be null.


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14745036#comment-14745036
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-140317652
  
Can anyone review this?


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14745280#comment-14745280
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-140359521
  
one remark, lgtm


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14745279#comment-14745279
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/792#discussion_r39499607
  
--- Diff: 
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
 ---
@@ -3407,6 +3390,18 @@ public void handleSrAndVdiDetach(final String iqn, 
final Connection conn) throws
 removeSR(conn, sr);
 }
 
+protected void destroyUnattachedVBD(Connection conn, VM vm) {
+try {
+for (VBD vbd : vm.getVBDs(conn)) {
+if (Types.VbdType.DISK.equals(vbd.getType(conn)) && 
!vbd.getCurrentlyAttached(conn)) {
+vbd.destroy(conn);
+}
+}
+} catch (final Exception e) {
+s_logger.debug("destroyUnattachedVBD failed due to " + 
e.toString());
--- End diff --

maybe add the exception or/and catch more specific exceptions


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14740637#comment-14740637
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-139524545
  
code LGTM


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14736416#comment-14736416
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/792#issuecomment-138819369
  
I had to undo changes from PR https://github.com/apache/cloudstack/pull/773 
to run the tests.


> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a span of 24 hrs).
> The failures happened as the device id for attaching volume wasn't available 
> in HV. Some detached volume didn't got cleaned up properly and so the device 
> id wasn't released.
> The fix would be clean up stale volumes before attaching new ones so the 
> device slots are released. Also using the device id should be best effort and 
> if that particular id is not available in XS, it should fallback on using an 
> id that is available and automatically assigned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (CLOUDSTACK-8826) XenServer - Use device id passed as part of attach volume API properly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-8826?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14736371#comment-14736371
 ] 

ASF GitHub Bot commented on CLOUDSTACK-8826:


GitHub user koushik-das opened a pull request:

https://github.com/apache/cloudstack/pull/792

CLOUDSTACK-8826: XenServer - Use device id passed as part of attach v…

…olume API properly

If device id passed as part of API and available then use it otherwise 
fallback on XS to automatically assign one.
For ISO device id used is 3 and it is processed before any other entry to 
avoid conflict.

This is only specific to XS, ran the following tests:

BVT: test/integration/smoke/test_volumes.py

Test Volume creation for all Disk Offerings (incl. custom) ... === 
TestName: test_01_create_volume | Status : SUCCESS ===
ok
Attach a created Volume to a Running VM ... === TestName: 
test_02_attach_volume | Status : SUCCESS ===
ok
Download a Volume attached to a VM ... === TestName: 
test_03_download_attached_volume | Status : SUCCESS ===
ok
Delete a Volume attached to a VM ... === TestName: 
test_04_delete_attached_volume | Status : SUCCESS ===
ok
Detach a Volume attached to a VM ... === TestName: test_05_detach_volume | 
Status : SUCCESS ===
ok
Download a Volume unattached to an VM ... === TestName: 
test_06_download_detached_volume | Status : SUCCESS ===
ok
Test resize (negative) non-existent volume ... === TestName: 
test_07_resize_fail | Status : SUCCESS ===
ok
Test resize a volume ... === TestName: test_08_resize_volume | Status : 
SUCCESS ===
ok
Delete a Volume unattached to an VM ... === TestName: 
test_09_delete_detached_volume | Status : SUCCESS ===
ok

--
Ran 9 tests in 1116.972s

OK



Regression: test/integration/component/test_volumes.py

Test Volume attach/detach to VM (5 data volumes) ... === TestName: 
test_01_volume_attach_detach | Status : SUCCESS ===
ok
Test Attach volumes (max capacity) ... === TestName: test_01_volume_attach 
| Status : SUCCESS ===
ok
Test attach volumes (more than max) to an instance ... === TestName: 
test_02_volume_attach_max | Status : SUCCESS ===
ok
Test Volumes and ISO attach ... === TestName: test_01_volume_iso_attach | 
Status : SUCCESS ===
ok
Test custom disk sizes beyond range ... === TestName: 
test_deployVmWithCustomDisk | Status : SUCCESS ===
ok
@Desc:Volume is not retaining same uuid when migrating from one ... SKIP: 
No suitable storage pools found for volume migration.
Skipping
Attach a created Volume to a Running VM ... === TestName: 
test_01_attach_volume | Status : SUCCESS ===
ok
Detach a Volume attached to a VM ... === TestName: test_02_detach_volume | 
Status : SUCCESS ===
ok
Delete a Volume unattached to an VM ... === TestName: 
test_03_delete_detached_volume | Status : SUCCESS ===
ok
Create a volume under a non-root domain as non-root-domain user ... === 
TestName: test_create_volume_under_domain | Status : SUCCESS ===
ok

--
Ran 10 tests in 1750.686s

OK (SKIP=1)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/koushik-das/cloudstack CLOUDSTACK-8826

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/792.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #792


commit 1dd27bd449a8e3d70b011e66af431e866c7627c2
Author: Koushik Das 
Date:   2015-09-09T07:30:17Z

CLOUDSTACK-8826: XenServer - Use device id passed as part of attach volume 
API properly
If device id passed as part of API and available then use it otherwise 
fallback on XS to automatically assign one.
For ISO device id used is 3 and it is processed before any other entry to 
avoid conflict.




> XenServer - Use device id passed as part of attach volume API properly
> --
>
> Key: CLOUDSTACK-8826
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8826
> Project: CloudStack
>  Issue Type: Bug
>  Security Level: Public(Anyone can view this level - this is the 
> default.) 
>  Components: XenServer
>Affects Versions: 4.6.0
>Reporter: Koushik Das
>Assignee: Koushik Das
> Fix For: 4.6.0
>
>
> Random failures were seen in XS attach/detach volume test scenarios (many 
> attach/detach were performed on the same VM over a