New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xDiskAccessPath - UniqueId format different between server 2012 and 2016 #104

Closed
Zuldan opened this Issue Jun 22, 2017 · 16 comments

Comments

Projects
None yet
2 participants
@Zuldan

Zuldan commented Jun 22, 2017

@PlagueHO I hope this one doesn't cause too much work. We've just deployed out first 2016 server in the lab and noticed that UniqueId has a different format between operating systems. I feel terrible because I was the one that initially suggested using UniqueId but it looks like Guid should have been used instead. I had no idea Microsoft changed the formatting between OS versions. Damn!

Have you seen this as well in your 2016 environment? The problem I face now is our CMDB is set to take in a Guid, 32 to 36 characters.

[LABSERVER2012]: PS C:\Users\labuser01\Documents> Get-Disk | Select-Object -Last 1 | Select-Object -Property Guid,UniqueId

Guid                                   UniqueId
----                                   --------
{3dec02ec-708e-45df-b6cc-2e644d57f035} 6000C292A946D3A9C79FDC453A6293FF
[LABSERVER2016]: PS C:\Users\labuser01\Documents> Get-Disk | Select-Object -Last 1 | Select-Object -Property Guid,UniqueId

Guid                                   UniqueId                                                                  
----                                   --------                                                                  
{f84ea0fe-3a78-4462-a1ed-9820d7965a51} SCSI\DISK&VEN_VMWARE&PROD_VIRTUAL_DISK\5&1F2A051C&0&000300:LABSERVER2016
@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jun 22, 2017

I guess one solution would be to add a 3rd DiskIdType called Guid.

Zuldan commented Jun 22, 2017

I guess one solution would be to add a 3rd DiskIdType called Guid.

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jun 23, 2017

Collaborator

Hi @Zuldan - that is actually OK 😁 I had suspected we might need other identifier types down the track, so I did write the code with that in mind. So I'll work on it (once I've got my other PR through) and get a solution! Always happy to have improvements to make!

Collaborator

PlagueHO commented Jun 23, 2017

Hi @Zuldan - that is actually OK 😁 I had suspected we might need other identifier types down the track, so I did write the code with that in mind. So I'll work on it (once I've got my other PR through) and get a solution! Always happy to have improvements to make!

@PlagueHO PlagueHO self-assigned this Jun 23, 2017

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jun 23, 2017

Thank you very much @PlagueHO

Zuldan commented Jun 23, 2017

Thank you very much @PlagueHO

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jun 30, 2017

Collaborator

Hi @Zuldan - I'll get onto this one this weekend and see if I can get it included in the next release of xStorage.

Collaborator

PlagueHO commented Jun 30, 2017

Hi @Zuldan - I'll get onto this one this weekend and see if I can get it included in the next release of xStorage.

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jun 30, 2017

@PlagueHO thank you very much. Looking forward to it.

Zuldan commented Jun 30, 2017

@PlagueHO thank you very much. Looking forward to it.

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jul 1, 2017

Collaborator

Hi @Zuldan - I've run into a bit of a big problem with this change:

The GUID for a disk only exists once it has been initialized with a GPT partition table - which of course can't be done because the disk can't be selected/found. And also, you'd not know the GUID for the disk to load into the DSC Config.

I could still perform this change, but this would only ever work for disks that were already initialized with a GPT partition table and you'd pulled the GUID out of it. Unfortunately I nearly completed the change before figuring this problem out so I'd rather not waste the effort. 😢

Collaborator

PlagueHO commented Jul 1, 2017

Hi @Zuldan - I've run into a bit of a big problem with this change:

The GUID for a disk only exists once it has been initialized with a GPT partition table - which of course can't be done because the disk can't be selected/found. And also, you'd not know the GUID for the disk to load into the DSC Config.

I could still perform this change, but this would only ever work for disks that were already initialized with a GPT partition table and you'd pulled the GUID out of it. Unfortunately I nearly completed the change before figuring this problem out so I'd rather not waste the effort. 😢

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jul 1, 2017

Ok now I feel really bad but if we had gone with GUID originally we would have run into the same problem. Surely Microsoft has a way to identify a disks through their entire life cycle. I'll do some digging around.

Adding GUID support is not a complete waste. I could modify our provisioning system to automatically initialize the disks before DSC runs. I'm sure others could do the same.

Zuldan commented Jul 1, 2017

Ok now I feel really bad but if we had gone with GUID originally we would have run into the same problem. Surely Microsoft has a way to identify a disks through their entire life cycle. I'll do some digging around.

Adding GUID support is not a complete waste. I could modify our provisioning system to automatically initialize the disks before DSC runs. I'm sure others could do the same.

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jul 1, 2017

Ok maybe a sign of hope.

UniqueId in Get-PhysicalDisk (in 2016) appears to be in the same format as UniqueId in Get-Disk (in 2012 R2). I haven't checked to see if Get-PhysicalDisk in 2012 R2 is the same format too.

Zuldan commented Jul 1, 2017

Ok maybe a sign of hope.

UniqueId in Get-PhysicalDisk (in 2016) appears to be in the same format as UniqueId in Get-Disk (in 2012 R2). I haven't checked to see if Get-PhysicalDisk in 2012 R2 is the same format too.

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jul 1, 2017

Collaborator

Oh no worries - it's all good 😁

I'm going to keep the feature in there anyway as I think it may still be useful for the reasons you've suggested. I also improved the code a little so that if there are other disk identifiers we can use then it should be easier to update in future.

You can try it out here:
https://github.com/PlagueHO/xStorage/tree/Issue-104

I've just got to fix the unit tests and I should be able to submit a PR for review.

I'll

Collaborator

PlagueHO commented Jul 1, 2017

Oh no worries - it's all good 😁

I'm going to keep the feature in there anyway as I think it may still be useful for the reasons you've suggested. I also improved the code a little so that if there are other disk identifiers we can use then it should be easier to update in future.

You can try it out here:
https://github.com/PlagueHO/xStorage/tree/Issue-104

I've just got to fix the unit tests and I should be able to submit a PR for review.

I'll

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jul 2, 2017

@PlagueHO thank you very much. I'll give it a test run in the lab tomorrow.

What are your thoughts on UniqueId in Get-PhysicalDisk in 2016?

Zuldan commented Jul 2, 2017

@PlagueHO thank you very much. I'll give it a test run in the lab tomorrow.

What are your thoughts on UniqueId in Get-PhysicalDisk in 2016?

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jul 2, 2017

Collaborator

Hi @Zuldan - I'm not sure. I used Get-PhysicalDisk and Get-Disk on one of my WS 2016 servers and both of them displayed the same uniqueId values - although the disks were returned in a different order.

So I'm not sure we can rely on this.

Collaborator

PlagueHO commented Jul 2, 2017

Hi @Zuldan - I'm not sure. I used Get-PhysicalDisk and Get-Disk on one of my WS 2016 servers and both of them displayed the same uniqueId values - although the disks were returned in a different order.

So I'm not sure we can rely on this.

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jul 2, 2017

No worries. Thanks for the feedback :)

Zuldan commented Jul 2, 2017

No worries. Thanks for the feedback :)

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jul 3, 2017

@PlagueHO the added GUID support worked a treat today. Thank you very much. A crate of beers to you!

Zuldan commented Jul 3, 2017

@PlagueHO the added GUID support worked a treat today. Thank you very much. A crate of beers to you!

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jul 4, 2017

Collaborator

Awesome @Zuldan ! Really great to hear! Hopefully I can get someone to review the PR so it can be included in the next release. No need for the beers 😁

Collaborator

PlagueHO commented Jul 4, 2017

Awesome @Zuldan ! Really great to hear! Hopefully I can get someone to review the PR so it can be included in the next release. No need for the beers 😁

@Zuldan

This comment has been minimized.

Show comment
Hide comment
@Zuldan

Zuldan Jul 4, 2017

I think I came across a bug today. If you change a fully configured volumes drive letter to something it shouldn't be then runs consistency check the resource says something like "creating partition on disk blah blah and assigning drive letter blah blah then fails saying not enough disk space. 2 issues from that, it shouldn't say creating partition when one exists and it's trying to do something funky with expanding the volume even though only the drive letter changed. I forgot to grab a copy of the exact wording. Doh! I'll try re-create the problem and create an issue for it. Just thought I'd let you know before hand.

Zuldan commented Jul 4, 2017

I think I came across a bug today. If you change a fully configured volumes drive letter to something it shouldn't be then runs consistency check the resource says something like "creating partition on disk blah blah and assigning drive letter blah blah then fails saying not enough disk space. 2 issues from that, it shouldn't say creating partition when one exists and it's trying to do something funky with expanding the volume even though only the drive letter changed. I forgot to grab a copy of the exact wording. Doh! I'll try re-create the problem and create an issue for it. Just thought I'd let you know before hand.

@PlagueHO

This comment has been minimized.

Show comment
Hide comment
@PlagueHO

PlagueHO Jul 4, 2017

Collaborator

Hi @Zuldan - this is most like the same cause as this one:
#103

But definitely report this if you can with logs and I'll get onto it... as soon as I can 😁

Collaborator

PlagueHO commented Jul 4, 2017

Hi @Zuldan - this is most like the same cause as this one:
#103

But definitely report this if you can with logs and I'll get onto it... as soon as I can 😁

@PlagueHO PlagueHO closed this in 11b1d7f Jul 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment