Skip to content
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

Implement disk.GetDiskSerialNumber for Windows via WMI #541

Merged
merged 3 commits into from
Jun 23, 2018

Conversation

sify21
Copy link

@sify21 sify21 commented Jun 22, 2018

Fix shirou/gopsutil#435. Parameter should be drive letter, so it can be used in IOCountersStat directly. See last part of https://msdn.microsoft.com/en-us/library/windows/desktop/aa394592(v=vs.85).aspx.

@sify21
Copy link
Author

sify21 commented Jun 22, 2018

Tested on windows 7/server 2016. The result is right, but slow.

InterfaceType string
SerialNumber string
}
type Win32_DiskPartition struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't export these types (I know there are similarly exported wmi types in other gopsutil packages but that's a mistake as Windows-only and thus should not be part of the public API).

@shirou
Copy link
Owner

shirou commented Jun 23, 2018

Thank you! This works on my Windows 10 environment.

@shirou shirou merged commit a9c2f23 into shirou:master Jun 23, 2018
@Lomanic
Copy link
Collaborator

Lomanic commented Jun 23, 2018

I wouldn't have merged this PR @shirou, on Windows a letter C:, D:, etc is a disk partition and not a physical disk (\.\PHYSICALDRIVEX is), see my commit message at Lomanic@8905b92.

Plus, this result should be compared to the one found on Linux, see https://stackoverflow.com/q/27297348/ (that's why I did not create a PR for my commit).

@shirou
Copy link
Owner

shirou commented Jun 23, 2018

ahhh.. I'm sorry. Could you revert this merge ? > @Lomanic

shirou added a commit that referenced this pull request Jun 23, 2018
This reverts commit a9c2f23, reversing
changes made to ebfe800.
@shirou
Copy link
Owner

shirou commented Jun 23, 2018

I have reverted this PR. sorry.

@sify21
Copy link
Author

sify21 commented Jun 25, 2018

Hi @Lomanic , to your first question, I did this on purpose so that the serial number can be returned with iocounters, like that on linux (/boot/efi, /, /home also not physical disk). Otherwise users of this library will have to find the physical disk id related to the iocounter's name themselves.

As to the second question, This method is supposed to be used on windows, so I think it's ok if all windows system do the reverse thing.

@Lomanic
Copy link
Collaborator

Lomanic commented Jun 28, 2018

I think we should have the following:

  • GetDiskSerialNumber on Windows should accept both \.\PHYSICALDRIVEX and X: paths (there is \Device\HarddiskVolumeX\ too, let's skip it for the moment, we would have to port this function, that also will be needed to get rid of some wmi in process AFAIR). Please note that this is the behavior of gopsutil on Linux where GetDiskSerialNumber obtains the right serial number if we give it a partition /dev/sda1 or a device /dev/sda.
  • (optional but nice to have) a disk.PhysicalDisks function to return physical disks instead of partitions (with \.\PHYSICALDRIVEX paths on Windows). disk.PartitionStat includes a device field, but the user has to deduplicate results and it's not even implemented on Windows.
  • the same serial numbers should be returned no matter the OS, if wmi inverts them on Windows, we shall reverse them back

So we might merge back this PR, but with some checks added if we query a physical drive path.

@sify21
Copy link
Author

sify21 commented Jul 7, 2018

Hi @Lomanic , I tested on my local machine (I have dual system installed, win10 professional 1607 1493.953, and fedora 4.16.12-200.fc27). Here is the result I get:

  • Disk serial number on windows and on linux are the same. I believe windows team has fixed that bug with wmi at some point.
  • On linux gopsutil returns the serial number with the format {Model Name}_{Serial Number}, windows version should keep the same.(I made a little change on my custom branch)
  • This wmi approach is buggy. It's slow, and sometimes it's too slow to get any result, especially right after the windows system boot up. I guess wmi needs some time to warm up?(I came across similar problem when using oshi). So I suggest use another method to achieve this goal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants