Smart State Analysis for Hyper-V Disks #2428

jerryk55 · 2015-03-30T13:42:11Z

See the summary - "What Works and What Doesn't" at the bottom.

Add a new MiqDisk Disk Module to handle the VHDX virtual disk
format supported by the new version of Hyper-V and SCVMM.

Get Hard disk information for VMs managed
by SCVMM from the SCVMM and Hyper-V host via powershell
functions.

Read the hard disk from the Hyper-V host.

Gets the vhd, host, vhd type from the Hyper-V Host.
Opens and reads the vhd on the Hyper-V host.

This supports both VHDX files via the new VHDXDisk disk module
and the older VHD files via the existing disk modules. Support
for communicating to Hyper-V via miq_hyperv_disk was added
to the VHD modules.

The Microsoft Provider has been extended to allow SSA scans to be
run in addition to the MiqDisk, MiqVm, and Scvmm gems/pending code.

Enhancements still required:

Currently Snapshots are created using VMNAME__EVM_SNAPSHOT
as the snapshot name. An enhancement will add the date/time into the
snapshot name.
Caching has been implemented in the miq_hyperv_disk code which
allows the VHD and VHDX disk modules to communicate to Hyper-V
to read the files. We need to continue to look at this caching to speed this
up as much as possible.
Virtual disk files residing on network-mapped drives on the Hyper-V server, seen
as starting with \ rather than a drive letter when the inventory is taken from the server,
show up in the database with a NULL datastore ID. These files currently fail SSA
because of this NULL ID. Hyper-V Inventory Doesn't Add Storage for VMs with VHDs on Network Drive #5383
describes this issue. In addition to handling the parsing the code in MiqScvmmVm::getCfg
needs to handle this situation. it currently does not.
Certain VMs seem to timeout after looping doing I/O for an inordinate amount of time while
local copies of the same VM VHD files run extremely quickly. Further investigation is needed;
this may be related to the caching item in Use anand version #2 above.

What Works And What Doesn't -

SSA works on local copies of VHD and VHDX disks and their checkpoints.
SSA works on remote copies of VHD and VHDX disks and their checkpoints residing on Hyper-V servers, except some larger disks may time out based on network bandwidth and possibly one other unresolved issue.
Hyper-V VMs with virtual disks residing on network-mapped drives starting with "" will not currently work.

@roliveri @Fryguy @chessbyte Please Review

By the way - an update to this old comment - this all works at this point.

Fryguy · 2015-03-30T13:56:24Z

lib/Scvmm/miq_hyperv_disk.rb

@@ -0,0 +1,46 @@
+


Can you remove all these bonus blank lines at the start of every file?

Absatootly. :-)

which of the requires below is coming from util? if none, then we do not need this push.

miq-bot · 2015-06-11T15:32:47Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

miq-bot · 2015-06-26T02:27:22Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

chessbyte · 2015-06-26T12:17:50Z

lib/Scvmm/miq_scvmm_vm_ssa_info.rb

@@ -0,0 +1,69 @@
+$LOAD_PATH.push("#{File.dirname(__FILE__)}/../util")


which of the requires below is coming from util? if none, then we do not need this push.

jrafanie · 2015-12-23T16:22:46Z

gems/pending/disk/modules/VhdxDisk.rb

+    @vhdx_file
+  end
+
+  def d_read(pos, len)


It would be nice to have targeted tests for complicated methods like this.

I wonder if we could run VCR like tests of this method by running a full end to end test of some scvmm disks, recording the inputs and outputs as test cases. It might help better understand the conditionals in this method.

I would suppose that is a possibility. I'm not sure if we have anything like that for any of the disk formats we already support, by the way. Not that two wrongs make a right.

@jrafanie @jerryk55 - I hope to enable testing things like this in a general way very soon - once we're able to deploy the camcorder FS. Then, we should be able to write tests for all disk file formats.

And in the meantime I would think that the lack of the general test would not be a blocker to merging.

jerryk55 · 2015-12-23T16:24:11Z

@Fryguy yes I'll look into that in VhdxDisk.rb, VhdxDiskProbe.rb, MSVSDiskProbe.rb, miq_hyperv_disk.rb, miq_scvmm_vm_ssa_info.rb, as well as host.rb and infra_manager.rb in the microsoft provider. I do see it still out there in a few spots besides this - are these places that were left over, or places it was added back by other PRs? For instance under app/models host.rb, filesystem.rb, miq_server/server_smart_proxy.rb.

chessbyte · 2015-12-23T16:31:51Z

@jerryk55 In general, this PR has several challenges.

Too much new code vs little/no new specs.
Too many unrelated changes or changes that should be split out into separate PRs. (eg new VHDx disk format).
Too many changes/additions for the reviewer to absorb in one PR.
Seems to add more duplication to our code base (ie lots of copy/paste without refactoring).

Lets discuss further in 2016.

Thanks,
Oleg

jerryk55 · 2015-12-23T16:45:10Z

@chessbyte okie dokie. Yes I've always thought that VhdxDisk could be its own PR, and the HyperVDisk methods could also stand on their own. Thanks.

jerryk55 · 2016-01-04T22:04:46Z

@roliveri @chessbyte @Fryguy @jrafanie This PR has been amended to pull the VhdxDisk module out as discussed. I will try to reword the log message of one or two of the commit messages but otherwise it is ready for any further review and merging when ready.

chessbyte · 2016-01-04T22:43:52Z

app/models/manageiq/providers/microsoft/infra_manager.rb

@@ -1,4 +1,4 @@
-$LOAD_PATH << File.join(GEMS_PENDING_ROOT, "Scvmm")
+# encoding: US-ASCII


why is this line being added?

Because I was under the mistaken impression that was standard practice in our code, but now I see that not to be the case.

jerryk55 · 2016-01-05T14:19:45Z

@chessbyte Removed the above extraneous encoding comments and re-pushed.

miq-bot · 2016-01-05T18:27:40Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

Get Hard disk information for VMs managed by SCVMM from the SCVMM via powershell functions. Read the hard disk from the Hyper-V host. Gets the vhd, host, vhd type from the SCVMM. Opens and reads the vhd on the Hyper-V host. This is part of the functionality to perform Hyper-V/SCVMM VM fleecing This commit was amended with peer review comments.

Get Hard disk information for VMs managed by SCVMM from the SCVMM via powershell functions. Read the hard disk from the Hyper-V host. Gets the vhd, host, vhd type from the SCVMM. Opens and reads the vhd on the Hyper-V host. This is part of the functionality to perform Hyper-V/SCVMM VM fleecing.

Add a new MiqDisk Disk Module to handle the VHDX virtual disk format supported by the new version of Hyper-V and SCVMM. Supports remote and local copies of vhdx and avhdx disk files although remote disks need additional testing.

Allow VHD files to be accessed remotely via miq_hyperv_disk to enable Smart State Analysis of VMs managed by SCVMM on Hyper-V servers. Note that pre-existing checksum handling for the header and footer in this code is non-functional and has been turned off via the "skip_check" argument.

Add a new MiqDisk Disk Module to handle the VHDX virtual disk format supported by the new version of Hyper-V and SCVMM. Supports remote and local copies of vhdx and avhdx disk files although remote disks need additional testing.

Add a new MiqDisk Disk Module to handle the VHDX virtual disk format supported by the new version of Hyper-V and SCVMM. Also supports older VHD virtual disk format. Supports remote and local copies of files.

Multiple RuboCop Warning cleanup. Of note is that MiqVM.rb has been modified to clean up all spacing issues. This was previously handled but is being accomodated again due to rebasing issues with this PR eliminating the previous spacing fix.

1) Bug in server_smart_proxy.rb caused VMware SSA to fail with "VM not found" 2) Reduce blocks read in miq_hyperv_disk.rb from 16 to 4. 3) Minor Rubocop cleanup.

1) Increase miq_hyperv_disk buffer cache from 200 to 300 entries. 2) Add a per-provider timeout adjustment to the VmScan object. It defaults to 1 but is set to 4 for Microsoft so that very slow scans may succeed.

Fix timeout adjustment so that it does not occur when rspec is running. Remove test which expected failure when running SSA on microsoft.

@roliveri

1. Fix test failures caused by timeout mods for Microsoft provider. 2. Mods for review comments by @roliveri to make provider->MiqVM interaction consistent with other providers - specifically change so that connect_to_ems connects and passed the handle to the connection in the ost structure.

@roliveri

1. Fix test failures caused by timeout mods for Microsoft provider. 2. Mods for review comments by @roliveri to i) make provider->MiqVM interaction consistent with other providers - specifically change so that connect_to_ems connects and passed the handle to the connection in the ost structure. ii) fix calculation of blocks in a disk in miq_hyperv_disk 3) Add debugging miq_hyperv_disk cache statistics code.

miq-bot · 2016-01-05T18:57:47Z

<github_pr_commenter_batch />Some comments on commits jerryk55/manageiq@3fabb83~...d17004d

Gemfile changes detected. /cc @JPrause @simaishi

miq-bot · 2016-01-05T18:59:05Z

Checked commits jerryk55/manageiq@3fabb83~...d17004d with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0
28 files checked, 26 offenses detected

app/models/manageiq/providers/microsoft/infra_manager/host.rb

🔹 - Line 10, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for verify_credentials_windows is too high. [15.56/15]

app/models/manageiq/providers/microsoft/infra_manager/vm_or_template_shared/scanning.rb

🔴 Warn - Line 77, Col 7 - Lint/RescueException - Avoid rescuing the Exception class. Perhaps you meant to rescue StandardError?
🔹 - Line 2, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for perform_metadata_scan is too high. [17.38/15]
🔹 - Line 37, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for connect_to_ems is too high. [57.91/15]
🔹 - Line 37, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for connect_to_ems is too high. [8/6]
🔹 - Line 37, Col 3 - Metrics/MethodLength - Method has too many lines. [39/25]
🔹 - Line 37, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for connect_to_ems is too high. [8/7]

gems/pending/MiqVm/MiqVm.rb

🔶 - Line 54, Col 7 - Style/VariableName - Use snake_case for variable names.
🔶 - Line 95, Col 11 - Style/VariableName - Use snake_case for variable names.

gems/pending/MiqVm/miq_scvmm_vm.rb

🔶 - Line 5, Col 7 - Style/MethodName - Use snake_case for method names.
🔹 - Line 5, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for getCfg is too high. [15.68/15]
🔹 - Line 25, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for init_disk_info is too high. [29.02/15]

gems/pending/Scvmm/miq_hyperv_disk.rb

🔹 - Line 11, Col 1 - Metrics/ClassLength - Class has too many lines. [135/100]
🔹 - Line 18, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for initialize is too high. [15.23/15]
🔹 - Line 33, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for open is too high. [15.68/15]
🔹 - Line 57, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for hit_or_miss is too high. [16/15]
🔹 - Line 67, Col 121 - Metrics/LineLength - Line is too long. [145/120]
🔹 - Line 84, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for read is too high. [15.39/15]
🔹 - Line 97, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for bread_cached is too high. [28.53/15]
🔹 - Line 118, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for bread is too high. [26.02/15]

gems/pending/Scvmm/miq_scvmm_parse_powershell.rb

🔹 - Line 24, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for parse_attribute_values is too high. [24.35/15]
🔹 - Line 24, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for parse_attribute_values is too high. [7/6]
🔹 - Line 24, Col 3 - Metrics/MethodLength - Method has too many lines. [26/25]

gems/pending/Scvmm/miq_scvmm_vm_ssa_info.rb

🔹 - Line 33, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for vm_all_harddisks is too high. [15.36/15]

gems/pending/disk/modules/MSCommon.rb

🔴 Warn - Line 250, Col 43 - Lint/UnusedBlockArgument - Unused block argument - block_num. You can omit the argument if you don't care about it.

gems/pending/util/miq_winrm.rb

🔹 - Line 32, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for validate_options is too high. [16.03/15]

Smart State Analysis for Hyper-V Disks

chessbyte added enhancement core/smart state providers/infrastructure labels Mar 30, 2015

Fryguy reviewed Mar 30, 2015
View reviewed changes

jerryk55 force-pushed the scvmm_harddrive_parsing branch from 3363723 to fdaa683 Compare April 1, 2015 23:09

Fryguy added the wip label Apr 20, 2015

Fryguy added providers/scvmm and removed providers/infrastructure labels May 14, 2015

jerryk55 force-pushed the scvmm_harddrive_parsing branch from fdaa683 to 443795a Compare May 29, 2015 21:02

jerryk55 changed the title ~~[WIP] Read Hyper-V VM Harddrive~~ [WIP] Read Hyper-V VM Harddrive And Parse via VhdxDisk Module May 29, 2015

jerryk55 force-pushed the scvmm_harddrive_parsing branch from 443795a to 9c2d4f7 Compare June 16, 2015 22:35

miq-bot added the dependencies label Jun 16, 2015

jerryk55 force-pushed the scvmm_harddrive_parsing branch from 9c2d4f7 to 642d1ac Compare June 17, 2015 18:56

chessbyte reviewed Jun 26, 2015
View reviewed changes

jerryk55 force-pushed the scvmm_harddrive_parsing branch 2 times, most recently from df6b6db to 967962d Compare June 29, 2015 16:24

jrafanie reviewed Dec 23, 2015
View reviewed changes

jerryk55 force-pushed the scvmm_harddrive_parsing branch from e9d93f6 to ef39884 Compare January 4, 2016 21:45

chessbyte reviewed Jan 4, 2016
View reviewed changes

jerryk55 force-pushed the scvmm_harddrive_parsing branch from ef39884 to 62a0df0 Compare January 5, 2016 14:18

jerryk55 and others added 13 commits January 5, 2016 13:52

Add Support for Hyper-V Fleecing

eb7314a

Add a new MiqDisk Disk Module to handle the VHDX virtual disk format supported by the new version of Hyper-V and SCVMM. Supports remote and local copies of vhdx and avhdx disk files although remote disks need additional testing.

Add Support for Hyper-V Fleecing

9480ee8

Add a new MiqDisk Disk Module to handle the VHDX virtual disk format supported by the new version of Hyper-V and SCVMM. Also supports older VHD virtual disk format. Supports remote and local copies of files.

RuboCop comment and bug fix for HyperV SSA Functionality

65a4cad

1) Bug in server_smart_proxy.rb caused VMware SSA to fail with "VM not found" 2) Reduce blocks read in miq_hyperv_disk.rb from 16 to 4. 3) Minor Rubocop cleanup.

Timing Mods for SSA of Hyper-V VMs

0a88ce2

1) Increase miq_hyperv_disk buffer cache from 200 to 300 entries. 2) Add a per-provider timeout adjustment to the VmScan object. It defaults to 1 but is set to 4 for Microsoft so that very slow scans may succeed.

Fix Test Failures Caused by Timing Mods and New Microsoft Functionality

fb0a792

Fix timeout adjustment so that it does not occur when rspec is running. Remove test which expected failure when running SSA on microsoft.

jerryk55 force-pushed the scvmm_harddrive_parsing branch from 62a0df0 to d17004d Compare January 5, 2016 18:53

chessbyte added a commit that referenced this pull request Jan 5, 2016

Merge pull request #2428 from jerryk55/scvmm_harddrive_parsing

5ae91e2

Smart State Analysis for Hyper-V Disks

chessbyte merged commit 5ae91e2 into ManageIQ:master Jan 5, 2016

chessbyte added this to the Sprint 35 Ending Jan 25, 2016 milestone Jan 5, 2016

Fryguy added the providers/infrastructure label Jan 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smart State Analysis for Hyper-V Disks #2428

Smart State Analysis for Hyper-V Disks #2428

jerryk55 commented Mar 30, 2015

Fryguy Mar 30, 2015

jerryk55 Mar 30, 2015

chessbyte Jun 26, 2015

jerryk55 Jun 26, 2015

miq-bot commented Jun 11, 2015

miq-bot commented Jun 26, 2015

chessbyte Jun 26, 2015

jerryk55 Jun 26, 2015

jrafanie Dec 23, 2015

jrafanie Dec 23, 2015

jerryk55 Dec 23, 2015

roliveri Dec 27, 2015

jerryk55 Jan 4, 2016

jerryk55 commented Dec 23, 2015

chessbyte commented Dec 23, 2015

jerryk55 commented Dec 23, 2015

jerryk55 commented Jan 4, 2016

chessbyte Jan 4, 2016

jerryk55 Jan 5, 2016

jerryk55 commented Jan 5, 2016

miq-bot commented Jan 5, 2016

miq-bot commented Jan 5, 2016

miq-bot commented Jan 5, 2016

		@@ -0,0 +1,69 @@
		$LOAD_PATH.push("#{File.dirname(__FILE__)}/../util")

		@@ -1,4 +1,4 @@
		$LOAD_PATH << File.join(GEMS_PENDING_ROOT, "Scvmm")
		# encoding: US-ASCII

Smart State Analysis for Hyper-V Disks #2428

Smart State Analysis for Hyper-V Disks #2428

Conversation

jerryk55 commented Mar 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Jun 11, 2015

miq-bot commented Jun 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryk55 commented Dec 23, 2015

chessbyte commented Dec 23, 2015

jerryk55 commented Dec 23, 2015

jerryk55 commented Jan 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryk55 commented Jan 5, 2016

miq-bot commented Jan 5, 2016

miq-bot commented Jan 5, 2016

miq-bot commented Jan 5, 2016